[webkit-reviews] review granted: [Bug 103655] Add PlugInOriginHash : [Attachment 177773] Patch

[bugzilla-daemon at webkit.org]

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.