Bug 66105 - [chromium] Add support for gesture events in the DRT
Summary: [chromium] Add support for gesture events in the DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-11 15:27 PDT by Sadrul Habib Chowdhury
Modified: 2011-08-12 15:50 PDT (History)
5 users (show)

See Also:


Attachments
patch (3.44 KB, patch)
2011-08-11 15:32 PDT, Sadrul Habib Chowdhury
no flags Details | Formatted Diff | Diff
patch (11.26 KB, patch)
2011-08-12 10:52 PDT, Sadrul Habib Chowdhury
abarth: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
patch (11.30 KB, patch)
2011-08-12 13:56 PDT, Sadrul Habib Chowdhury
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sadrul Habib Chowdhury 2011-08-11 15:27:54 PDT
This patch adds support for gesture events in the DumpRenderTree. This will allow testing gesture recognition code and gesture handling code separately.
Comment 1 Sadrul Habib Chowdhury 2011-08-11 15:32:43 PDT
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.
Comment 2 Robert Kroeger 2011-08-12 07:36:38 PDT
(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.
Comment 3 Sadrul Habib Chowdhury 2011-08-12 10:52:42 PDT
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).
Comment 4 Robert Kroeger 2011-08-12 11:21:49 PDT
Looks reasonable to me. (Though it will probably have to change to track changes to PlatformGestureEvent.)
Comment 5 Adam Barth 2011-08-12 12:26:04 PDT
Comment on attachment 103782 [details]
patch

Technically we need fishd to approve the WebKit API change, but I think this is probably ok.
Comment 6 Adam Barth 2011-08-12 12:27:01 PDT
+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.
Comment 7 Darin Fisher (:fishd, Google) 2011-08-12 12:34:19 PDT
Comment on attachment 103782 [details]
patch

LGTM
Comment 8 Sadrul Habib Chowdhury 2011-08-12 12:47:04 PDT
Comment on attachment 103782 [details]
patch

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

cq?
Comment 9 WebKit Review Bot 2011-08-12 13:47:57 PDT
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
Comment 10 Sadrul Habib Chowdhury 2011-08-12 13:56:30 PDT
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.
Comment 11 WebKit Review Bot 2011-08-12 14:28:50 PDT
Comment on attachment 103812 [details]
patch

Clearing flags on attachment: 103812

Committed r92997: <http://trac.webkit.org/changeset/92997>
Comment 12 WebKit Review Bot 2011-08-12 14:28:55 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Ryosuke Niwa 2011-08-12 15:33:26 PDT
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?
Comment 14 Adam Barth 2011-08-12 15:50:01 PDT
> 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).