Bug 79823

Summary: [Chromium] Every compositing layout test with render surfaces is flaky
Product: WebKit Reporter: Adam Klein <adamk>
Component: Tools / TestsAssignee: James Robinson <jamesr>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, bashi, enne, haraken, jamesr, jannywatson1999, morrita, senorblanco, shawnsingh, suchiabram, trchen, ukai, vangelis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch enne: review+

Adam Klein
Reported 2012-02-28 12:40:37 PST
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.
Attachments
Patch (5.50 KB, patch)
2012-03-14 13:30 PDT, James Robinson 		enne: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Klein
Comment 1 2012-02-28 16:00:38 PST
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
Comment 2 2012-02-28 16:03:35 PST
Committed r109164: <http://trac.webkit.org/changeset/109164>
Adam Barth
Comment 3 2012-03-13 15:04:14 PDT
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
Comment 4 2012-03-13 15:13:38 PDT
Committed r110621: <http://trac.webkit.org/changeset/110621>
Adam Barth
Comment 5 2012-03-13 15:13:54 PDT
Not fixed...
Adam Barth
Comment 6 2012-03-13 15:35:25 PDT
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
Comment 7 2012-03-13 15:35:44 PDT
+enne for additional thoughts.
Adrienne Walker
Comment 8 2012-03-13 15:57:57 PDT
(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
Comment 9 2012-03-13 16:15:23 PDT
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
Comment 10 2012-03-13 16:57:30 PDT
(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
Comment 11 2012-03-13 22:59:05 PDT
*** Bug 81080 has been marked as a duplicate of this bug. ***
James Robinson
Comment 12 2012-03-14 11:23:19 PDT
Taking a look - this is really bad :/
Stephen White
Comment 13 2012-03-14 13:07:40 PDT
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
Comment 14 2012-03-14 13:17:50 PDT
(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
Comment 15 2012-03-14 13:21:29 PDT
Yeah, there's a bug with caching a CCRenderSurface pointer in LayerRendererChromium. Patch on the way.
James Robinson
Comment 16 2012-03-14 13:22:40 PDT
*** Bug 80563 has been marked as a duplicate of this bug. ***
James Robinson
Comment 17 2012-03-14 13:25:17 PDT
*** Bug 74731 has been marked as a duplicate of this bug. ***
James Robinson
Comment 18 2012-03-14 13:27:07 PDT
*** Bug 80007 has been marked as a duplicate of this bug. ***
James Robinson
Comment 19 2012-03-14 13:27:28 PDT
*** Bug 79572 has been marked as a duplicate of this bug. ***
James Robinson
Comment 20 2012-03-14 13:30:23 PDT
Created attachment 131910 [details] Patch
James Robinson
Comment 21 2012-03-14 13:31:27 PDT
*** Bug 74050 has been marked as a duplicate of this bug. ***
Adrienne Walker
Comment 22 2012-03-14 13:35:16 PDT
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
Comment 23 2012-03-14 13:52:53 PDT
Committed r110741: <http://trac.webkit.org/changeset/110741>
James Robinson
Comment 24 2012-03-14 14:54:15 PDT
*** Bug 79647 has been marked as a duplicate of this bug. ***
Stephen White
Comment 25 2012-03-15 07:47:14 PDT
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!
Note You need to log in before you can comment on or make changes to this bug.