[Webkit-unassigned] [Bug 20952] QtWebKit needs the old history pull model for API Compability

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=20952


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23590|review?                     |review-
               Flag|                            |




------- Comment #5 from darin at apple.com  2008-10-03 13:06 PDT -------
(From update of attachment 23590)
I'm disappointed that this makes the visited link code more complicated --
those two new Document functions have interfaces that are hard to understand
with a mysterious boolean result and a fixed size buffer in the header, while
visitedLinkHash is so much easier to understand!

Can we remove this in the future? Would you be willing to change the Qt API?

This is an area we are likely to want to refine further -- I had been planning
to change the algorithm to be faster and not require putting the URL into a
buffer. The legacy dependency here is going to make it hard to make that
improvement.

+// We have to fallback to the old historyContains behaviour for the Qt4.4 API

Should be "fall back", the verb, rather than "fallback", the noun.

 unsigned Document::visitedLinkHash(const AtomicString& attributeURL) const
 {
-    const UChar* characters = attributeURL.characters();
     unsigned length = attributeURL.length();
     if (!length)
         return 0;

It doesn't make sense to put this into a local variable when it's only used
once. I also don't think we need a special case for 0 in this function any
more. The other functions already have special cases.

+    return AlreadyHashed::avoidDeletedValue(result ?
+                                           
attributeURL.string().impl()->hash() :
+                                           
StringImpl::computeHash(buffer.data(), buffer.size()));

This is not our usual formatting. Maybe you need to use another local variable
to avoid having this deeply indented ? : expression.

     if (hasColonSlashSlash && !needsTrailingSlash(characters, length))
-        return
AlreadyHashed::avoidDeletedValue(attributeURL.string().impl()->hash());
-
-    Vector<UChar, 512> buffer;
+        return true;

I think this needs to be return false, not true.

+        result.append(characters, length);
+        result.append('/');
+        return false;

I think this needs to be return true, not false.

-    return
AlreadyHashed::avoidDeletedValue(StringImpl::computeHash(buffer.data(),
buffer.size()));
+    return false;

I think this needs to be return true, not false.

How did you test?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.