Bug 197819

Summary: eventSender.monitorWheelEvents() is very fragile
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Tools / TestsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, cdumez, cmarcelo, esprehn+autocc, ews-watchlist, fred.wang, jacob_uphoff, jamesr, jlewis3, kangil.han, Lawrence.j, lforschler, luiz, ryanhaddad, simon.fraser, thorton, tonikitoo, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=148407
https://bugs.webkit.org/show_bug.cgi?id=198757
Attachments:
Description Flags
Test with issues
none
Patch
thorton: review+
Patch
none
Patch none

Description Simon Fraser (smfr) 2019-05-11 15:45:11 PDT
Created attachment 369664 [details]
Test with issues

I can't get a test using eventSender.monitorWheelEvents() to work. Test attached.

It always times out and I have no idea why. See comments in the test.
Comment 1 Radar WebKit Bug Importer 2019-05-31 16:27:09 PDT
<rdar://problem/51319456>
Comment 2 Simon Fraser (smfr) 2019-10-15 20:32:54 PDT
I've spent way too long trying to make the eventSender.monitorWheelEvents() more robust. I think the design is fundamentally broken; there's no way to tell, when you get the 'ended' phase on the scroll gesture, that you'll be starting a momentum phase and need to keep postponing the final callback.

A better design would be to use JSON to describe the sequence of events, and make a single call into EventSender that starts monitoring, and knows to keep monitoring until all the events have been dispatched and received by the web process.
Comment 3 Simon Fraser (smfr) 2019-10-16 13:34:00 PDT
Going to look at a UIScriptController replacement.
Comment 4 Simon Fraser (smfr) 2020-03-18 16:57:18 PDT
Created attachment 393917 [details]
Patch
Comment 5 Simon Fraser (smfr) 2020-03-18 16:57:22 PDT
*** Bug 209255 has been marked as a duplicate of this bug. ***
Comment 6 Tim Horton 2020-03-18 17:02:54 PDT
Comment on attachment 393917 [details]
Patch

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

> Tools/DumpRenderTree/mac/EventSendingController.mm:855
> +    [[[mainFrame frameView] documentView] layout];

!

> Tools/DumpRenderTree/mac/EventSendingController.mm:881
> +    if (phase == 4 /* kCGScrollPhaseEnded */ || phase == 8 /* kCGScrollPhaseCancelled */)

Why can't we use the constants?
Comment 7 Simon Fraser (smfr) 2020-03-18 17:20:59 PDT
Created attachment 393923 [details]
Patch
Comment 8 Alexey Proskuryakov 2020-03-18 17:34:03 PDT
Comment on attachment 393923 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Deflake tests using eventSender.monitorWheelEvents() by fixing several causes of flakiness,

This removes flaky expectation for one test, and changes another. Are you planning to watch results for other such tests, and remove the expectations after a while?
Comment 9 Simon Fraser (smfr) 2020-03-18 18:02:42 PDT
(In reply to Alexey Proskuryakov from comment #8)
> Comment on attachment 393923 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=393923&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Deflake tests using eventSender.monitorWheelEvents() by fixing several causes of flakiness,
> 
> This removes flaky expectation for one test, and changes another. Are you
> planning to watch results for other such tests, and remove the expectations
> after a while?

I plan to remove flakey expectations for all tests which I suspect have been suffering from monitorWheelEvents() flakiness, after some local testing and EWSing. We might have to put some back if it turns out I did not fix them. I'll do that after this has landed.
Comment 10 Simon Fraser (smfr) 2020-03-18 18:45:33 PDT
Created attachment 393933 [details]
Patch
Comment 11 EWS 2020-03-18 21:06:42 PDT
Committed r258679: <https://trac.webkit.org/changeset/258679>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 393933 [details].
Comment 12 Simon Fraser (smfr) 2020-03-21 11:49:20 PDT
*** Bug 208160 has been marked as a duplicate of this bug. ***
Comment 13 Simon Fraser (smfr) 2020-03-21 11:52:52 PDT
*** Bug 208471 has been marked as a duplicate of this bug. ***
Comment 14 Simon Fraser (smfr) 2020-03-21 11:56:09 PDT
*** Bug 207518 has been marked as a duplicate of this bug. ***
Comment 15 Simon Fraser (smfr) 2020-03-21 11:57:47 PDT
*** Bug 207848 has been marked as a duplicate of this bug. ***
Comment 16 Simon Fraser (smfr) 2020-03-21 11:58:44 PDT
*** Bug 207953 has been marked as a duplicate of this bug. ***
Comment 17 Simon Fraser (smfr) 2020-03-21 12:01:06 PDT
*** Bug 207861 has been marked as a duplicate of this bug. ***
Comment 18 Simon Fraser (smfr) 2020-03-21 12:03:47 PDT
*** Bug 207120 has been marked as a duplicate of this bug. ***
Comment 19 Simon Fraser (smfr) 2020-03-21 12:06:04 PDT
*** Bug 171839 has been marked as a duplicate of this bug. ***
Comment 20 Simon Fraser (smfr) 2020-03-21 12:07:20 PDT
*** Bug 207948 has been marked as a duplicate of this bug. ***
Comment 21 Simon Fraser (smfr) 2020-03-21 12:10:58 PDT
*** Bug 158237 has been marked as a duplicate of this bug. ***