[webkit-reviews] review granted: [Bug 103655] Add PlugInOriginHash : [Attachment 177773] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Dec 6 17:53:21 PST 2012
Alexey Proskuryakov <ap at webkit.org> has granted Jon Lee <jonlee at apple.com>'s
request for review:
Bug 103655: Add PlugInOriginHash
https://bugs.webkit.org/show_bug.cgi?id=103655
Attachment 177773: Patch
https://bugs.webkit.org/attachment.cgi?id=177773&action=review
------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=177773&action=review
Looks like Darin's comments have been addressed.
> Source/WebCore/plugins/PlugInOriginHash.cpp:39
> +static inline void addCaseFoldedCharacters(StringHasher &hasher, const
String& string)
Misplaced & here.
> Source/WebCore/plugins/PlugInOriginHash.cpp:49
> + if (!plugInElement->document()->page())
> + return 0;
That's quite likely to cause collisions!
Perhaps we should require the document to be in page instead of silently
returning a bad hash?
> Source/WebCore/plugins/PlugInOriginHash.cpp:59
> + LOG(Plugins, "Hash: %s %s %s",
plugInElement->document()->page()->mainFrame()->document()->baseURL().host().ut
f8().data(), plugInURL.host().utf8().data(),
plugInElement->serviceType().utf8().data());
Do you still need this? It looks like it will be too chatty with Plugins
channel enabled.
> Source/WebCore/plugins/PlugInOriginHash.h:35
> + static unsigned hash(HTMLPlugInImageElement* plugInElement, const KURL&
plugInURL);
Argument names seem unnecessary here.
More information about the webkit-reviews
mailing list