Recompute PositionInWidget for each wheel or touchpad gesture event

This issue is because we use a fixed transform to compute PositionInWidget
for each wheel or touchpad gesture event. If the OOPIF scrolled, use of the
fixed transform would give wrong coordinates.

In this patch, we recompute all scroll event's PositionInWidget in scroll
sequence.

This patch only changes wheel and touchpad gesture event. Touch and gesture
events be fixed in a fix in following patch.

Bug: 923560
Change-Id: I557d65e756f0933f2efbc26802179b3531c03826
Reviewed-on: https://chromium-review.googlesource.com/c/1427607Reviewed-by: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628228}

content/browser/renderer_host/render_widget_host_input_event_router.cc

@@ -264,8 +264,8 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
   }
   touch_event_ack_queue_->UpdateQueueAfterTargetDestroyed(view);
 
-  if (view == wheel_target_.target)
-    wheel_target_.target = nullptr;
+  if (view == wheel_target_)
+    wheel_target_ = nullptr;
 
   // If the target that's being destroyed is in the gesture target map, we
   // replace it with nullptr so that we maintain the 1:1 correspondence between
@@ -281,8 +281,8 @@ void RenderWidgetHostInputEventRouter::OnRenderWidgetHostViewBaseDestroyed(
   if (view == touchscreen_gesture_target_.target)
     touchscreen_gesture_target_.target = nullptr;
 
-  if (view == touchpad_gesture_target_.target)
-    touchpad_gesture_target_.target = nullptr;
+  if (view == touchpad_gesture_target_)
+    touchpad_gesture_target_ = nullptr;
 
   if (view == bubbling_gesture_scroll_target_) {
     bubbling_gesture_scroll_target_ = nullptr;
@@ -586,20 +586,13 @@ void RenderWidgetHostInputEventRouter::DispatchMouseWheelEvent(
     const blink::WebMouseWheelEvent& mouse_wheel_event,
     const ui::LatencyInfo& latency,
     const base::Optional<gfx::PointF>& target_location) {
-  base::Optional<gfx::PointF> point_in_target = target_location;
   if (!root_view->IsMouseLocked()) {
     if (mouse_wheel_event.phase == blink::WebMouseWheelEvent::kPhaseBegan) {
-      wheel_target_.target = target;
-      if (target_location.has_value()) {
-        wheel_target_.delta =
-            target_location.value() - mouse_wheel_event.PositionInWidget();
-      }
+      wheel_target_ = target;
     } else {
-      if (wheel_target_.target) {
-        DCHECK(!target && !target_location.has_value());
-        target = wheel_target_.target;
-        point_in_target.emplace(mouse_wheel_event.PositionInWidget() +
-                                wheel_target_.delta);
+      if (wheel_target_) {
+        DCHECK(!target);
+        target = wheel_target_;
       } else if ((mouse_wheel_event.phase ==
                       blink::WebMouseWheelEvent::kPhaseEnded ||
                   mouse_wheel_event.momentum_phase ==
@@ -618,21 +611,22 @@ void RenderWidgetHostInputEventRouter::DispatchMouseWheelEvent(
                              INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
     return;
   }
-  // If target_location doesn't have a value, it can be for two reasons:
-  // 1. |target| is null, in which case we would have early returned from the
-  // check above.
-  // 2. The event we are receiving is not a phaseBegan, in which case we should
-  // have got a valid |point_in_target| from wheel_target_.delta above.
-  DCHECK(point_in_target.has_value());
 
   blink::WebMouseWheelEvent event = mouse_wheel_event;
-  event.SetPositionInWidget(point_in_target->x(), point_in_target->y());
+  gfx::PointF point_in_target;
+  if (target_location) {
+    point_in_target = target_location.value();
+  } else {
+    point_in_target = target->TransformRootPointToViewCoordSpace(
+        mouse_wheel_event.PositionInWidget());
+  }
+  event.SetPositionInWidget(point_in_target.x(), point_in_target.y());
   target->ProcessMouseWheelEvent(event, latency);
 
   if (mouse_wheel_event.phase == blink::WebMouseWheelEvent::kPhaseEnded ||
       mouse_wheel_event.momentum_phase ==
           blink::WebMouseWheelEvent::kPhaseEnded) {
-    wheel_target_.target = nullptr;
+    wheel_target_ = nullptr;
   }
 }
 
@@ -1023,7 +1017,7 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
     // should inform the child view, so that it does not go on to send us
     // the updates. See https://crbug.com/828422
     if (target_view == touchscreen_gesture_target_.target ||
-        target_view == touchpad_gesture_target_.target ||
+        target_view == touchpad_gesture_target_ ||
         target_view == touch_target_.target) {
       return;
     }
@@ -1524,12 +1518,14 @@ void RenderWidgetHostInputEventRouter::DispatchTouchpadGestureEvent(
   // of routing.
   if (touchpad_gesture_event.GetType() ==
       blink::WebInputEvent::kGestureFlingStart) {
-    if (wheel_target_.target) {
+    if (wheel_target_) {
       blink::WebGestureEvent gesture_fling = touchpad_gesture_event;
-      gesture_fling.SetPositionInWidget(gesture_fling.PositionInWidget() +
-                                        wheel_target_.delta);
-      wheel_target_.target->ProcessGestureEvent(gesture_fling, latency);
-      last_fling_start_target_ = wheel_target_.target;
+      gfx::PointF point_in_target =
+          wheel_target_->TransformRootPointToViewCoordSpace(
+              gesture_fling.PositionInWidget());
+      gesture_fling.SetPositionInWidget(point_in_target);
+      wheel_target_->ProcessGestureEvent(gesture_fling, latency);
+      last_fling_start_target_ = wheel_target_;
     } else {
       root_view->GestureEventAck(touchpad_gesture_event,
                                  INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
@@ -1552,37 +1548,35 @@ void RenderWidgetHostInputEventRouter::DispatchTouchpadGestureEvent(
   }
 
   if (target) {
-    touchpad_gesture_target_.target = target;
-    // TODO(mohsen): Instead of just computing a delta, we should extract the
-    // complete transform. We assume it doesn't change for the duration of the
-    // touchpad gesture sequence, though this could be wrong; a better approach
-    // might be to always transform each point to the
-    // |touchpad_gesture_target_.target| for the duration of the sequence.
-    DCHECK(target_location.has_value());
-    touchpad_gesture_target_.delta =
-        target_location.value() - touchpad_gesture_event.PositionInWidget();
+    touchpad_gesture_target_ = target;
 
     // Abort any scroll bubbling in progress to avoid double entry.
-    CancelScrollBubblingIfConflicting(touchpad_gesture_target_.target);
+    CancelScrollBubblingIfConflicting(touchpad_gesture_target_);
   }
 
-  if (!touchpad_gesture_target_.target) {
+  if (!touchpad_gesture_target_) {
     root_view->GestureEventAck(touchpad_gesture_event,
                                INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
     return;
   }
 
   blink::WebGestureEvent gesture_event = touchpad_gesture_event;
-  // TODO(mohsen): Add tests to check event location.
-  gesture_event.SetPositionInWidget(gesture_event.PositionInWidget() +
-                                    touchpad_gesture_target_.delta);
-  touchpad_gesture_target_.target->ProcessGestureEvent(gesture_event, latency);
+  gfx::PointF point_in_target;
+  if (target_location) {
+    point_in_target = target_location.value();
+  } else {
+    point_in_target =
+        touchpad_gesture_target_->TransformRootPointToViewCoordSpace(
+            gesture_event.PositionInWidget());
+  }
+  gesture_event.SetPositionInWidget(point_in_target);
+  touchpad_gesture_target_->ProcessGestureEvent(gesture_event, latency);
 
   if (touchpad_gesture_event.GetType() ==
           blink::WebInputEvent::kGesturePinchEnd ||
       touchpad_gesture_event.GetType() ==
           blink::WebInputEvent::kGestureDoubleTap) {
-    touchpad_gesture_target_.target = nullptr;
+    touchpad_gesture_target_ = nullptr;
   }
 }
 

content/browser/renderer_host/render_widget_host_input_event_router.h

@@ -189,7 +189,6 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
                                                  viz::FrameSinkIdHash>;
   struct TargetData {
     RenderWidgetHostViewBase* target;
-    gfx::Vector2dF delta;
     gfx::Transform transform;
 
     TargetData() : target(nullptr) {}
@@ -331,11 +330,11 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
   // The following variable is temporary, for diagnosis of
   // https://crbug.com/824774.
   bool touchscreen_gesture_target_in_map_;
-  TargetData touchpad_gesture_target_;
+  RenderWidgetHostViewBase* touchpad_gesture_target_ = nullptr;
   RenderWidgetHostViewBase* bubbling_gesture_scroll_target_ = nullptr;
   RenderWidgetHostViewChildFrame* bubbling_gesture_scroll_origin_ = nullptr;
   // Used to target wheel events for the duration of a scroll.
-  TargetData wheel_target_;
+  RenderWidgetHostViewBase* wheel_target_ = nullptr;
   // Maintains the same target between mouse down and mouse up.
   TargetData mouse_capture_target_;
 

content/browser/site_per_process_hit_test_browsertest.cc

@@ -3539,7 +3539,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessMouseWheelHitTestBrowserTest,
   SendMouseWheel(pos);
   waiter.Wait();
 
-  EXPECT_EQ(child_rwhv, router->wheel_target_.target);
+  EXPECT_EQ(child_rwhv, router->wheel_target_);
 
   // Send a mouse wheel event to the main frame. It will be still routed to
   // child till the end of current scrolling sequence. Since wheel scroll
@@ -3547,7 +3547,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessMouseWheelHitTestBrowserTest,
   // InputEventAckWaiter is not needed here.
   TestInputEventObserver child_frame_monitor(child_rwhv->GetRenderWidgetHost());
   SendMouseWheel(pos);
-  EXPECT_EQ(child_rwhv, router->wheel_target_.target);
+  EXPECT_EQ(child_rwhv, router->wheel_target_);
 
   // Verify that this a mouse wheel event was sent to the child frame renderer.
   EXPECT_TRUE(child_frame_monitor.EventWasReceived());
@@ -3561,7 +3561,133 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessMouseWheelHitTestBrowserTest,
       child_process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
   child_process->Shutdown(0);
   crash_observer.Wait();
-  EXPECT_EQ(nullptr, router->wheel_target_.target);
+  EXPECT_EQ(nullptr, router->wheel_target_);
+}
+
+// Ensure that the positions of mouse wheel events sent to cross-process
+// subframes account for any change in the position of the subframe during the
+// scroll sequence.
+IN_PROC_BROWSER_TEST_P(SitePerProcessMouseWheelHitTestBrowserTest,
+                       MouseWheelEventPositionChange) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "/frame_tree/page_with_tall_positioned_frame.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  auto* rwhv_root = static_cast<RenderWidgetHostViewAura*>(
+      web_contents()->GetRenderWidgetHostView());
+  set_rwhv_root(rwhv_root);
+
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+  ASSERT_EQ(1U, root->child_count());
+
+  // Synchronize with the child and parent renderers to guarantee that the
+  // surface information required for event hit testing is ready.
+  RenderWidgetHostViewChildFrame* child_rwhv =
+      static_cast<RenderWidgetHostViewChildFrame*>(
+          root->child_at(0)->current_frame_host()->GetView());
+  WaitForHitTestDataOrChildSurfaceReady(
+      root->child_at(0)->current_frame_host());
+
+  RenderWidgetHostInputEventRouter* router =
+      web_contents()->GetInputEventRouter();
+
+  auto await_gesture_event_with_position = base::BindRepeating(
+      [](blink::WebInputEvent::Type expected_type,
+         RenderWidgetHostViewBase* rwhv, gfx::PointF expected_position,
+         gfx::PointF expected_position_in_root, InputEventAckSource,
+         InputEventAckState, const blink::WebInputEvent& event) {
+        if (event.GetType() != expected_type)
+          return false;
+
+        const auto& gesture_event =
+            static_cast<const blink::WebGestureEvent&>(event);
+        const gfx::PointF root_point = rwhv->TransformPointToRootCoordSpaceF(
+            gesture_event.PositionInWidget());
+
+        EXPECT_FLOAT_EQ(gesture_event.PositionInWidget().x,
+                        expected_position.x());
+        EXPECT_FLOAT_EQ(gesture_event.PositionInWidget().y,
+                        expected_position.y());
+        EXPECT_FLOAT_EQ(root_point.x(), expected_position_in_root.x());
+        EXPECT_FLOAT_EQ(root_point.y(), expected_position_in_root.y());
+        return true;
+      });
+  MainThreadFrameObserver thread_observer(rwhv_root->GetRenderWidgetHost());
+
+  // Send a mouse wheel begin event to child.
+  blink::WebMouseWheelEvent scroll_event(
+      blink::WebInputEvent::kMouseWheel, blink::WebInputEvent::kNoModifiers,
+      blink::WebInputEvent::GetStaticTimeStampForTests());
+  gfx::Point child_point_in_root(90, 90);
+  SetWebEventPositions(&scroll_event, child_point_in_root, rwhv_root);
+  scroll_event.delta_x = 0.0f;
+  scroll_event.delta_y = -20.0f;
+  scroll_event.phase = blink::WebMouseWheelEvent::kPhaseBegan;
+  scroll_event.has_precise_scrolling_deltas = true;
+
+  {
+    InputEventAckWaiter await_begin_in_child(
+        child_rwhv->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureScrollBegin,
+                            child_rwhv, gfx::PointF(38, 38),
+                            gfx::PointF(child_point_in_root)));
+    InputEventAckWaiter await_update_in_child(
+        child_rwhv->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureScrollUpdate,
+                            child_rwhv, gfx::PointF(38, 38),
+                            gfx::PointF(child_point_in_root)));
+    InputEventAckWaiter await_update_in_root(
+        rwhv_root->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureScrollUpdate,
+                            rwhv_root, gfx::PointF(child_point_in_root),
+                            gfx::PointF(child_point_in_root)));
+    router->RouteMouseWheelEvent(rwhv_root, &scroll_event, ui::LatencyInfo());
+    await_begin_in_child.Wait();
+    await_update_in_child.Wait();
+    await_update_in_root.Wait();
+    thread_observer.Wait();
+  }
+
+  // Send mouse wheel update event to child.
+  {
+    scroll_event.phase = blink::WebMouseWheelEvent::kPhaseChanged;
+    InputEventAckWaiter await_update_in_child(
+        child_rwhv->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureScrollUpdate,
+                            child_rwhv, gfx::PointF(38, 58),
+                            gfx::PointF(child_point_in_root)));
+    InputEventAckWaiter await_update_in_root(
+        rwhv_root->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureScrollUpdate,
+                            rwhv_root, gfx::PointF(child_point_in_root),
+                            gfx::PointF(child_point_in_root)));
+    router->RouteMouseWheelEvent(rwhv_root, &scroll_event, ui::LatencyInfo());
+    await_update_in_child.Wait();
+    await_update_in_root.Wait();
+    thread_observer.Wait();
+  }
+
+#if !defined(OS_WIN)
+  {
+    ui::ScrollEvent fling_start(ui::ET_SCROLL_FLING_START, child_point_in_root,
+                                ui::EventTimeForNow(), 0, 10, 0, 10, 0, 1);
+    UpdateEventRootLocation(&fling_start, rwhv_root);
+
+    InputEventAckWaiter await_fling_start_in_child(
+        child_rwhv->GetRenderWidgetHost(),
+        base::BindRepeating(await_gesture_event_with_position,
+                            blink::WebInputEvent::kGestureFlingStart,
+                            child_rwhv, gfx::PointF(38, 78),
+                            gfx::PointF(child_point_in_root)));
+    rwhv_root->OnScrollEvent(&fling_start);
+    await_fling_start_in_child.Wait();
+    thread_observer.Wait();
+  }
+#endif
 }
 
 // Ensure that a cross-process subframe with a touch-handler can receive touch
@@ -4186,7 +4312,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
       contents->GetRenderWidgetHostView());
 
   RenderWidgetHostInputEventRouter* router = contents->GetInputEventRouter();
-  EXPECT_EQ(nullptr, router->touchpad_gesture_target_.target);
+  EXPECT_EQ(nullptr, router->touchpad_gesture_target_);
 
   // TODO(848050): If we send multiple touchpad pinch sequences to separate
   // views and the timing of the acks are such that the begin ack of the second
@@ -4215,38 +4341,37 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
   gfx::Point child_center(150, 150);
 
   // Send touchpad pinch sequence to main-frame.
-  SendTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, main_frame_point, router->touchpad_gesture_target_.target,
-      rwhv_parent);
+  SendTouchpadPinchSequenceWithExpectedTarget(rwhv_parent, main_frame_point,
+                                              router->touchpad_gesture_target_,
+                                              rwhv_parent);
 
   wait_for_pinch_sequence_end.Run();
 
   // Send touchpad pinch sequence to child.
   SendTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, child_center, router->touchpad_gesture_target_.target,
-      rwhv_child);
+      rwhv_parent, child_center, router->touchpad_gesture_target_, rwhv_child);
 
   wait_for_pinch_sequence_end.Run();
 
   // Send another touchpad pinch sequence to main frame.
-  SendTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, main_frame_point, router->touchpad_gesture_target_.target,
-      rwhv_parent);
+  SendTouchpadPinchSequenceWithExpectedTarget(rwhv_parent, main_frame_point,
+                                              router->touchpad_gesture_target_,
+                                              rwhv_parent);
 
 #if !defined(OS_WIN)
   // Sending touchpad fling events is not supported on Windows.
 
   // Send touchpad fling sequence to main-frame.
   SendTouchpadFlingSequenceWithExpectedTarget(
-      rwhv_parent, main_frame_point, router->wheel_target_.target, rwhv_parent);
+      rwhv_parent, main_frame_point, router->wheel_target_, rwhv_parent);
 
   // Send touchpad fling sequence to child.
   SendTouchpadFlingSequenceWithExpectedTarget(
-      rwhv_parent, child_center, router->wheel_target_.target, rwhv_child);
+      rwhv_parent, child_center, router->wheel_target_, rwhv_child);
 
   // Send another touchpad fling sequence to main frame.
   SendTouchpadFlingSequenceWithExpectedTarget(
-      rwhv_parent, main_frame_point, router->wheel_target_.target, rwhv_parent);
+      rwhv_parent, main_frame_point, router->wheel_target_, rwhv_parent);
 #endif
 }
 
@@ -4279,7 +4404,7 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
       contents->GetRenderWidgetHostView());
 
   RenderWidgetHostInputEventRouter* router = contents->GetInputEventRouter();
-  EXPECT_EQ(nullptr, router->touchpad_gesture_target_.target);
+  EXPECT_EQ(nullptr, router->touchpad_gesture_target_);
 
   const float scale_factor =
       render_frame_submission_observer.LastRenderFrameMetadata()
@@ -4288,9 +4413,9 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
                                   gfx::ToCeiledInt(100 * scale_factor));
 
   content::TestPageScaleObserver scale_observer(shell()->web_contents());
-  SendTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, point_in_child, router->touchpad_gesture_target_.target,
-      rwhv_child);
+  SendTouchpadPinchSequenceWithExpectedTarget(rwhv_parent, point_in_child,
+                                              router->touchpad_gesture_target_,
+                                              rwhv_child);
 
   // Ensure the child frame saw the wheel event.
   bool default_prevented = false;

content/browser/site_per_process_mac_browsertest.mm

@@ -313,20 +313,19 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessMacBrowserTest,
       contents->GetRenderWidgetHostView());
 
   RenderWidgetHostInputEventRouter* router = contents->GetInputEventRouter();
-  EXPECT_EQ(nullptr, router->touchpad_gesture_target_.target);
+  EXPECT_EQ(nullptr, router->touchpad_gesture_target_);
 
   gfx::Point main_frame_point(25, 575);
   gfx::Point child_center(150, 450);
 
   // Send touchpad pinch sequence to main-frame.
   SendMacTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, main_frame_point, router->touchpad_gesture_target_.target,
+      rwhv_parent, main_frame_point, router->touchpad_gesture_target_,
       rwhv_parent);
 
   // Send touchpad pinch sequence to child.
   SendMacTouchpadPinchSequenceWithExpectedTarget(
-      rwhv_parent, child_center, router->touchpad_gesture_target_.target,
-      rwhv_child);
+      rwhv_parent, child_center, router->touchpad_gesture_target_, rwhv_child);
 }
 
 }  // namespace content