eventSender.monitorWheelEvents() is very fragile

[Simon Fraser (smfr)]

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.

[Radar WebKit Bug Importer]

<rdar://problem/51319456>

[Simon Fraser (smfr)]

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)]

Going to look at a UIScriptController replacement.

[Simon Fraser (smfr)]

Created attachment 393917 [details]
Patch

[Simon Fraser (smfr)]

*** Bug 209255 has been marked as a duplicate of this bug. ***

[Tim Horton]

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)]

Created attachment 393923 [details]
Patch

[Alexey Proskuryakov]

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)]

(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)]

Created attachment 393933 [details]
Patch

[EWS]

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)]

*** Bug 208160 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 208471 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207518 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207848 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207953 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207861 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207120 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 171839 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 207948 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 158237 has been marked as a duplicate of this bug. ***