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

Simon Fraser (smfr)
Reported 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.
Attachments
Test with issues (3.03 KB, text/html)
2019-05-11 15:45 PDT, Simon Fraser (smfr)
no flags
Patch (57.50 KB, patch)
2020-03-18 16:57 PDT, Simon Fraser (smfr)
thorton: review+
Patch (58.67 KB, patch)
2020-03-18 17:20 PDT, Simon Fraser (smfr)
no flags
Patch (59.19 KB, patch)
2020-03-18 18:45 PDT, Simon Fraser (smfr)
no flags
Radar WebKit Bug Importer
Comment 1 2019-05-31 16:27:09 PDT
Simon Fraser (smfr)
Comment 2 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.
Simon Fraser (smfr)
Comment 3 2019-10-16 13:34:00 PDT
Going to look at a UIScriptController replacement.
Simon Fraser (smfr)
Comment 4 2020-03-18 16:57:18 PDT
Simon Fraser (smfr)
Comment 5 2020-03-18 16:57:22 PDT
*** Bug 209255 has been marked as a duplicate of this bug. ***
Tim Horton
Comment 6 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?
Simon Fraser (smfr)
Comment 7 2020-03-18 17:20:59 PDT
Alexey Proskuryakov
Comment 8 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?
Simon Fraser (smfr)
Comment 9 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.
Simon Fraser (smfr)
Comment 10 2020-03-18 18:45:33 PDT
EWS
Comment 11 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].
Simon Fraser (smfr)
Comment 12 2020-03-21 11:49:20 PDT
*** Bug 208160 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 13 2020-03-21 11:52:52 PDT
*** Bug 208471 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 14 2020-03-21 11:56:09 PDT
*** Bug 207518 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 15 2020-03-21 11:57:47 PDT
*** Bug 207848 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 16 2020-03-21 11:58:44 PDT
*** Bug 207953 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 17 2020-03-21 12:01:06 PDT
*** Bug 207861 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 18 2020-03-21 12:03:47 PDT
*** Bug 207120 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 19 2020-03-21 12:06:04 PDT
*** Bug 171839 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 20 2020-03-21 12:07:20 PDT
*** Bug 207948 has been marked as a duplicate of this bug. ***
Simon Fraser (smfr)
Comment 21 2020-03-21 12:10:58 PDT
*** Bug 158237 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.