Bug 85289 - [chromium] Accept four parameters when dispatching a WebInputEvent::GestureTap in chromium DRT eventSender
Summary: [chromium] Accept four parameters when dispatching a WebInputEvent::GestureTa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Terry Anderson
URL:
Keywords:
Depends on: 85314
Blocks: 83947 85101
  Show dependency treegraph
 
Reported: 2012-05-01 09:16 PDT by Terry Anderson
Modified: 2012-05-01 17:20 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.44 KB, patch)
2012-05-01 09:18 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff
Patch (1.54 KB, patch)
2012-05-01 09:44 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff
Patch (1.62 KB, patch)
2012-05-01 16:13 PDT, Terry Anderson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Terry Anderson 2012-05-01 09:16:57 PDT
The parameters deltaX and deltaY will be used in GestureTap to represent the padding around the center of a rectangle used for improved rect-based hit testing.  Need to land this in order to test https://bugs.webkit.org/show_bug.cgi?id=85101.
Comment 1 Terry Anderson 2012-05-01 09:18:35 PDT
Created attachment 139634 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-05-01 09:34:37 PDT
Comment on attachment 139634 [details]
Patch

What tests does this affect?
Comment 3 Terry Anderson 2012-05-01 09:36:24 PDT
(In reply to comment #2)
> (From update of attachment 139634 [details])
> What tests does this affect?

fast/events/touch/gesture (in particular, gesture-click.html)
Comment 4 Eric Seidel (no email) 2012-05-01 09:37:41 PDT
The change is fine, but you should mention which (some general idea) tests this fix in the ChangeLog (and ideally include the test_expectations update in the change.
Comment 5 Eric Seidel (no email) 2012-05-01 09:42:08 PDT
When is this eventSender method needed instead of just generating a synthetic event in the JS?
Comment 6 Eric Seidel (no email) 2012-05-01 09:42:56 PDT
I see, you explain in your bug description why you need this.  I think you should just copy that to your ChangeLog. :)  Many reviewers (myself included) mostly just read the ChangeLog, since that's the part which is actually commited and sticks around for easy git blaming. :)
Comment 7 Terry Anderson 2012-05-01 09:44:50 PDT
Created attachment 139642 [details]
Patch
Comment 8 Eric Seidel (no email) 2012-05-01 09:45:50 PDT
Comment on attachment 139642 [details]
Patch

Thanks.
Comment 9 WebKit Review Bot 2012-05-01 10:18:38 PDT
Comment on attachment 139642 [details]
Patch

Clearing flags on attachment: 139642

Committed r115735: <http://trac.webkit.org/changeset/115735>
Comment 10 WebKit Review Bot 2012-05-01 10:18:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Zhenyao Mo 2012-05-01 14:55:10 PDT
fast/events/touch/page-scaled-touch-gesture-click.html
fast/events/touch/gesture/gesture-click.html

These two tests are crashing on debug bots:

STDOUT: <empty>
STDERR: ASSERTION FAILED: i < size()
STDERR: third_party/WebKit/Source/WTF/wtf/Vector.h(532) : const T& WTF::Vector<T, inlineCapacity>::at(size_t) const [with T = CppVariant, long unsigned int inlineCapacity = 0ul]
STDERR: 1   0x421be5
STDERR: 2   0x42171f
STDERR: 3   0x43ab46
STDERR: 4   0x43a7c1
STDERR: 5   0x43e428
STDERR: 6   0x42b8ca
STDERR: 7   0x42af87
STDERR: 8   0x1338999
STDERR: 9   0x1338b14
STDERR: 10  0xb33c10
STDERR: 11  0xb2eb84
STDERR: 12  0xb2eb55
STDERR: 13  0x1e670530618e
STDERR: [26085:26085:3369300989752:ERROR:process_util_posix.cc(143)] Received signal 11
STDERR: 	base::debug::StackTrace::StackTrace() [0x86cfb2]
STDERR: 	base::(anonymous namespace)::StackDumpSignalHandler() [0x82dbd1]
STDERR: 	0x7fd10cc31af0
STDERR: 	WTF::Vector<>::at() [0x421bef]
STDERR: 	WTF::Vector<>::operator[]() [0x42171f]
STDERR: 	EventSender::gestureEvent() [0x43ab46]
STDERR: 	EventSender::gestureTap() [0x43a7c1]
STDERR: 	CppBoundClass::MemberCallback<>::run() [0x43e428]
STDERR: 	CppBoundClass::invoke() [0x42b8ca]
STDERR: 	CppNPObject::invoke() [0x42af87]
STDERR: 	WebCore::npObjectInvokeImpl() [0x1338999]
STDERR: 	WebCore::npObjectMethodHandler() [0x1338b14]
STDERR: 	v8::internal::HandleApiCallHelper<>() [0xb33c10]
STDERR: 	v8::internal::Builtin_Impl_HandleApiCall() [0xb2eb84]
STDERR: 	v8::internal::Builtin_HandleApiCall() [0xb2eb55]
STDERR: 	0x1e670530618e
Comment 12 Terry Anderson 2012-05-01 16:07:36 PDT
I carelessly forgot to check the size of |arguments|, which caused two tests to crash. The landed patch was rolled out by Zhenyao, so I am about to upload a new one that will not break any tests.
Comment 13 Terry Anderson 2012-05-01 16:13:20 PDT
Created attachment 139699 [details]
Patch
Comment 14 Eric Seidel (no email) 2012-05-01 16:20:26 PDT
Comment on attachment 139699 [details]
Patch

ok.
Comment 15 WebKit Review Bot 2012-05-01 17:20:34 PDT
Comment on attachment 139699 [details]
Patch

Clearing flags on attachment: 139699

Committed r115767: <http://trac.webkit.org/changeset/115767>
Comment 16 WebKit Review Bot 2012-05-01 17:20:39 PDT
All reviewed patches have been landed.  Closing bug.