VR: Stop mouse move events when scrolling/fling

VR: Stop mouse move events when scrolling/fling

When in VR, stop processing mouse move events during scroll and fling.

BUG=688108
Review-Url: https://codereview.chromium.org/2813103003
Cr-Commit-Position: refs/heads/master@{#464520}
Committed: https://chromium.googlesource.com/chromium/src/+/8ee20e2429ecd4448c95d30b8bb96a28c5ee77a7

[asimjour1, 2017-04-12 18:27:51]

asimjour@chromium.org changed reviewers:
+ tdresser@chromium.org

[asimjour1, 2017-04-12 18:27:52]

PTAL

[tdresser, 2017-04-12 19:11:40]

This feels a bit out of place in the RenderWidgetHostImpl.

Could we just filter these events out way earlier in the pipeline? How is fling
plumbed through in this case. Could we suppress mouse events at the
firmware/os/browser level?

[asimjour1, 2017-04-12 19:41:58]

On 2017/04/12 19:11:40, tdresser wrote:
> This feels a bit out of place in the RenderWidgetHostImpl.
> 
> Could we just filter these events out way earlier in the pipeline? How is
fling
> plumbed through in this case. Could we suppress mouse events at the
> firmware/os/browser level?


Fling is generated inside of VrController class and sent by
VrInputManager::ForwardGestureEvent.

We can stop sending mouse event when scrolling, but we don't know anything about
timing of the fling. Normally (when using touchpad) if user wants to move the
cursor during a fling, a FlingCancel will be sent first. But in our case, moving
the controller shouldn't stop fling.

[tdresser, 2017-04-13 14:40:34]

tdresser@chromium.org changed reviewers:
+ dtapuska@chromium.org

[tdresser, 2017-04-13 14:40:35]

Right, makes sense.

Dave, any thoughts on how we could structure this to separate out VR specific
logic?

We could use some kind of injectable filter object, where VR injects a VR
specific filter, but that feels pretty heavier than is really worth it.

[dtapuska, 2017-04-13 14:48:19]

On 2017/04/13 14:40:35, tdresser wrote:
> Right, makes sense.
> 
> Dave, any thoughts on how we could structure this to separate out VR specific
> logic?
> 
> We could use some kind of injectable filter object, where VR injects a VR
> specific filter, but that feels pretty heavier than is really worth it.

Ya this isn't too heavy weight so I think it is ok.

[dtapuska, 2017-04-13 14:48:25]

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1022: if
(GetView()->IsInVR() &&
Can we put a comment here indicating why we are doing this?

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1120: if
(gesture_event.type() == blink::WebInputEvent::GestureFlingStart)
I think this needs a check that the source device is touchpad no?

[asimjour1, 2017-04-13 16:31:36]

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1022: if
(GetView()->IsInVR() &&
On 2017/04/13 14:48:25, dtapuska wrote:
> Can we put a comment here indicating why we are doing this?

Done.

https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.cc:1120: if
(gesture_event.type() == blink::WebInputEvent::GestureFlingStart)
On 2017/04/13 14:48:25, dtapuska wrote:
> I think this needs a check that the source device is touchpad no?

Done.

[dtapuska, 2017-04-13 16:46:26]

On 2017/04/13 16:31:36, asimjour1 wrote:
>
https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
> File content/browser/renderer_host/render_widget_host_impl.cc (right):
> 
>
https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
> content/browser/renderer_host/render_widget_host_impl.cc:1022: if
> (GetView()->IsInVR() &&
> On 2017/04/13 14:48:25, dtapuska wrote:
> > Can we put a comment here indicating why we are doing this?
> 
> Done.
> 
>
https://codereview.chromium.org/2813103003/diff/1/content/browser/renderer_ho...
> content/browser/renderer_host/render_widget_host_impl.cc:1120: if
> (gesture_event.type() == blink::WebInputEvent::GestureFlingStart)
> On 2017/04/13 14:48:25, dtapuska wrote:
> > I think this needs a check that the source device is touchpad no?
> 
> Done.

lgtm

[tdresser, 2017-04-13 17:08:14]

LGTM

[asimjour, 2017-04-13 19:15:52]

The CQ bit was checked by asimjour@google.com

[asimjour, 2017-04-13 19:16:04]

The CQ bit was unchecked by asimjour@google.com

[asimjour1, 2017-04-13 19:16:15]

The CQ bit was checked by asimjour@chromium.org

[commit-bot: I haz the power, 2017-04-13 19:17:03]

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-04-13 20:15:48]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492110975290140,
"parent_rev": "546e184b881ef71c27b50a4e0b571a6b479e172d", "commit_rev":
"8ee20e2429ecd4448c95d30b8bb96a28c5ee77a7"}

[commit-bot: I haz the power, 2017-04-13 20:16:41]

Description was changed from

==========
VR: Stop mouse move events when scrolling/fling

When in VR, stop processing mouse move events during scroll and fling.

BUG=688108
==========

to

==========
VR: Stop mouse move events when scrolling/fling

When in VR, stop processing mouse move events during scroll and fling.

BUG=688108

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

[commit-bot: I haz the power, 2017-04-13 20:16:42]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8ee20e2429ecd4448c95d30b8bb9...