REGRESSION (r252205?): [ Mac wk2 ] tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html became very flaky

• Component: Scrolling
• Status: RESOLVED FIXED

[Truitt Savell]

tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html

Description:
This test has been flaky for a while, history first shows failures starting around r251229.

History:
https://results.webkit.org/?suite=layout-tests&test=tiled-drawing%2Fscrolling%2Ffast-scroll-select-latched-mainframe-with-handler.html&limit=23718

Diff:
--- /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler-expected.txt
+++ /Volumes/Data/slave/catalina-debug-tests-wk2/build/layout-test-results/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler-actual.txt
@@ -14,15 +14,15 @@
 (GraphicsLayer
   (anchor 0.00 0.00)
   (bounds 2008.00 2266.00)
-  (visible rect 0.00, 70.00 785.00 x 585.00)
-  (coverage rect 0.00, 70.00 785.00 x 585.00)
+  (visible rect 0.00, 0.00 785.00 x 585.00)
+  (coverage rect 0.00, 0.00 785.00 x 585.00)
   (intersects coverage rect 1)
   (contentsScale 1.00)
   (children 1
     (GraphicsLayer
       (bounds 2008.00 2266.00)
       (contentsOpaque 1)
-      (visible rect 0.00, 70.00 785.00 x 585.00)
+      (visible rect 0.00, 0.00 785.00 x 585.00)
       (coverage rect 0.00, 0.00 1570.00 x 1755.00)
       (intersects coverage rect 1)
       (contentsScale 1.00)

[Radar WebKit Bug Importer]

<rdar://problem/58609558>

[Truitt Savell]

This reproduces by running the test in iterations.

[Truitt Savell]

I am able to reproduce this on ToT and as far back as r251002, so I am unsure where this really regressed

[Truitt Savell]

Marking test as failing on Mac Debug wk2 while this is investigated:
https://trac.webkit.org/changeset/254577/webkit

[Alexey Proskuryakov]

This test was failing rarely intake November 7th, when it turned into a very frequent failure. r252205 seems like a very likely culprit.

[Alexey Proskuryakov]

> was failing rarely intake November 7th

until November 7th

[Ryosuke Niwa]

That seems like a reasonable assessment to me.

[Simon Fraser (smfr)]

Created attachment 388124 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 388124 [details]
Patch

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

> LayoutTests/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html:69
> +        eventSender.callAfterScrollingCompletes(checkForScroll);

We should probably wait for requestAnimationFrame after callAfterScrollingCompletes.

[Simon Fraser (smfr)]

(In reply to Ryosuke Niwa from comment #9)
> Comment on attachment 388124 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388124&action=review
> 
> > LayoutTests/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html:69
> > +        eventSender.callAfterScrollingCompletes(checkForScroll);
> 
> We should probably wait for requestAnimationFrame after
> callAfterScrollingCompletes.

Ideally eventSender.callAfterScrollingCompletes() would do that internally.

[Ryosuke Niwa]

Comment on attachment 388124 [details]
Patch

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

>>> LayoutTests/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html:69
>>> +        eventSender.callAfterScrollingCompletes(checkForScroll);
>> 
>> We should probably wait for requestAnimationFrame after callAfterScrollingCompletes.
> 
> Ideally eventSender.callAfterScrollingCompletes() would do that internally.

But does it?

[WebKit Commit Bot]

Comment on attachment 388124 [details]
Patch

Clearing flags on attachment: 388124

Committed r254793: <https://trac.webkit.org/changeset/254793>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

(In reply to Ryosuke Niwa from comment #11)
> Comment on attachment 388124 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388124&action=review
> 
> >>> LayoutTests/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html:69
> >>> +        eventSender.callAfterScrollingCompletes(checkForScroll);
> >> 
> >> We should probably wait for requestAnimationFrame after callAfterScrollingCompletes.
> > 
> > Ideally eventSender.callAfterScrollingCompletes() would do that internally.
> 
> But does it?

No. But we shouldn't need to wait for rAF; DRT and WTR call Page::updateRendering() when tests complete.

[Ryosuke Niwa]

(In reply to Simon Fraser (smfr) from comment #14)
> (In reply to Ryosuke Niwa from comment #11)
> > Comment on attachment 388124 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=388124&action=review
> > 
> > >>> LayoutTests/tiled-drawing/scrolling/fast-scroll-select-latched-mainframe-with-handler.html:69
> > >>> +        eventSender.callAfterScrollingCompletes(checkForScroll);
> > >> 
> > >> We should probably wait for requestAnimationFrame after callAfterScrollingCompletes.
> > > 
> > > Ideally eventSender.callAfterScrollingCompletes() would do that internally.
> > 
> > But does it?
> 
> No. But we shouldn't need to wait for rAF; DRT and WTR call
> Page::updateRendering() when tests complete.

Ah, I see.