Bug 66105 – [chromium] Add support for gesture events in the DRT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Sadrul Habib Chowdhury]

This patch adds support for gesture events in the DumpRenderTree. This will allow testing gesture recognition code and gesture handling code separately.

[Sadrul Habib Chowdhury]

Created attachment 103687 [details]
patch

There aren't any gesture-handling code at the moment. So there are not layout tests with this patch. Gesture-handling code with layout tests are forthcoming.

[Robert Kroeger]

(In reply to comment #1)
> Created an attachment (id=103687) [details]
> patch
> 
> There aren't any gesture-handling code at the moment. So there are not layout tests with this patch. Gesture-handling code with layout tests are forthcoming.

I don't think this is strictly true. You could add a layout test to invoke the PlatformGestureEvent for tap and scroll as results from touch events in touch-gesture-click.html and touch-gesture-scroll.html but I would hope that this could happen in a subsequent patch.

[Sadrul Habib Chowdhury]

Created attachment 103782 [details]
patch

Ah, indeed. I have added a layout-test for the scenario you explain for tap (and added WebInputEvent::GestureTap corresponding to PlatforGestureEvent::TapType).

[Robert Kroeger]

Looks reasonable to me. (Though it will probably have to change to track changes to PlatformGestureEvent.)

[Adam Barth]

Comment on attachment 103782 [details]
patch

Technically we need fishd to approve the WebKit API change, but I think this is probably ok.

[Adam Barth]

+fishd for (extremely minor) WebKit API change.

fishd, I marked the patch R+ because the change looked fine, but I wanted you to see the patch.  Let me know if you'd prefer to give the official R+ for these sorts of changes in the future.

[Darin Fisher (:fishd, Google)]

Comment on attachment 103782 [details]
patch

LGTM

[Sadrul Habib Chowdhury]

Comment on attachment 103782 [details]
patch

Thanks! I will CC all of you guys (rjkroege, abarth, fishd) for future CLs.

cq?

[WebKit Review Bot]

Comment on attachment 103782 [details]
patch

Rejecting attachment 103782 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 1

Last 500 characters of output:
1cbc174e811bc674267f4a234e3e0ffc8e6c9bb4
r92989 = 0febe8e5f0a9cdc55493f526abf986a430f43783
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9351724

[Sadrul Habib Chowdhury]

Created attachment 103812 [details]
patch

Looks like the error was caused by a missing 'Reviewed by' line in Tools/ChangeLog? My bad, it got removed when I was editing the changelog file. I have added back that line.

[WebKit Review Bot]

Comment on attachment 103812 [details]
patch

Clearing flags on attachment: 103812

Committed r92997: <http://trac.webkit.org/changeset/92997>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

The test added by this patch is failing on Qt:
http://build.webkit.org/results/Qt%20Linux%20Release/r93002%20(36451)/fast/events/touch/gesture/gesture-click-pretty-diff.html

Why are we adding this test to fast/events/touch if it were to fail on all other platforms but chromium?  Are there bugs filed for each port to implement this DRT feature?

[Adam Barth]

> Why are we adding this test to fast/events/touch if it were to fail on all other platforms but chromium?

The tests are not Chromium-specific.  The fact that they fail on other platforms currently doesn't make them Chromium-specific.

> Are there bugs filed for each port to implement this DRT feature?

The fast/events/touch should be skipped on ports that don't implement Touch events.  If the ports implement Touch events but don't have this DRT feature, then we should file bugs to implement the DRT feature (or just note it in the Skipped list).