[webkit-reviews] review granted: [Bug 66531] Don't detach elements from the render tree when entering fullscreen mode : [Attachment 106371] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted James Kozianski
<koz at chromium.org>'s request for review:
Bug 66531: Don't detach elements from the render tree when entering fullscreen
mode
https://bugs.webkit.org/show_bug.cgi?id=66531

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106371&action=review


The C++ changes all look good, but I have some concerns about how the test is
working.

> LayoutTests/plugins/fullscreen-plugins-dont-reload-expected.txt:4
> +ALERT: Plugin Loaded!
> +layer at (0,0) size 800x600
> +  RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600

I can't really tell what this test output is trying to tell me.  What happens
if the test fails?  Is the render tree part of the expected test output?

Are you trying to tell if the plugin is loading once or twice?	If so then this
should be dumpAsText() and probably have some sort of text in the output
indicating what is supposed to be there.

> LayoutTests/plugins/fullscreen-plugins-dont-reload.html:21
> +    }, 100);

why setTimeout(..., 100)? that seems like a recipe for flake - is there
something in particular you are waiting for?