Bug 197819 - eventSender.monitorWheelEvents() is very fragile
Summary: eventSender.monitorWheelEvents() is very fragile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
: 158237 171839 207120 207848 207861 207948 207953 208160 208471 209255 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-05-11 15:45 PDT by Simon Fraser (smfr)
Modified: 2020-03-21 12:10 PDT (History)
20 users (show)

See Also:


Attachments
Test with issues (3.03 KB, text/html)
2019-05-11 15:45 PDT, Simon Fraser (smfr)
no flags Details
Patch (57.50 KB, patch)
2020-03-18 16:57 PDT, Simon Fraser (smfr)
thorton: review+
Details | Formatted Diff | Diff
Patch (58.67 KB, patch)
2020-03-18 17:20 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (59.19 KB, patch)
2020-03-18 18:45 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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. ***