[webkit-reviews] review denied: [Bug 41019] Canvas: Remember verified clean origins for drawImage() : [Attachment 59427] Proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 41019: Canvas: Remember verified clean origins for drawImage()
https://bugs.webkit.org/show_bug.cgi?id=41019

Attachment 59427: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=59427&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Why is this optimization important? It seems dangerous to cache security
decisions because the conditions the security decision was based on could
change in theory, so we should only do it if we absolutely need to. How did you
determine the optimization was needed? Did this come up in some real world
benchmark?

Is the common code path for checkOrigin the KURL one or the String one? I ask
because we could easily store strings in the hash instead of URLs and then we
could do the fast check before even turning a string into a URL. It's free to
turn an URL back into a string with string(), and strings hash even better than
URLs do, so I think you should really consider using strings instead of URLs as
the hash keys.

> +	   ListHashSet<KURL> m_cleanOrigins;

You want HashSet, not ListHashSet. ListHashSet is a more expensive and complex
variant of HashSet that guarantees you can get the elements of the set back in
the order you added them.

review- because this should be HashSet not ListHashSet.