Prepare SyntheticPointerAction to handle mouse actions

Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
Committed: https://crrev.com/f536869d57e5ce2e7a5a4b860dcc1e675432face
Cr-Commit-Position: refs/heads/master@{#410124}

[lanwei, 2016-07-25 21:37:05]

Description was changed from

==========
mouse pointer


 test

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

to

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

[lanwei, 2016-07-25 21:37:05]

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

[lanwei, 2016-07-25 21:37:20]

Description was changed from

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

to

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

[lanwei, 2016-07-25 21:37:36]

Description was changed from

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

to

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

[lanwei, 2016-07-25 21:37:56]

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

[commit-bot: I haz the power, 2016-07-25 21:38:29]

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, 2016-07-25 22:38:50]

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

[commit-bot: I haz the power, 2016-07-25 22:38:51]

Dry run: This issue passed the CQ dry run.

[tdresser, 2016-07-26 12:27:42]

https://codereview.chromium.org/2178153002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/synthetic_pointer_action_unittest.cc
(right):

https://codereview.chromium.org/2178153002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/synthetic_pointer_action_unittest.cc:370:
TEST_F(SyntheticPointerActionTest, PointerMouseAction) {
How different are the touch and mouse tests? Could we have parameterized tests
that are the same, and are instantiated once for touch and once for mouse?

See
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
File content/common/input/synthetic_pointer_action_params.cc (left):

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
content/common/input/synthetic_pointer_action_params.cc:14: :
pointer_action_type_(type), index_(-1) {}
Do we ever end up with -1 in the index_map_ now? If not, none of the validation
code will work, will it?

[lanwei, 2016-07-27 22:19:41]

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

[commit-bot: I haz the power, 2016-07-27 22:19:53]

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

[lanwei, 2016-07-27 22:23:15]

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

[commit-bot: I haz the power, 2016-07-27 22:23:29]

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, 2016-07-27 22:43:51]

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

[commit-bot: I haz the power, 2016-07-27 22:43:52]

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

[lanwei, 2016-07-27 23:28:24]

Patchset #2 (id:20001) has been deleted

[lanwei, 2016-07-28 04:47:23]

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
File content/common/input/synthetic_pointer_action_params.cc (left):

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
content/common/input/synthetic_pointer_action_params.cc:14: :
pointer_action_type_(type), index_(-1) {}
On 2016/07/26 12:27:41, tdresser wrote:
> Do we ever end up with -1 in the index_map_ now? If not, none of the
validation
> code will work, will it?

In the constructor, the sourcetype is unknown. How about I always return 0?

[tdresser, 2016-07-28 12:29:55]

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
File content/common/input/synthetic_pointer_action_params.cc (left):

https://codereview.chromium.org/2178153002/diff/1/content/common/input/synthe...
content/common/input/synthetic_pointer_action_params.cc:14: :
pointer_action_type_(type), index_(-1) {}
On 2016/07/28 04:47:23, lanwei wrote:
> On 2016/07/26 12:27:41, tdresser wrote:
> > Do we ever end up with -1 in the index_map_ now? If not, none of the
> validation
> > code will work, will it?
> 
> In the constructor, the sourcetype is unknown. How about I always return 0?

I'm not clear on what your proposal is here. If you always return 0, validation
logic doesn't work, correct? (Don't we have a test that should fail in this
case?)

We could change the variable to be called "touch_index_", or something, and then
have a method called "index()" which, if the source is touch, returns the
touch_index_, and otherwise returns 0.

[lanwei, 2016-08-03 05:53:29]

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

[commit-bot: I haz the power, 2016-08-03 05:53:41]

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, 2016-08-03 06:53:30]

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

[commit-bot: I haz the power, 2016-08-03 06:53:31]

Dry run: This issue passed the CQ dry run.

[tdresser, 2016-08-04 13:46:01]

LGTM

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

https://codereview.chromium.org/2178153002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_action.cc:80: if
(synthetic_pointer_->SourceType() == SyntheticGestureParams::TOUCH_INPUT &&
This should also hold true for mouse, shouldn't it?

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

https://codereview.chromium.org/2178153002/diff/60001/content/common/input/sy...
content/common/input/synthetic_pointer_action_params.h:51: void set_index(int
index) {
Maybe add a comment that the only valid index for mouse is 0.

[samuong, 2016-08-04 18:07:20]

lgtm

[lanwei, 2016-08-05 17:10:30]

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

[commit-bot: I haz the power, 2016-08-05 17:10:53]

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, 2016-08-05 18:02:39]

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

[commit-bot: I haz the power, 2016-08-05 18:02:40]

Dry run: This issue passed the CQ dry run.

[lanwei, 2016-08-05 18:33:00]

The CQ bit was checked by lanwei@chromium.org

[lanwei, 2016-08-05 18:33:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
samuong@chromium.org, tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2178153002/#ps80001
(title: "Add comment")

[commit-bot: I haz the power, 2016-08-05 18:33:19]

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, 2016-08-05 19:06:31]

Description was changed from

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

to

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

[commit-bot: I haz the power, 2016-08-05 19:06:32]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-08-05 19:09:02]

Description was changed from

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187
==========

to

==========
Prepare SyntheticPointerAction to handle mouse actions

This patch is to make SyntheticPointerAction handle mouse actions.

Next patch: Modify SyntheticGestureController to queue all the actions in one
SyntheticPointerAction until we see a PROCESS action, then pushing in a queue
and flush.

BUG=525187

Committed: https://crrev.com/f536869d57e5ce2e7a5a4b860dcc1e675432face
Cr-Commit-Position: refs/heads/master@{#410124}
==========

[commit-bot: I haz the power, 2016-08-05 19:09:04]

Patchset 4 (id:??) landed as
https://crrev.com/f536869d57e5ce2e7a5a4b860dcc1e675432face
Cr-Commit-Position: refs/heads/master@{#410124}

[lanwei, 2016-09-23 15:13:43]

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

https://codereview.chromium.org/2178153002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_action.cc:80: if
(synthetic_pointer_->SourceType() == SyntheticGestureParams::TOUCH_INPUT &&
On 2016/08/04 13:46:01, tdresser wrote:
> This should also hold true for mouse, shouldn't it?

For mouse, the move can be happened without a press first, so we do not need
this check for mouse.

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

https://codereview.chromium.org/2178153002/diff/60001/content/common/input/sy...
content/common/input/synthetic_pointer_action_params.h:51: void set_index(int
index) {
On 2016/08/04 13:46:01, tdresser wrote:
> Maybe add a comment that the only valid index for mouse is 0.

Done.