Replace mouse actions in pointer event tests with pointerActionSequence

Replace mouse actions in pointer event tests with pointerActionSequence

Replace mouseMove, mouseClick and mouseDrag in pointerevent_common_input.js
with pointerActionSequence.

BUG=680330
Review-Url: https://codereview.chromium.org/2621353003
Cr-Commit-Position: refs/heads/master@{#445052}
Committed: https://chromium.googlesource.com/chromium/src/+/8d7393562f8ce8900cdd642ea0b78eafde767b61

[lanwei, 2017-01-12 00:05:36]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 00:06:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-12 00:20:04]

Description was changed from

==========
mouse test

BUG=
==========

to

==========
Replace mouse actions in pointer event tests with pointerActionSequence

Replace mouseMove, mouseClick and mouseDrag in pointerevent_common_input.js
with pointerActionSequence.

BUG=680330
==========

[lanwei, 2017-01-12 00:20:05]

lanwei@chromium.org changed reviewers:
+ nzolghadr@chromium.org, tdresser@chromium.org

[lanwei, 2017-01-12 00:49:05]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 00:50:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 02:35:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 02:35:06]

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-12 05:49:37]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 05:49:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-12 06:04:34]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[lanwei, 2017-01-12 06:05:13]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-01-12 06:05:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-12 06:05:21]

Patchset #3 (id:40001) has been deleted

[lanwei, 2017-01-12 06:07:50]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-01-12 07:36:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 07:36:10]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-01-12 18:23:46]

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_mouse_driver.cc (right):

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:15: case 0:
Do we ever hit this case? Should we ever have a case that we call this function
and the value of the button is not any of left/middle/right?

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:17: case 1:
Can we just use the same numbers as already existing Button enum class for
these? We have -1 as no button and 0 as left and so on.
In that case, the initial value of the button in other parts of this patch will
be zero I believe.

https://codereview.chromium.org/2621353003/diff/60001/content/common/input/sy...
File content/common/input/synthetic_pointer_action_params.h (right):

https://codereview.chromium.org/2621353003/diff/60001/content/common/input/sy...
content/common/input/synthetic_pointer_action_params.h:80: DCHECK_LT(button_,
3);
Shouldn't this be LE with the current code? Although if you changed the button
values from [0,3] to [0,2]. This will be Alright.

https://codereview.chromium.org/2621353003/diff/60001/content/renderer/gpu/ac...
File content/renderer/gpu/actions_parser.cc (right):

https://codereview.chromium.org/2621353003/diff/60001/content/renderer/gpu/ac...
content/renderer/gpu/actions_parser.cc:211: action_param.set_button(button);
Is there any reason we don't set the x/y here similar to press?

https://codereview.chromium.org/2621353003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2621353003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:163:
function touchActionMouseScroll(targetSelector) {
Does pointerevent_touch-action-mouse-manual pass using this function pass on all
the platforms? I remember I needed to click in the box so the box gets the focus
and the wheel scroll applies to the box and not to the page. 
Also this function seems very single purpose only for that test as it scrolls in
both direction. I believe still having two functions scrolling in different
directions or just a parameter specifying that will be more useful. Then each
test can decide to how to use them.

[lanwei, 2017-01-12 22:32:28]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 22:33:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-12 22:51:00]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 22:52:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 23:36:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 23:36:01]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[lanwei, 2017-01-13 01:47:01]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 01:47:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-13 01:47:57]

Patchset #4 (id:90001) has been deleted

[commit-bot: I haz the power, 2017-01-13 01:51:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 01:51:09]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[lanwei, 2017-01-13 02:04:38]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[lanwei, 2017-01-13 02:05:02]

Patchset #4 (id:110001) has been deleted

[commit-bot: I haz the power, 2017-01-13 02:05:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 02:21:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 02:21:39]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[lanwei, 2017-01-13 03:21:10]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[lanwei, 2017-01-13 03:21:43]

Patchset #4 (id:130001) has been deleted

[commit-bot: I haz the power, 2017-01-13 03:22:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 04:56:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 04:56:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lanwei, 2017-01-13 05:28:01]

Patchset #3 (id:50016) has been deleted

[lanwei, 2017-01-13 16:36:46]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 16:37:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-13 16:39:05]

Patchset #3 (id:150001) has been deleted

[lanwei, 2017-01-13 17:29:30]

Navid, can you please take another look, thanks?

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_mouse_driver.cc (right):

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:15: case 0:
On 2017/01/12 18:23:46, Navid Zolghadr wrote:
> Do we ever hit this case? Should we ever have a case that we call this
function
> and the value of the button is not any of left/middle/right?

No, I just list all the case. Now, I will just list all the possible buttons.

https://codereview.chromium.org/2621353003/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:17: case 1:
On 2017/01/12 18:23:46, Navid Zolghadr wrote:
> Can we just use the same numbers as already existing Button enum class for
> these? We have -1 as no button and 0 as left and so on.
> In that case, the initial value of the button in other parts of this patch
will
> be zero I believe.

Done.

https://codereview.chromium.org/2621353003/diff/60001/content/common/input/sy...
File content/common/input/synthetic_pointer_action_params.h (right):

https://codereview.chromium.org/2621353003/diff/60001/content/common/input/sy...
content/common/input/synthetic_pointer_action_params.h:80: DCHECK_LT(button_,
3);
On 2017/01/12 18:23:46, Navid Zolghadr wrote:
> Shouldn't this be LE with the current code? Although if you changed the button
> values from [0,3] to [0,2]. This will be Alright. 

Done.

https://codereview.chromium.org/2621353003/diff/60001/content/renderer/gpu/ac...
File content/renderer/gpu/actions_parser.cc (right):

https://codereview.chromium.org/2621353003/diff/60001/content/renderer/gpu/ac...
content/renderer/gpu/actions_parser.cc:211: action_param.set_button(button);
On 2017/01/12 18:23:46, Navid Zolghadr wrote:
> Is there any reason we don't set the x/y here similar to press?

For SyntheticTouchDriver::Release and SyntheticMouseDriver::Release, we do not
need x/y values to dispatch a release event, and we do not request user to put
x/y for RELEASE action.

https://codereview.chromium.org/2621353003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2621353003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imported/wpt_automation/pointerevents/pointerevent_common_input.js:163:
function touchActionMouseScroll(targetSelector) {
On 2017/01/12 18:23:46, Navid Zolghadr wrote:
> Does pointerevent_touch-action-mouse-manual pass using this function pass on
all
> the platforms? I remember I needed to click in the box so the box gets the
focus
> and the wheel scroll applies to the box and not to the page. 
> Also this function seems very single purpose only for that test as it scrolls
in
> both direction. I believe still having two functions scrolling in different
> directions or just a parameter specifying that will be more useful. Then each
> test can decide to how to use them.

Yes, the test is passed on all platforms, but I added a click action to make
sure scrolling happen.

[commit-bot: I haz the power, 2017-01-13 18:28:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 18:28:13]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-01-16 15:51:09]

lgtm

[tdresser, 2017-01-16 15:56:20]

I'd prefer splitting this patch in two - one patch for the C++ changes, and one
patch for the LayoutTests changes.

That means that if the layout tests break, you won't need to revert the whole
patch.

[lanwei, 2017-01-19 02:43:15]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 02:43:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-19 04:09:35]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 04:10:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-19 04:10:20]

Patchset #3 (id:170001) has been deleted

[lanwei, 2017-01-19 04:10:47]

Patchset #3 (id:190001) has been deleted

[lanwei, 2017-01-19 04:11:01]

Patchset #3 (id:210001) has been deleted

[lanwei, 2017-01-19 05:22:52]

lanwei@chromium.org changed reviewers:
+ jochen@chromium.org

[commit-bot: I haz the power, 2017-01-19 05:44:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 05:44:59]

Dry run: This issue passed the CQ dry run.

[tdresser, 2017-01-19 14:41:57]

https://codereview.chromium.org/2621353003/diff/230001/content/renderer/gpu/a...
File content/renderer/gpu/actions_parser.cc (right):

https://codereview.chromium.org/2621353003/diff/230001/content/renderer/gpu/a...
content/renderer/gpu/actions_parser.cc:46: else
Let's be explicit here, DCHECK if the string isn't correct.

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:97:
scrollPageIfNeeded(targetSelector, targetDocument);
Interesting - so we need to scroll pages to click things offscreen, whereas
before we didn't need to?

This is likely to break stuff, isn't it?

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:136:
chrome.gpuBenchmarking.pointerActionSequence(JSON.parse(text), resolve);
Can't we build up the json as a JS object, not a string?

[lanwei, 2017-01-19 18:32:06]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 18:32:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-19 18:35:57]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[lanwei, 2017-01-19 18:36:11]

Patchset #4 (id:250001) has been deleted

[commit-bot: I haz the power, 2017-01-19 18:36:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-19 19:32:53]

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js
(right):

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:97:
scrollPageIfNeeded(targetSelector, targetDocument);
On 2017/01/19 14:41:57, tdresser wrote:
> Interesting - so we need to scroll pages to click things offscreen, whereas
> before we didn't need to?
> 
> This is likely to break stuff, isn't it?

We always use this function to make sure we can click on the position we want,
even when the area is not on the screen. So far it does not break anything, and
I confirmed with Rick when I first used this in the tests.
https://codesearch.chromium.org/chromium/src/third_party/WebKit/LayoutTests/e...

https://codereview.chromium.org/2621353003/diff/230001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt_automation/pointerevents/pointerevent_common_input.js:136:
chrome.gpuBenchmarking.pointerActionSequence(JSON.parse(text), resolve);
On 2017/01/19 14:41:57, tdresser wrote:
> Can't we build up the json as a JS object, not a string?

Done.

[tdresser, 2017-01-19 19:35:13]

LGTM

[commit-bot: I haz the power, 2017-01-19 20:05:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 20:05:52]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2017-01-20 07:37:36]

lgtm

[lanwei, 2017-01-20 14:42:36]

The CQ bit was checked by lanwei@chromium.org

[lanwei, 2017-01-20 14:42:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2621353003/#ps270001
(title: "pointer mouse event")

[commit-bot: I haz the power, 2017-01-20 14:42:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 14:46:11]

CQ is committing da patch.

Bot data: {"patchset_id": 270001, "attempt_start_ts": 1484923356821760,
"parent_rev": "0e2de9dbb7a8507438b24f2a70be000ccb7a8265", "commit_rev":
"8d7393562f8ce8900cdd642ea0b78eafde767b61"}

[commit-bot: I haz the power, 2017-01-20 14:46:44]

Description was changed from

==========
Replace mouse actions in pointer event tests with pointerActionSequence

Replace mouseMove, mouseClick and mouseDrag in pointerevent_common_input.js
with pointerActionSequence.

BUG=680330
==========

to

==========
Replace mouse actions in pointer event tests with pointerActionSequence

Replace mouseMove, mouseClick and mouseDrag in pointerevent_common_input.js
with pointerActionSequence.

BUG=680330

Review-Url: https://codereview.chromium.org/2621353003
Cr-Commit-Position: refs/heads/master@{#445052}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d7393562f8ce8900cdd642ea0b7...
==========

[commit-bot: I haz the power, 2017-01-20 14:46:48]

Committed patchset #4 (id:270001) as
https://chromium.googlesource.com/chromium/src/+/8d7393562f8ce8900cdd642ea0b7...