Add the pointer type of pen to the synthetic WebMousEvent

Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In order to
simulate actions for pen devices, we will add the pointer type of pen to the synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with pointerActionSequence API.

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

[lanwei, 2017-01-17 00:10:48]

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

[commit-bot: I haz the power, 2017-01-17 00:11:02]

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-17 00:13:57]

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

[commit-bot: I haz the power, 2017-01-17 00:14: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-17 01:02:33]

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

[commit-bot: I haz the power, 2017-01-17 01:02:34]

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-17 01:39:48]

Description was changed from

==========
pen type

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

to

==========
Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In
order to
simulate actions for pen devices, we will add the pointer type of pen to the
synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with
pointerActionSequence.

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

[lanwei, 2017-01-17 01:39:48]

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

[lanwei, 2017-01-17 01:40:48]

Description was changed from

==========
Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In
order to
simulate actions for pen devices, we will add the pointer type of pen to the
synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with
pointerActionSequence.

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

to

==========
Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In
order to
simulate actions for pen devices, we will add the pointer type of pen to the
synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with
pointerActionSequence API.

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

[lanwei, 2017-01-17 01:40:59]

Patchset #1 (id:1) has been deleted

[Navid Zolghadr, 2017-01-19 15:46:22]

There is a little typo in the description. WebMousEvent -> WebMouseEvent

https://codereview.chromium.org/2633233002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_pointer_driver.cc (right):

https://codereview.chromium.org/2633233002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_driver.cc:23: case
SyntheticGestureParams::PEN_INPUT:
Do you think it is worth to introduce the eraser input at the same time? I
believe the idea there was to you use the end of the pen to erase something. So
some pens will report a new type if I recall correctly from what Mustaq was
saying and for some others instead of the end of the pen you need to press the
eraser button. I wonder if we need to support both cases. Basically letting the
user to either set the pointer type to eraser or just set the eraser button
(which I believe we should have already if we just add them to our switch
cases). WDYT?

https://codereview.chromium.org/2633233002/diff/20001/content/common/input/sy...
File content/common/input/synthetic_web_input_event_builders.cc (right):

https://codereview.chromium.org/2633233002/diff/20001/content/common/input/sy...
content/common/input/synthetic_web_input_event_builders.cc:41:
result.pointerType = pointer_type;
Do we expect this to be only mouse and pen? Do you like to have a DCHECK for it?

[lanwei, 2017-01-20 05:49:47]

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

[commit-bot: I haz the power, 2017-01-20 05:50:06]

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-20 06:49:47]

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

[commit-bot: I haz the power, 2017-01-20 06:50: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-20 08:13:11]

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

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

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-20 14:40:44]

Patchset #2 (id:40001) has been deleted

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

https://codereview.chromium.org/2633233002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_pointer_driver.cc (right):

https://codereview.chromium.org/2633233002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_driver.cc:23: case
SyntheticGestureParams::PEN_INPUT:
On 2017/01/19 15:46:22, Navid Zolghadr wrote:
> Do you think it is worth to introduce the eraser input at the same time? I
> believe the idea there was to you use the end of the pen to erase something.
So
> some pens will report a new type if I recall correctly from what Mustaq was
> saying and for some others instead of the end of the pen you need to press the
> eraser button. I wonder if we need to support both cases. Basically letting
the
> user to either set the pointer type to eraser or just set the eraser button
> (which I believe we should have already if we just add them to our switch
> cases). WDYT?

As we discussed offline, we decide to add just pen type right now, thanks!

https://codereview.chromium.org/2633233002/diff/20001/content/common/input/sy...
File content/common/input/synthetic_web_input_event_builders.cc (right):

https://codereview.chromium.org/2633233002/diff/20001/content/common/input/sy...
content/common/input/synthetic_web_input_event_builders.cc:41:
result.pointerType = pointer_type;
On 2017/01/19 15:46:22, Navid Zolghadr wrote:
> Do we expect this to be only mouse and pen? Do you like to have a DCHECK for
it?

Done.

[Navid Zolghadr, 2017-01-20 20:55:53]

lgtm

[lanwei, 2017-01-24 21:11:54]

Tim, could you please take a look. thank you?

[tdresser, 2017-01-25 15:22:46]

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:141: const
blink::WebMouseEvent& web_mouse,
While we're here, let's rename this to web_mouse_event, which fits with our
naming scheme a bit better.

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:16:
gesture_source_type == SyntheticGestureParams::PEN_INPUT);
This DCHECK is redundant with the NOTREACHED at the bottom. I'd get rid of the
DCHECK.

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.h:41:
blink::WebPointerProperties::PointerType pointer_type_;
WebMouseEvent is a WebPointerProperties, which already has a pointer type.

Why do we need this field?

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_driver.cc:24: return
base::MakeUnique<SyntheticMouseDriver>(gesture_source_type);
This feels a bit clumsy - can we make it so there's a 1-1 mapping between
gesture_source_type and synthetic input driver types?

Could we make a SyntheticPenDriver which is a subclass of SyntheticMouseDriver?

[lanwei, 2017-01-25 23:10:57]

Patchset #3 (id:80001) has been deleted

[lanwei, 2017-01-25 23:21:35]

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

[commit-bot: I haz the power, 2017-01-25 23:22:04]

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-25 23:22:28]

Patchset #3 (id:100001) has been deleted

[lanwei, 2017-01-25 23:26:54]

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

[lanwei, 2017-01-25 23:27:21]

Patchset #3 (id:120001) has been deleted

[commit-bot: I haz the power, 2017-01-25 23:27:22]

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-26 02:51:35]

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

[commit-bot: I haz the power, 2017-01-26 02:51:36]

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

[lanwei, 2017-01-26 02:55:41]

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:141: const
blink::WebMouseEvent& web_mouse,
On 2017/01/25 15:22:46, tdresser wrote:
> While we're here, let's rename this to web_mouse_event, which fits with our
> naming scheme a bit better.

Done.

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.cc:16:
gesture_source_type == SyntheticGestureParams::PEN_INPUT);
On 2017/01/25 15:22:46, tdresser wrote:
> This DCHECK is redundant with the NOTREACHED at the bottom. I'd get rid of the
> DCHECK.

Done.

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_mouse_driver.h:41:
blink::WebPointerProperties::PointerType pointer_type_;
On 2017/01/25 15:22:46, tdresser wrote:
> WebMouseEvent is a WebPointerProperties, which already has a pointer type.
> 
> Why do we need this field?
> 

Acknowledged.

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

https://codereview.chromium.org/2633233002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_pointer_driver.cc:24: return
base::MakeUnique<SyntheticMouseDriver>(gesture_source_type);
On 2017/01/25 15:22:46, tdresser wrote:
> This feels a bit clumsy - can we make it so there's a 1-1 mapping between
> gesture_source_type and synthetic input driver types?
> 
> Could we make a SyntheticPenDriver which is a subclass of
SyntheticMouseDriver?

Done.

[tdresser, 2017-01-26 21:03:10]

LGTM, thanks!

https://codereview.chromium.org/2633233002/diff/140001/content/browser/render...
File content/browser/renderer_host/input/synthetic_mouse_driver.h (right):

https://codereview.chromium.org/2633233002/diff/140001/content/browser/render...
content/browser/renderer_host/input/synthetic_mouse_driver.h:38: unsigned
last_modifiers_;
Does last_modifiers_ need to be protected? 

DISALLOW_COPY_AND_ASSIGN at least needs to be part of a private block.

[lanwei, 2017-01-26 21:19:14]

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

[commit-bot: I haz the power, 2017-01-26 21:19:47]

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-26 21:21:11]

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

[lanwei, 2017-01-26 21:21:12]

boliu@chromium.org: Please review changes in 

content/browser/BUILD.gn, thank you

https://codereview.chromium.org/2633233002/diff/140001/content/browser/render...
File content/browser/renderer_host/input/synthetic_mouse_driver.h (right):

https://codereview.chromium.org/2633233002/diff/140001/content/browser/render...
content/browser/renderer_host/input/synthetic_mouse_driver.h:38: unsigned
last_modifiers_;
On 2017/01/26 21:03:10, tdresser wrote:
> Does last_modifiers_ need to be protected? 
> 
> DISALLOW_COPY_AND_ASSIGN at least needs to be part of a private block.

Done.

[boliu, 2017-01-26 21:22:34]

On 2017/01/26 21:21:12, lanwei wrote:
> mailto:boliu@chromium.org: Please review changes in 
> 
> content/browser/BUILD.gn, thank you

rs lgtm

[commit-bot: I haz the power, 2017-01-26 22:39:01]

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

[commit-bot: I haz the power, 2017-01-26 22:39:02]

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-26 22:39:17]

The CQ bit was checked by lanwei@chromium.org

[lanwei, 2017-01-26 22:39:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
nzolghadr@chromium.org, tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2633233002/#ps160001
(title: "Make disallow copy private")

[commit-bot: I haz the power, 2017-01-26 22:40:22]

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-26 22:52:24]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1485470357588920,
"parent_rev": "289eed5c4c8aee75ef6e89c553578b66ec646d0e", "commit_rev":
"d7de40d188cb377011ad81c9b82b814752c9124a"}

[commit-bot: I haz the power, 2017-01-26 22:53:00]

Description was changed from

==========
Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In
order to
simulate actions for pen devices, we will add the pointer type of pen to the
synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with
pointerActionSequence API.

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

to

==========
Add the source type of pen to the synthetic WebMousEvent

SyntheticWebMouseEventBuilder::Build now only supports pointer type of mouse. In
order to
simulate actions for pen devices, we will add the pointer type of pen to the
synthetic WebMousEvent.

The next patch will replace pen actions in pointer event tests with
pointerActionSequence API.

BUG=680330

Review-Url: https://codereview.chromium.org/2633233002
Cr-Commit-Position: refs/heads/master@{#446479}
Committed:
https://chromium.googlesource.com/chromium/src/+/d7de40d188cb377011ad81c9b82b...
==========

[commit-bot: I haz the power, 2017-01-26 22:53:02]

Committed patchset #4 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/d7de40d188cb377011ad81c9b82b...