Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent in
DesktopWindowTreeHostX11::DispatchEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

ET_MOUSE_MOVED events created from an EnterNotify are marked as
synthesized elsewhere, so we move this logic into
EventFlagsFromXEvent, so the events are consistently marked as
synthesized.

BUG=731914
Review-Url: https://codereview.chromium.org/2933353003
Cr-Commit-Position: refs/heads/master@{#484279}
Committed: https://chromium.googlesource.com/chromium/src/+/08381b6e5c3fc97d80e116f770d4fe0f1c06e0e5

[Kevin McNee, 2017-06-13 16:09:16]

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

[commit-bot: I haz the power, 2017-06-13 16:09:33]

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-06-13 17:04:07]

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

[commit-bot: I haz the power, 2017-06-13 17:04:08]

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

[Kevin McNee, 2017-06-13 17:08:32]

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

[commit-bot: I haz the power, 2017-06-13 17:08:40]

Dry run: CQ is trying da patch.

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

[Kevin McNee, 2017-06-13 20:15:58]

mcnee@chromium.org changed reviewers:
+ sadrul@chromium.org

[Kevin McNee, 2017-06-13 20:15:59]

sadrul: Could you take a look at this change? Thanks.

[commit-bot: I haz the power, 2017-06-13 20:31:50]

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

[commit-bot: I haz the power, 2017-06-13 20:31:51]

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

[Kevin McNee, 2017-06-13 22:06:02]

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

[commit-bot: I haz the power, 2017-06-13 22:06:33]

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-06-14 01:10:54]

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

[commit-bot: I haz the power, 2017-06-14 01:10:55]

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

[Kevin McNee, 2017-06-15 16:22:20]

Description was changed from

==========
Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

We now mark this ET_MOUSE_MOVED created from an EnterNotify as
synthesized as is done elsewhere.

BUG=731914
==========

to

==========
DO NOT COMMIT

Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

We now mark this ET_MOUSE_MOVED created from an EnterNotify as
synthesized as is done elsewhere.

BUG=731914
==========

[sadrul, 2017-06-16 16:03:36]

https://codereview.chromium.org/2933353003/diff/90001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/2933353003/diff/90001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1924:
mouse_event.set_flags(mouse_event.flags() | ui::EF_IS_SYNTHESIZED);
Looks like we are always doing this [1, 2]. Can we instead update [3] so we
don't need to explicitly do this?

[1]
https://cs.chromium.org/chromium/src/ui/platform_window/x11/x11_window.cc?l=94
[2]
https://cs.chromium.org/chromium/src/ui/events/platform/x11/x11_event_source_...
[3] https://cs.chromium.org/chromium/src/ui/events/x/events_x_utils.cc?l=478

[Kevin McNee, 2017-06-28 21:34:03]

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

[commit-bot: I haz the power, 2017-06-28 21:34:37]

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-06-28 22:59:00]

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

[commit-bot: I haz the power, 2017-06-28 22:59:01]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-06-29 22:30:17]

Description was changed from

==========
DO NOT COMMIT

Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

We now mark this ET_MOUSE_MOVED created from an EnterNotify as
synthesized as is done elsewhere.

BUG=731914
==========

to

==========
Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent in
DesktopWindowTreeHostX11::DispatchEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

ET_MOUSE_MOVED events created from an EnterNotify are marked as
synthesized elsewhere, so we move this logic into
EventFlagsFromXEvent, so the events are consistently marked as
synthesized.

BUG=731914
==========

[Kevin McNee, 2017-06-29 22:31:11]

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

[commit-bot: I haz the power, 2017-06-29 22:31:41]

Dry run: CQ is trying da patch.

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

[Kevin McNee, 2017-06-29 22:41:42]

The change in ui/aura/env_input_state_controller.cc addresses the UAF.

https://codereview.chromium.org/2933353003/diff/90001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/2933353003/diff/90001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1924:
mouse_event.set_flags(mouse_event.flags() | ui::EF_IS_SYNTHESIZED);
On 2017/06/16 16:03:36, sadrul wrote:
> Looks like we are always doing this [1, 2]. Can we instead update [3] so we
> don't need to explicitly do this?
> 
> [1]
> https://cs.chromium.org/chromium/src/ui/platform_window/x11/x11_window.cc?l=94
> [2]
>
https://cs.chromium.org/chromium/src/ui/events/platform/x11/x11_event_source_...
> [3] https://cs.chromium.org/chromium/src/ui/events/x/events_x_utils.cc?l=478

Done.

[commit-bot: I haz the power, 2017-06-29 23:40:10]

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

[commit-bot: I haz the power, 2017-06-29 23:40:11]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-06-30 19:20:06]

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

[commit-bot: I haz the power, 2017-06-30 19:20:34]

Dry run: CQ is trying da patch.

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

[Kevin McNee, 2017-06-30 19:27:18]

Also, good news. This appears to fix
WebContentsViewAuraTest.QuickOverscrollDirectionChange on linux. I'm seeing the
same spurious mouse move in that test which this CL now drops. I'll follow up
with a CL to re-enable it.

[commit-bot: I haz the power, 2017-06-30 20:31:32]

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

[commit-bot: I haz the power, 2017-06-30 20:31:33]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-07-04 17:16:49]

lgtm

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
File ui/aura/env_input_state_controller.cc (right):

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
ui/aura/env_input_state_controller.cc:46: #endif  // defined(USE_X11)
Can you just set the mouse location without having to look at the native event
type? Alternatively, can you change the condition above in line 33:34 to
something like:

  if (event.type() != CAPTURE_CHANGED &&
      (!synthesized || has_native_event)) {
    SetLastMouseLocation(...);
  }

Add a comment that if an event is triggered by a native event, that probably
means we should actually take that location.

[Kevin McNee, 2017-07-04 22:42:42]

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

[commit-bot: I haz the power, 2017-07-04 22:42:45]

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-07-04 23:45:40]

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

[commit-bot: I haz the power, 2017-07-04 23:45:41]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-07-05 13:52:28]

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
File ui/aura/env_input_state_controller.cc (right):

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
ui/aura/env_input_state_controller.cc:46: #endif  // defined(USE_X11)
On 2017/07/04 17:16:49, sadrul wrote:
> Can you just set the mouse location without having to look at the native event
> type? Alternatively, can you change the condition above in line 33:34 to
> something like:
> 
>   if (event.type() != CAPTURE_CHANGED &&
>       (!synthesized || has_native_event)) {
>     SetLastMouseLocation(...);
>   }
> 
> Add a comment that if an event is triggered by a native event, that probably
> means we should actually take that location.

Done.

Although, I think I could've worded that comment better. I'll clarify it before
committing.

[Kevin McNee, 2017-07-05 14:08:46]

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
File ui/aura/env_input_state_controller.cc (right):

https://codereview.chromium.org/2933353003/diff/190001/ui/aura/env_input_stat...
ui/aura/env_input_state_controller.cc:46: #endif  // defined(USE_X11)
On 2017/07/05 13:52:27, Kevin McNee wrote:
> On 2017/07/04 17:16:49, sadrul wrote:
> > Can you just set the mouse location without having to look at the native
event
> > type? Alternatively, can you change the condition above in line 33:34 to
> > something like:
> > 
> >   if (event.type() != CAPTURE_CHANGED &&
> >       (!synthesized || has_native_event)) {
> >     SetLastMouseLocation(...);
> >   }
> > 
> > Add a comment that if an event is triggered by a native event, that probably
> > means we should actually take that location.
> 
> Done.
> 
> Although, I think I could've worded that comment better. I'll clarify it
before
> committing.

Done.

[Kevin McNee, 2017-07-05 14:11:03]

The CQ bit was checked by mcnee@chromium.org

[Kevin McNee, 2017-07-05 14:11:04]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2933353003/#ps230001
(title: "Clarify comment.")

[commit-bot: I haz the power, 2017-07-05 14:11: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-07-05 15:23:17]

CQ is committing da patch.

Bot data: {"patchset_id": 230001, "attempt_start_ts": 1499263863689310,
"parent_rev": "1d1b41b50ba5cdb8b3c5739c1c9193c466cb5cf9", "commit_rev":
"08381b6e5c3fc97d80e116f770d4fe0f1c06e0e5"}

[commit-bot: I haz the power, 2017-07-05 15:23:32]

Description was changed from

==========
Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent in
DesktopWindowTreeHostX11::DispatchEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

ET_MOUSE_MOVED events created from an EnterNotify are marked as
synthesized elsewhere, so we move this logic into
EventFlagsFromXEvent, so the events are consistently marked as
synthesized.

BUG=731914
==========

to

==========
Mark the ET_MOUSE_MOVED created from a pointer grab as synthesized.

When the mouse pointer is outside of the window and an overscroll
navigation is started with a touchscreen, an ET_MOUSE_MOVED event
is created from the EnterNotify XEvent in
DesktopWindowTreeHostX11::DispatchEvent. This is not marked as
synthesized, so it interferes with the overscroll gesture.

ET_MOUSE_MOVED events created from an EnterNotify are marked as
synthesized elsewhere, so we move this logic into
EventFlagsFromXEvent, so the events are consistently marked as
synthesized.

BUG=731914

Review-Url: https://codereview.chromium.org/2933353003
Cr-Commit-Position: refs/heads/master@{#484279}
Committed:
https://chromium.googlesource.com/chromium/src/+/08381b6e5c3fc97d80e116f770d4...
==========

[commit-bot: I haz the power, 2017-07-05 15:23:33]

Committed patchset #13 (id:230001) as
https://chromium.googlesource.com/chromium/src/+/08381b6e5c3fc97d80e116f770d4...