[Webkit-unassigned] [Bug 10230] SVGDOMImplementation should die (and be rolled into DOMImplementation)

bugzilla-daemon at opendarwin.org bugzilla-daemon at opendarwin.org
Sat Aug 5 16:18:57 PDT 2006


http://bugzilla.opendarwin.org/show_bug.cgi?id=10230


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #9894|review+                     |review-
               Flag|                            |




------- Comment #13 from darin at apple.com  2006-08-05 16:18 PDT -------
(From update of attachment 9894)
(In reply to comment #12)
> Could you provide a bit more detail about what's wrong with the hash additions?

This change:

     template<> struct StrHash<WebCore::StringImpl*> {
+        static unsigned hash(const WebCore::String& key) { return
key.impl()->hash(); }
         static unsigned hash(const WebCore::StringImpl* key) { return
key->hash(); }

should be unnecessary. That's a string hash for StringImpl* and it doesn't make
sense to also add a hash to it for String, nor should it be needed.

The two equal function overloads inside CaseInsensitiveHash should be OK. It's
reasonable to try to make CaseInsensitiveHash work for both String and
StringImpl

But this one is definitely wrong:

+        static const WebCore::String& deletedValue() {
+            static WebCore::String null;
+            return null;
+        }

It's going to create a separate global variable for each translation unit that
it's used in. It's also going to make empty values and deleted values be the
same, since emptyValueIsZero causes a String with all values 0 (any null
string) be the empty value. The hash code goes completely awry if empty values
and deleted values are the same.


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



More information about the webkit-unassigned mailing list