Bug 95732 – [chromium] Dynamically removing preserve-3d from a layer has no effect

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Sami Kyöstilä]

http://trac.webkit.org/changeset/126378 accidentally broke the logic for removing preserve-3d in GraphicsLayerChromium::updateLayerPreserves3D().

[Sami Kyöstilä]

Created attachment 162025 [details]
Patch

[James Robinson]

Comment on attachment 162025 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162025&action=review

D'oh! Thanks for adding the test, but why is it in platform/chromium? This should work anywhere, shouldn't it?

> LayoutTests/platform/chromium/compositing/preserve-3d-toggle.html:52
> +      }, 0);

why the setTimeout?

[Sami Kyostila]

Thanks for the review.

(In reply to comment #2)
> (From update of attachment 162025 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162025&action=review
> 
> D'oh! Thanks for adding the test, but why is it in platform/chromium? This should work anywhere, shouldn't it?

Right, I just put it there because this was a chromium bug. I'll make it a shared test.

> > LayoutTests/platform/chromium/compositing/preserve-3d-toggle.html:52
> > +      }, 0);
> 
> why the setTimeout?

It's needed so that we first do a commit cycle with preserve-3d enabled and then another one with it disabled. Otherwise we won't hit bug in question, since there needs to be an existing layer with preserve-3d turned on.

[Sami Kyöstilä]

Created attachment 162195 [details]
Patch

[WebKit Review Bot]

Comment on attachment 162195 [details]
Patch

Attachment 162195 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13740936

New failing tests:
http/tests/cache/subresource-expiration-1.html
http/tests/cache/stopped-revalidation.html

[James Robinson]

Comment on attachment 162195 [details]
Patch

Silly cq, let's try again

[WebKit Review Bot]

Comment on attachment 162195 [details]
Patch

Rejecting attachment 162195 [details] from commit-queue.

New failing tests:
http/tests/cache/subresource-expiration-1.html
http/tests/cache/stopped-revalidation.html
Full output: http://queues.webkit.org/results/13768069

[Sami Kyöstilä]

Not seeing those failures locally, so let's give it another go...

[Sami Kyöstilä]

Created attachment 162734 [details]
Patch

[WebKit Review Bot]

Comment on attachment 162734 [details]
Patch

Rejecting attachment 162734 [details] from commit-queue.

New failing tests:
inspector/styles/protocol-css-regions-commands.html
Full output: http://queues.webkit.org/results/13774726

[WebKit Review Bot]

Comment on attachment 162734 [details]
Patch

Clearing flags on attachment: 162734

Committed r127865: <http://trac.webkit.org/changeset/127865>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Sami Kyöstilä]

D'oh, looks like I picked up the wrong version of this patch when rebasing which still had the tests in platform/chromium/compositing instead of compositing/. Let's fix that...

[Sami Kyöstilä]

Created attachment 162760 [details]
Patch

Move tests to correct directory.

[WebKit Review Bot]

Comment on attachment 162760 [details]
Patch

Clearing flags on attachment: 162760

Committed r127908: <http://trac.webkit.org/changeset/127908>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.