Bug 66267 – [chromium] Update scroll-mechanism from GestureRecognizer

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Sadrul Habib Chowdhury]

This patch updates how the gesture-recognizer performs scroll: Instead of just sending a series of 'GestureUpdate' events, send a single 'GestureBegin' event, followed by a series of 'GestureUpdate' events, and end with a 'GestureEnd' event.

This change is covered by existing tests (e.g. fast/events/touch/touch-gesture-scroll.html). A series of subsequent patches will update how the EventHandler processes the ScrollUpdate gesture-event, add support in EventSender to send gesture-update events from the DRT, and include additional layout tests.

[Sadrul Habib Chowdhury]

Created attachment 103983 [details]
patch

[Robert Kroeger]

Comment on attachment 103983 [details]
patch

looks good to me. And is a step towards what I'm proposing to Benjamin. Can you add one or more unit tests that demonstrate that the new protocol of start, update, end is getting generated?

[Sadrul Habib Chowdhury]

Created attachment 104394 [details]
added a unit-test

I have added a unit-test to test that the correct gesture events are being generated from touch-events. Please take another look.

[WebKit Review Bot]

Attachment 104394 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp:81:  The parameter name "state" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp:278:  gesture_start is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebKit/chromium/tests/InnerGestureRecognizerTest.cpp:298:  gesture_end is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sadrul Habib Chowdhury]

Created attachment 104396 [details]
fixed style

[Adam Barth]

Comment on attachment 104396 [details]
fixed style

Looks reasonable to me.  I'm relying on  Robert Kroeger's comments for actual knowledge of scrolling.  :)

[WebKit Review Bot]

Comment on attachment 104396 [details]
fixed style

Clearing flags on attachment: 104396

Committed r93358: <http://trac.webkit.org/changeset/93358>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

After this patch landed, the WebKit Mac 10.5 canary bots started to fail 5 gesture tests.  This patch seems to be the most likely culprit:
http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.5%20%28CG%29/builds/35

Specifically, it looks like the tests started to time out:
http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.5%20%28CG%29/builds/35

Can you please investigate?

[Tony Chang]

(In reply to comment #9)

> Specifically, it looks like the tests started to time out:
> http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.5%20%28CG%29/builds/35

Oops, this link was supposed to be:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fevents%2Ftouch%2Fbasic-single-touch-events.html%2Cfast%2Fevents%2Ftouch%2Fmulti-touch-grouped-targets.html%2Cfast%2Fevents%2Ftouch%2Ftouch-gesture-scroll.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target-limited.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target.html

[Robert Kroeger]

(In reply to comment #10)
> (In reply to comment #9)
> 
> > Specifically, it looks like the tests started to time out:
> > http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.5%20%28CG%29/builds/35
> 
> Oops, this link was supposed to be:
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fevents%2Ftouch%2Fbasic-single-touch-events.html%2Cfast%2Fevents%2Ftouch%2Fmulti-touch-grouped-targets.html%2Cfast%2Fevents%2Ftouch%2Ftouch-gesture-scroll.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target-limited.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target.html

I'll look into it.