VR: Sending the fling event to the right target

chrome/browser/android/vr_shell/vr_shell_gl.cc

Index: chrome/browser/android/vr_shell/vr_shell_gl.cc
diff --git a/chrome/browser/android/vr_shell/vr_shell_gl.cc b/chrome/browser/android/vr_shell/vr_shell_gl.cc
index 2f6ca1858794f98ba5b079eec8914441c31f2b4f..5ad90e8d85f3dc053804f9e7f5afa352f186baa1 100644
--- a/chrome/browser/android/vr_shell/vr_shell_gl.cc
+++ b/chrome/browser/android/vr_shell/vr_shell_gl.cc
@@ -653,25 +653,32 @@ void VrShellGl::SendEventsToTarget(InputTarget input_target,
       // Once the user starts scrolling send all the scroll events to this
       // element until the scrolling stops.
       case WebInputEvent::GestureScrollBegin:
-        current_scroll_target = input_target;
-        if (current_scroll_target != InputTarget::NONE) {
-          SendGesture(current_scroll_target, std::move(movableGesture));
+        current_scroll_target_ = input_target;
+        if (current_scroll_target_ != InputTarget::NONE) {
+          SendGesture(current_scroll_target_, std::move(movableGesture));
         }
         break;
       case WebInputEvent::GestureScrollEnd:
-        if (current_scroll_target != InputTarget::NONE) {
-          SendGesture(current_scroll_target, std::move(movableGesture));
+        if (current_scroll_target_ != InputTarget::NONE) {
+          SendGesture(current_scroll_target_, std::move(movableGesture));
         }
-        current_scroll_target = InputTarget::NONE;
+        current_fling_target_ = current_scroll_target_;
+        current_scroll_target_ = InputTarget::NONE;
         break;
       case WebInputEvent::GestureScrollUpdate:
-      case WebInputEvent::GestureFlingCancel:
+        if (current_scroll_target_ != InputTarget::NONE) {
+          SendGesture(current_scroll_target_, std::move(movableGesture));
+        }
+        break;
       case WebInputEvent::GestureFlingStart:
-        if (current_scroll_target != InputTarget::NONE) {
-          SendGesture(current_scroll_target, std::move(movableGesture));
+        if (current_fling_target_ != InputTarget::NONE) {
+          SendGesture(current_fling_target_, std::move(movableGesture));
         }
+        current_fling_target_ = InputTarget::NONE;
         break;
+      case WebInputEvent::GestureFlingCancel:

[mthiesse, 2017/04/12 17:46:20]

Shouldn't FlingCancel do something like sending a cancel gesture to the
current_fling_target_? Why is it falling through to GestureTapDown?

[asimjour, 2017/04/12 18:00:58]

Fling cancel happens when user start to touch the touchpad during a fling. I
think it should only stop the fling if the laser is pointing to the same target
that fling was sent to.

[mthiesse, 2017/04/12 18:05:28]

Sure, but now we're sending a FlingCancel gesture to something that was probably
not in a fling. Just for clarity sake, even if we use the exact same logic in
the FlingCancel and TapDown cases, we shouldn't have it fall through because
they should be they're distinct events that might diverge in behaviour.

[asimjour1, 2017/04/12 18:15:32]

I repeated the code under FlingCancel and TapDown, and made the separate.

It is ok to send FlingCancel to a content that has not received a FlingStart
before.

       case WebInputEvent::GestureTapDown:
+        current_fling_target_ = InputTarget::NONE;
         if (input_target != InputTarget::NONE) {
           SendGesture(input_target, std::move(movableGesture));
         }