Bug 79823 – [Chromium] Every compositing layout test with render surfaces is flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Klein]

The following layout test is failing flakily on all platforms:

compositing/repaint/opacity-between-absolute.html

See http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=compositing%2Frepaint%2Fopacity-between-absolute.html

From the dashboard, http://trac.webkit.org/changeset/108886/ looks like a possibility.  Since that change, the test has been failing consistently on Linux dbg, and most of the time on Windows.

[Adam Klein]

Also seeing similar flakiness (similarly correlated with 108886) in compositing/reflections/masked-reflection-on-composited.html:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=compositing%2Freflections%2Fmasked-reflection-on-composited.html

[Adam Klein]

Committed r109164: <http://trac.webkit.org/changeset/109164>

[Adam Barth]

New, seemingly related failures:

compositing/geometry/fixed-position.html
compositing/reflections/compositing-change-inside-reflection.html
compositing/reflections/reflection-on-composited.html
compositing/webgl/webgl-reflection.html
platform/chromium/compositing/render-surface-alpha-blending.html

[Adam Barth]

Committed r110621: <http://trac.webkit.org/changeset/110621>

[Adam Barth]

Not fixed...

[Adam Barth]

jamesr: I think there's something wrong with the compositor dropping images.  Here's another example:

compositing/masks/direct-image-mask.html

http://build.chromium.org/f/chromium/layout_test_results/Webkit_Linux_32/results/layout-test-results/compositing/masks/direct-image-mask-diff.png

It's some kind of flaky problem, which makes it hard to track down.  Have their been any changes related to masking and/or reflections in the compositor recently?

[Adam Barth]

+enne for additional thoughts.

[Adrienne Walker]

(In reply to comment #7)
> +enne for additional thoughts.

I'd be kind of surprised if r108886 broke things in a flaky manner, but I don't have any better suggestions or insight here, sorry.

[Tien-Ren Chen]

I probably see why r108886 can cause a problem with reflection. The LayerChromium is shared between its owner GraphicsLayer and also the reflection GraphicsLayer (as replica). The impl-side clone shouldn't really use an OwnPtr for the replica layer.

I think I can come up with a fix reasonably soon. Just don't know why the tests are just flaky instead of failing constantly.

[Tien-Ren Chen]

(In reply to comment #9)
> I probably see why r108886 can cause a problem with reflection. The LayerChromium is shared between its owner GraphicsLayer and also the reflection GraphicsLayer (as replica). The impl-side clone shouldn't really use an OwnPtr for the replica layer.
> 
> I think I can come up with a fix reasonably soon. Just don't know why the tests are just flaky instead of failing constantly.

NVM, it was a wrong guess. The replica layers don't seem to be shared.

[Hajime Morrita]

*** Bug 81080 has been marked as a duplicate of this bug. ***

[James Robinson]

Taking a look - this is really bad :/

[Stephen White]

Some other layout tests with similar symptoms:

https://bugs.webkit.org/show_bug.cgi?id=79572
https://bugs.webkit.org/show_bug.cgi?id=80563
http://crbug.com/114777

Those bugs all seem to have RenderSurface in common, which  I think that's also used for reflections and masks, which are some of the failures reported on this bug.  Could be related to RenderSurface in some way?

[Stephen White]

(In reply to comment #13)
> Some other layout tests with similar symptoms:
> 
> https://bugs.webkit.org/show_bug.cgi?id=79572
> https://bugs.webkit.org/show_bug.cgi?id=80563
> http://crbug.com/114777
> 
> Those bugs all seem to have RenderSurface in common, which  I think that's also used for reflections and masks, which are some of the failures reported on this bug.  Could be related to RenderSurface in some way?

By RenderSurface I meant CCRenderSurface, of course.  (And by <really poor grammar> I meant <really excellent grammar>.)

[James Robinson]

Yeah, there's a bug with caching a CCRenderSurface pointer in LayerRendererChromium. Patch on the way.

[James Robinson]

*** Bug 80563 has been marked as a duplicate of this bug. ***

[James Robinson]

*** Bug 74731 has been marked as a duplicate of this bug. ***

[James Robinson]

*** Bug 80007 has been marked as a duplicate of this bug. ***

[James Robinson]

*** Bug 79572 has been marked as a duplicate of this bug. ***

[James Robinson]

Created attachment 131910 [details]
Patch

[James Robinson]

*** Bug 74050 has been marked as a duplicate of this bug. ***

[Adrienne Walker]

Comment on attachment 131910 [details]
Patch

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

R=me.  Nice catch.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:-395
> -    // The GL viewport covers the entire visible area, including the scrollbars.
> -    GLC(m_context.get(), m_context->viewport(0, 0, viewportWidth(), viewportHeight()));
> -    m_windowMatrix = screenMatrix(0, 0, viewportWidth(), viewportHeight());
> -

Good riddance to this too.

[James Robinson]

Committed r110741: <http://trac.webkit.org/changeset/110741>

[James Robinson]

*** Bug 79647 has been marked as a duplicate of this bug. ***

[Stephen White]

Thanks!  I had it on my list to look into this after my gardening shift, but I got sidelined by other things.  Fix is much appreciated!