Refactor browser process coordinate transformation methods

content/browser/frame_host/cross_process_frame_connector.cc

Index: content/browser/frame_host/cross_process_frame_connector.cc
diff --git a/content/browser/frame_host/cross_process_frame_connector.cc b/content/browser/frame_host/cross_process_frame_connector.cc
index 5ee851727a0e0a25704f064a089ab69d52df2a1a..94122b0fa81c02518a9cbeb66105670873737171 100644
--- a/content/browser/frame_host/cross_process_frame_connector.cc
+++ b/content/browser/frame_host/cross_process_frame_connector.cc
@@ -5,6 +5,7 @@
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 
 #include "cc/surfaces/surface.h"
+#include "cc/surfaces/surface_hittest.h"
 #include "cc/surfaces/surface_manager.h"
 #include "content/browser/compositor/surface_utils.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ -20,6 +21,7 @@
 #include "content/common/frame_messages.h"
 #include "gpu/ipc/common/gpu_messages.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
+#include "ui/gfx/geometry/dip_util.h"
 
 namespace content {
 
@@ -136,13 +138,63 @@ void CrossProcessFrameConnector::UpdateCursor(const WebCursor& cursor) {
 
 gfx::Point CrossProcessFrameConnector::TransformPointToRootCoordSpace(
     const gfx::Point& point,
-    cc::SurfaceId surface_id) {
-  gfx::Point transformed_point = point;
+    const cc::SurfaceId& surface_id) {
+  return TransformPointToCoordSpaceForView(point, GetRootRenderWidgetHostView(),
+                                           surface_id);
+}
+
+gfx::Point CrossProcessFrameConnector::TransformPointToLocalCoordSpace(
+    const gfx::Point& point,
+    const cc::SurfaceId& original_surface,
+    const cc::SurfaceId& local_surface_id) {
+  if (original_surface == local_surface_id)
+    return point;
+
+  // Transformations use physical pixels rather than DIP, so conversion
+  // is necessary.
+  gfx::Point point_in_pixels =
+      gfx::ConvertPointToPixel(device_scale_factor_, point);
+  gfx::Transform transform;
+  gfx::Point transformed_point = point_in_pixels;
+  cc::SurfaceHittest hittest(nullptr, GetSurfaceManager());
+  // Two possibilities need to be considered: original_surface can be embedded
+  // in local_surface_id, or vice versa.
+  if (hittest.GetTransformToTargetSurface(local_surface_id, original_surface,
+                                          &transform) &&
+      transform.GetInverse(&transform)) {
+    transform.TransformPoint(&transformed_point);
+  } else if (hittest.GetTransformToTargetSurface(
+                 original_surface, local_surface_id, &transform)) {
+    // No need to invert the transform matrix in this case.
+    transform.TransformPoint(&transformed_point);
+  } else {
+    // Neither surface is embedded in the other, which violates the
+    // preconditions of this method.
+    DCHECK(false);
+    return point;
+  }
+  return gfx::ConvertPointToDIP(device_scale_factor_, transformed_point);
+}
+
+gfx::Point CrossProcessFrameConnector::TransformPointToCoordSpaceForView(
+    const gfx::Point& point,
+    RenderWidgetHostViewBase* target_view,
+    const cc::SurfaceId& local_surface_id) {
   RenderWidgetHostViewBase* root_view = GetRootRenderWidgetHostView();
-  if (root_view)
-    root_view->TransformPointToLocalCoordSpace(point, surface_id,
-                                               &transformed_point);
-  return transformed_point;
+  if (!root_view)

[nasko, 2016/07/27 22:52:13]

How would root_view be null?

[kenrb, 2016/07/28 16:24:36]

I don't know, but it might be possible for it to happen in a nested WebContents
during, for instance, a renderer crash. I doubt it can happen for OOPIFs (and
just ran SitePerProcessBrowserTest.* with a DCHECK in
GetRootRenderWidgetHostView), but I am reluctant to remove all of the guards
because I don't know if there is adequate test coverage for OOPIF-webview.

+    return point;
+
+  // It is possible that neither the original surface or target surface is an
+  // ancestor of the other in the RenderWidgetHostView tree (e.g. they could
+  // be siblings). To account for this, the point is first tranformed into the
+  // root coordinate space and then the root is asked to perform the conversion.
+  gfx::Point root_point =
+      root_view->TransformPointToLocalCoordSpace(point, local_surface_id);
+
+  if (target_view == root_view)
+    return root_point;
+
+  return root_view->TransformPointToCoordSpaceForView(point, target_view);
 }
 
 void CrossProcessFrameConnector::ForwardProcessAckedTouchEvent(