OOPIF: Transform point of resent gesture event instead of applying offset.

When bubbling scroll events to ancestor views, we adjust the event's
position by applying the parent's offset. This is incorrect if an OOPIF
is subject to CSS scaling, since the point in the child is scaled. We
now use a transformation function which accounts for scaling.

We were also assuming that the event was being resent to the parent. This
is true for GestureScrollBegin events, but for the remaining events in the
sequence, we may bubble directly to an ancestor, so those events have the
coordinates for the wrong view. We now defer the transformation of the
coordinates to the target view's coordinate space to the
RenderWidgetHostInputEventRouter which knows which view is the target.

Bug: 626020, 817392
Change-Id: I86331f7bfc5fe7dd9e9032beb16ebb75b579f5b1
Reviewed-on: https://chromium-review.googlesource.com/978647Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545890}

content/browser/frame_host/cross_process_frame_connector.cc

@@ -211,13 +211,14 @@ void CrossProcessFrameConnector::BubbleScrollEvent(
 
   auto* event_router = parent_view->host()->delegate()->GetInputEventRouter();
 
-  gfx::Vector2d offset_from_parent =
-      screen_space_rect_in_dip_.OffsetFromOrigin();
+  // We will only convert the coordinates back to the root here. The
+  // RenderWidgetHostInputEventRouter will determine which ancestor view will
+  // receive a resent gesture event, so it will be responsible for converting to
+  // the coordinates of the target view.
   blink::WebGestureEvent resent_gesture_event(event);
-  // TODO(kenrb, wjmaclean): Do we need to account for transforms here?
-  // See https://crbug.com/626020.
-  resent_gesture_event.SetPositionInWidget(
-      resent_gesture_event.PositionInWidget() + offset_from_parent);
+  const gfx::PointF root_point =
+      view_->TransformPointToRootCoordSpaceF(event.PositionInWidget());
+  resent_gesture_event.SetPositionInWidget(root_point);
 
   if (view_->wheel_scroll_latching_enabled()) {
     if (event.GetType() == blink::WebInputEvent::kGestureScrollBegin) {

content/browser/renderer_host/render_widget_host_input_event_router.cc

@@ -743,6 +743,22 @@ void RenderWidgetHostInputEventRouter::ReportBubblingScrollToSameView(
   base::debug::DumpWithoutCrashing();
 }
 
+namespace {
+
+// Given |event| in root coordinates, return an event in |target_view|'s
+// coordinates.
+blink::WebGestureEvent GestureEventInTarget(
+    const blink::WebGestureEvent& event,
+    RenderWidgetHostViewBase* target_view) {
+  const gfx::PointF point_in_target =
+      target_view->TransformRootPointToViewCoordSpace(event.PositionInWidget());
+  blink::WebGestureEvent event_for_target(event);
+  event_for_target.SetPositionInWidget(point_in_target);
+  return event_for_target;
+}
+
+}  // namespace
+
 void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
     RenderWidgetHostViewBase* target_view,
     const blink::WebGestureEvent& event,
@@ -773,10 +789,14 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
       // event ack which didn't consume any scroll delta, and so another level
       // of bubbling is needed. This requires a GestureScrollEnd be sent to the
       // last view, which will no longer be the scroll target.
-      if (bubbling_gesture_scroll_target_.target)
-        SendGestureScrollEnd(bubbling_gesture_scroll_target_.target, event);
-      else
+      if (bubbling_gesture_scroll_target_.target) {
+        SendGestureScrollEnd(
+            bubbling_gesture_scroll_target_.target,
+            GestureEventInTarget(event,
+                                 bubbling_gesture_scroll_target_.target));
+      } else {
         first_bubbling_scroll_target_.target = target_view;
+      }
 
       bubbling_gesture_scroll_target_.target = target_view;
     } else {  // !(event.GetType() == blink::WebInputEvent::kGestureScrollBegin)
@@ -808,8 +828,9 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
       return;
     }
 
-    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(event,
-                                                                latency_info);
+    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(
+        GestureEventInTarget(event, bubbling_gesture_scroll_target_.target),
+        latency_info);
     if (event.GetType() == blink::WebInputEvent::kGestureScrollEnd ||
         event.GetType() == blink::WebInputEvent::kGestureFlingStart) {
       first_bubbling_scroll_target_.target = nullptr;
@@ -829,8 +850,9 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
   // If target_view is already set up for bubbled scrolls, we forward
   // the event to the current scroll target without further consideration.
   if (target_view == first_bubbling_scroll_target_.target) {
-    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(event,
-                                                                latency_info);
+    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(
+        GestureEventInTarget(event, bubbling_gesture_scroll_target_.target),
+        latency_info);
     if (event.GetType() == blink::WebInputEvent::kGestureScrollEnd ||
         event.GetType() == blink::WebInputEvent::kGestureFlingStart) {
       first_bubbling_scroll_target_.target = nullptr;
@@ -849,8 +871,9 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
   // have been sent to a renderer before the first one was ACKed, and the ACK
   // caused a bubble retarget. In this case they all get forwarded.
   if (target_view == bubbling_gesture_scroll_target_.target) {
-    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(event,
-                                                                latency_info);
+    bubbling_gesture_scroll_target_.target->ProcessGestureEvent(
+        GestureEventInTarget(event, bubbling_gesture_scroll_target_.target),
+        latency_info);
     return;
   }
 
@@ -867,15 +890,19 @@ void RenderWidgetHostInputEventRouter::BubbleScrollEvent(
   // unused scroll delta, and so another level of bubbling is needed. This
   // requires a GestureScrollEnd be sent to the last view, which will no
   // longer be the scroll target.
-  if (bubbling_gesture_scroll_target_.target)
-    SendGestureScrollEnd(bubbling_gesture_scroll_target_.target, event);
-  else
+  if (bubbling_gesture_scroll_target_.target) {
+    SendGestureScrollEnd(
+        bubbling_gesture_scroll_target_.target,
+        GestureEventInTarget(event, bubbling_gesture_scroll_target_.target));
+  } else {
     first_bubbling_scroll_target_.target = target_view;
+  }
 
   bubbling_gesture_scroll_target_.target = target_view;
 
-  SendGestureScrollBegin(target_view, event);
-  target_view->ProcessGestureEvent(event, latency_info);
+  SendGestureScrollBegin(target_view, GestureEventInTarget(event, target_view));
+  target_view->ProcessGestureEvent(GestureEventInTarget(event, target_view),
+                                   latency_info);
 }
 
 void RenderWidgetHostInputEventRouter::SendGestureScrollBegin(

content/browser/renderer_host/render_widget_host_input_event_router.h

@@ -79,6 +79,7 @@ class CONTENT_EXPORT RenderWidgetHostInputEventRouter
                        blink::WebTouchEvent *event,
                        const ui::LatencyInfo& latency);
 
+  // |event| is in root coordinates.
   void BubbleScrollEvent(RenderWidgetHostViewBase* target_view,
                          const blink::WebGestureEvent& event,
                          const RenderWidgetHostViewBase* resending_view);

content/browser/site_per_process_hit_test_browsertest.cc

@@ -911,6 +911,76 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
   scroll_end_observer.Wait();
 }
 
+// When a scroll event is bubbled, ensure that the bubbled event's coordinates
+// are correctly updated to the ancestor's coordinate space. In particular,
+// ensure that the transformation considers CSS scaling of the child where
+// simply applying the ancestor's offset does not produce the correct
+// coordinates in the ancestor's coordinate space.
+// See https://crbug.com/817392
+IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
+                       BubbledScrollEventsTransformedCorrectly) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "/frame_tree/page_with_positioned_scaled_frame.html"));
+  ASSERT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+  ASSERT_EQ(1U, root->child_count());
+
+  FrameTreeNode* iframe_node = root->child_at(0);
+  GURL site_url(embedded_test_server()->GetURL("baz.com", "/title1.html"));
+  EXPECT_EQ(site_url, iframe_node->current_url());
+
+  RenderWidgetHostViewBase* root_rwhv = static_cast<RenderWidgetHostViewBase*>(
+      root->current_frame_host()->GetRenderWidgetHost()->GetView());
+
+  RenderWidgetHostInputEventRouter* router =
+      static_cast<WebContentsImpl*>(shell()->web_contents())
+          ->GetInputEventRouter();
+
+  WaitForChildFrameSurfaceReady(iframe_node->current_frame_host());
+
+  const float scale_factor = GetPageScaleFactor(shell());
+  // Due to the CSS scaling of the iframe, the position in the child view's
+  // coordinates is (96, 96) and not (48, 48) (or approximately these values
+  // if there's rounding due to the scale factor).
+  const gfx::Point position_in_root(gfx::ToCeiledInt(150 * scale_factor),
+                                    gfx::ToCeiledInt(150 * scale_factor));
+
+  auto expect_gsb_with_position = base::BindRepeating(
+      [](const gfx::Point& expected_position, content::InputEventAckSource,
+         content::InputEventAckState, const blink::WebInputEvent& event) {
+        if (event.GetType() != blink::WebInputEvent::kGestureScrollBegin)
+          return false;
+
+        const blink::WebGestureEvent& gesture_event =
+            static_cast<const blink::WebGestureEvent&>(event);
+        EXPECT_NEAR(expected_position.x(), gesture_event.PositionInWidget().x,
+                    1);
+        EXPECT_NEAR(expected_position.y(), gesture_event.PositionInWidget().y,
+                    1);
+        return true;
+      });
+
+  InputEventAckWaiter root_scroll_begin_observer(
+      root_rwhv->GetRenderWidgetHost(),
+      base::BindRepeating(expect_gsb_with_position, position_in_root));
+
+  // Scroll the iframe upward, scroll events get bubbled up to the root.
+  blink::WebMouseWheelEvent scroll_event(
+      blink::WebInputEvent::kMouseWheel, blink::WebInputEvent::kNoModifiers,
+      blink::WebInputEvent::GetStaticTimeStampForTests());
+  SetWebEventPositions(&scroll_event, position_in_root, root_rwhv);
+  scroll_event.delta_x = 0.0f;
+  scroll_event.delta_y = 5.0f;
+  scroll_event.phase = blink::WebMouseWheelEvent::kPhaseBegan;
+  scroll_event.has_precise_scrolling_deltas = true;
+
+  router->RouteMouseWheelEvent(root_rwhv, &scroll_event, ui::LatencyInfo());
+
+  root_scroll_begin_observer.Wait();
+}
+
 #if defined(USE_AURA) || defined(OS_ANDROID)
 
 // When unconsumed scrolls in a child bubble to the root and start an

content/test/data/frame_tree/page_with_positioned_scaled_frame.html

+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      iframe {
+        position: absolute;
+        top: 50px;
+        left: 50px;
+        width: 200px;
+        height: 200px;
+        transform: scale(0.5);
+      }
+    </style>
+  </head>
+  <body>
+    <iframe src="/cross-site/baz.com/title1.html"></iframe>
+    This page contains a positioned and scaled cross-origin iframe.
+  </body>
+</html>

testing/buildbot/filters/viz.content_browsertests.filter

@@ -34,6 +34,7 @@
 -SitePerProcessBrowserTest.TwoSubframesCreatePopupMenuWidgetsSimultaneously
 -SitePerProcessBrowserTest.ViewBoundsInNestedFrameTest
 -SitePerProcessHitTestBrowserTest.AsynchronousHitTestChildTimeout*
+-SitePerProcessHitTestBrowserTest.BubbledScrollEventsTransformedCorrectly*
 -SitePerProcessHitTestBrowserTest.CancelWheelScrollBubblingOnWheelTargetDeletion*
 -SitePerProcessHitTestBrowserTest.CreateContextMenuTest*
 -SitePerProcessHitTestBrowserTest.CrossProcessMouseCapture*