[webkit-reviews] review denied: [Bug 88908] [chromium] webkit-backface-visibility doesn't work with <video> : [Attachment 148471] Patch

[bugzilla-daemon at webkit.org]

Adrienne Walker <enne at google.com> has denied  review:
Bug 88908: [chromium] webkit-backface-visibility doesn't work with <video>
https://bugs.webkit.org/show_bug.cgi?id=88908

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

------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=148471&action=review


(You can't set R+ yourself.  The webkit-review bot will come by and complain.)

The image test looks good to me.  Are there tests for video/canvas/webgl too? 
I realize that Chromium's implementation sends all these down the same
setupContents path, but the tests will be run by other ports and they might
behave differently.

If you haven't yet, I'll run this patch through Safari and see if this test
should be marked as failing for them, since I know there's some lingering
backface visibility bugs that Shawn filed earlier.

> Source/Platform/chromium/public/WebLayer.h:112
> +    // Set whether this layer is a content layer (e.g, canvas, plugin,
WebGL, or video)

nit: I think this comment is talking too much about the implementation details
of GraphicsLayerChromium and its backface visibility behavior.	There are other
consumers of WebLayer (e.g. ash), so maybe it'd be better to not have a comment
at all; I think the function is well-named enough to describe exactly what it
does.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:252
> +    if (layer->useParentBackfaceVisibility()) {
> +	   ASSERT(layer->parent());
> +	   backfaceTestLayer = layer->parent();
> +    }

This is kind of an edge case, but what happens if a child and its parent both
useParentBackfaceVisibility()?