[webkit-reviews] review granted: [Bug 62257] Disallow use of cross-domain media (images, video) in WebGL : [Attachment 96360] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kenneth Russell <kbr at google.com>'s
request for review:
Bug 62257: Disallow use of cross-domain media (images, video) in WebGL
https://bugs.webkit.org/show_bug.cgi?id=62257

Attachment 96360: Patch
https://bugs.webkit.org/attachment.cgi?id=96360&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96360&action=review

Looks great.  Thanks.

> Source/WebCore/html/canvas/CanvasRenderingContext.cpp:103
> +    m_cleanOrigins.add(url.string());

m_cleanOrigins => m_cleanURLs since this hold URLs and not origins?

> Source/WebCore/html/canvas/CanvasRenderingContext.cpp:130
> +void CanvasRenderingContext::checkOrigin(const CanvasPattern* pattern)
> +{
> +    if (wouldTaintOrigin(pattern))
> +	   canvas()->setOriginTainted();
> +}
> +
> +void CanvasRenderingContext::checkOrigin(const HTMLCanvasElement*
sourceCanvas)
> +{
> +    if (wouldTaintOrigin(sourceCanvas))
> +	   canvas()->setOriginTainted();
> +}
> +
> +void CanvasRenderingContext::checkOrigin(const HTMLImageElement* image)
> +{
> +    if (wouldTaintOrigin(image))
> +	   canvas()->setOriginTainted();
> +}
> +
> +void CanvasRenderingContext::checkOrigin(const HTMLVideoElement* video)
> +{
> +    if (wouldTaintOrigin(video))
> +	   canvas()->setOriginTainted();
> +}

This feel ripe for a template.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2812
> +    UNUSED_PARAM(ec);

You can just remove the name of the ec parameter instead of using this macro.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2815
> +    // It is no longer possible to taint the origin using the WebGL API.

I'd remove this comment.  That's going to read strange in two years.  :)