Fix TouchSelectionHandle updates on Android.

This CL addresses the remaining issues with Android touch selection
handles. First, it removes the manual scaling done by the child frame
client for the device scale factor; apparently this is now handled by
the point-transform functions. Second, it makes sure the child frame's
handles update during pinch just like the main frame's handles do.

Because a child frame's handles are not locked to a surface like the
main frame's handles, there is a small amount of jitter/lag when
pinching or scrolling the main-frame, but not enough to impair
functionality.

Note this CL also resolves the issues for desktop handles not updating
after pinch as well.

Bug: 809122
Change-Id: I120386c3d7a0d87dacc655e83b07e0cfeaf4920a
Reviewed-on: https://chromium-review.googlesource.com/c/1259945
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600412}

content/browser/renderer_host/input/touch_selection_controller_client_child_frame.cc

@@ -35,40 +35,32 @@ void TouchSelectionControllerClientChildFrame::DidStopFlinging() {
   manager_->DidStopFlinging();
 }
 
+void TouchSelectionControllerClientChildFrame::
+    TransformSelectionBoundsAndUpdate() {
+  gfx::SelectionBound transformed_selection_start(selection_start_);
+  gfx::SelectionBound transformed_selection_end(selection_end_);
+
+  // TODO(wjmaclean): Get the transform between the views to lower the
+  // overhead here, instead of calling the transform functions four times.
+  transformed_selection_start.SetEdge(
+      rwhv_->TransformPointToRootCoordSpaceF(selection_start_.edge_top()),
+      rwhv_->TransformPointToRootCoordSpaceF(selection_start_.edge_bottom()));
+  transformed_selection_end.SetEdge(
+      rwhv_->TransformPointToRootCoordSpaceF(selection_end_.edge_top()),
+      rwhv_->TransformPointToRootCoordSpaceF(selection_end_.edge_bottom()));
+
+  manager_->UpdateClientSelectionBounds(transformed_selection_start,
+                                        transformed_selection_end, this, this);
+}
+
 void TouchSelectionControllerClientChildFrame::UpdateSelectionBoundsIfNeeded(
     const viz::Selection<gfx::SelectionBound>& selection,
     float device_scale_factor) {
-  gfx::PointF start_edge_top = selection.start.edge_top();
-  gfx::PointF start_edge_bottom = selection.start.edge_bottom();
-  gfx::PointF end_edge_top = selection.end.edge_top();
-  gfx::PointF end_edge_bottom = selection.end.edge_bottom();
-
-  if (IsUseZoomForDSFEnabled()) {
-    float viewportToDIPScale = 1.0f / device_scale_factor;
-
-    start_edge_top.Scale(viewportToDIPScale);
-    start_edge_bottom.Scale(viewportToDIPScale);
-    end_edge_top.Scale(viewportToDIPScale);
-    end_edge_bottom.Scale(viewportToDIPScale);
-  }
+  if (selection.start != selection_start_ || selection.end != selection_end_) {
+    selection_start_ = selection.start;
+    selection_end_ = selection.end;
 
-  gfx::PointF origin = rwhv_->TransformPointToRootCoordSpaceF(gfx::PointF());
-  start_edge_top = rwhv_->TransformPointToRootCoordSpaceF(start_edge_top);
-  start_edge_bottom = rwhv_->TransformPointToRootCoordSpaceF(start_edge_bottom);
-  end_edge_top = rwhv_->TransformPointToRootCoordSpaceF(end_edge_top);
-  end_edge_bottom = rwhv_->TransformPointToRootCoordSpaceF(end_edge_bottom);
-
-  viz::Selection<gfx::SelectionBound> transformed_selection(selection);
-  transformed_selection.start.SetEdge(start_edge_top, start_edge_bottom);
-  transformed_selection.end.SetEdge(end_edge_top, end_edge_bottom);
-
-  if (transformed_selection.start != selection_start_ ||
-      transformed_selection.end != selection_end_) {
-    selection_start_ = transformed_selection.start;
-    selection_end_ = transformed_selection.end;
-    view_origin_at_last_update_ = origin;
-    manager_->UpdateClientSelectionBounds(selection_start_, selection_end_,
-                                          this, this);
+    TransformSelectionBoundsAndUpdate();
   }
 }
 
@@ -77,17 +69,7 @@ void TouchSelectionControllerClientChildFrame::UpdateSelectionBoundsIfNeeded(
 // sending a new compositor frame), we must manually recompute the screen
 // position if requested to do so and it has changed.
 void TouchSelectionControllerClientChildFrame::DidScroll() {
-  gfx::PointF origin = rwhv_->TransformPointToRootCoordSpaceF(gfx::PointF());
-  if (origin != view_origin_at_last_update_) {
-    gfx::Vector2dF delta = origin - view_origin_at_last_update_;
-    selection_start_.SetEdge(selection_start_.edge_top() + delta,
-                             selection_start_.edge_bottom() + delta);
-    selection_end_.SetEdge(selection_end_.edge_top() + delta,
-                           selection_end_.edge_bottom() + delta);
-    view_origin_at_last_update_ = origin;
-    manager_->UpdateClientSelectionBounds(selection_start_, selection_end_,
-                                          this, this);
-  }
+  TransformSelectionBoundsAndUpdate();
 }
 
 gfx::Point TouchSelectionControllerClientChildFrame::ConvertFromRoot(

content/browser/renderer_host/input/touch_selection_controller_client_child_frame.h

@@ -37,6 +37,8 @@ class CONTENT_EXPORT TouchSelectionControllerClientChildFrame
       float device_scale_factor);
 
  private:
+  void TransformSelectionBoundsAndUpdate();
+
   // ui::TouchSelectionControllerClient:
   bool SupportsAnimation() const override;
   void SetNeedsAnimate() override;
@@ -60,11 +62,9 @@ class CONTENT_EXPORT TouchSelectionControllerClientChildFrame
   RenderWidgetHostViewChildFrame* rwhv_;
   TouchSelectionControllerClientManager* manager_;
 
-  // The last selection bounds reported by the view.
+  // The last selection bounds reported by the view, in view coordinates.
   gfx::SelectionBound selection_start_;
   gfx::SelectionBound selection_end_;
-  // Keep track of the view origin as of the last time selection was updated.
-  gfx::PointF view_origin_at_last_update_;
 
   DISALLOW_COPY_AND_ASSIGN(TouchSelectionControllerClientChildFrame);
 };

content/browser/renderer_host/input/touch_selection_controller_client_manager_android.cc

@@ -4,19 +4,29 @@
 
 #include "content/browser/renderer_host/input/touch_selection_controller_client_manager_android.h"
 
+#include "components/viz/common/hit_test/aggregated_hit_test_region.h"
+#include "components/viz/common/surfaces/frame_sink_id.h"
+#include "components/viz/host/host_frame_sink_manager.h"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 
 namespace content {
 
 TouchSelectionControllerClientManagerAndroid::
     TouchSelectionControllerClientManagerAndroid(
-        RenderWidgetHostViewAndroid* rwhv)
-    : rwhv_(rwhv), active_client_(rwhv) {
+        RenderWidgetHostViewAndroid* rwhv,
+        viz::HostFrameSinkManager* host_frame_sink_manager)
+    : rwhv_(rwhv),
+      host_frame_sink_manager_(host_frame_sink_manager),
+      active_client_(rwhv) {
   DCHECK(rwhv_);
+  DCHECK(host_frame_sink_manager_);
 }
 
 TouchSelectionControllerClientManagerAndroid::
     ~TouchSelectionControllerClientManagerAndroid() {
+  if (active_client_ != rwhv_)
+    host_frame_sink_manager_->RemoveHitTestRegionObserver(this);
+
   for (auto& observer : observers_)
     observer.OnManagerWillDestroy(this);
 }
@@ -39,6 +49,16 @@ void TouchSelectionControllerClientManagerAndroid::UpdateClientSelectionBounds(
     return;
   }
 
+  // Since the observer method does very little processing, and early-outs when
+  // not displaying handles, we don't bother un-installing it when an OOPIF
+  // client is not currently displaying handles.
+  if (client != active_client_) {
+    if (active_client_ == rwhv_)  // We are switching to an OOPIF client.
+      host_frame_sink_manager_->AddHitTestRegionObserver(this);
+    else if (client == rwhv_)  // We are switching to a non-OOPIF client.
+      host_frame_sink_manager_->RemoveHitTestRegionObserver(this);
+  }
+
   active_client_ = client;
   manager_selection_start_ = start;
   manager_selection_end_ = end;
@@ -118,4 +138,19 @@ void TouchSelectionControllerClientManagerAndroid::DidScroll() {
   // Nothing needs to be done here.
 }
 
+void TouchSelectionControllerClientManagerAndroid::
+    OnAggregatedHitTestRegionListUpdated(
+        const viz::FrameSinkId& frame_sink_id,
+        const std::vector<viz::AggregatedHitTestRegion>& hit_test_data) {
+  DCHECK(active_client_ != rwhv_);
+
+  if (!GetTouchSelectionController() ||
+      GetTouchSelectionController()->active_status() ==
+          ui::TouchSelectionController::INACTIVE) {
+    return;
+  }
+
+  active_client_->DidScroll();
+}
+
 }  // namespace content

content/browser/renderer_host/input/touch_selection_controller_client_manager_android.h

@@ -7,19 +7,30 @@
 
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "components/viz/host/hit_test/hit_test_region_observer.h"
 #include "content/public/browser/touch_selection_controller_client_manager.h"
 #include "ui/touch_selection/touch_selection_controller.h"
 
+namespace viz {
+
+class HostFrameSinkManager;
+class FrameSinkId;
+struct AggregatedHitTestRegion;
+
+}  // namespace viz
+
 namespace content {
 
 class RenderWidgetHostViewAndroid;
 
 class TouchSelectionControllerClientManagerAndroid
     : public TouchSelectionControllerClientManager,
-      public ui::TouchSelectionControllerClient {
+      public ui::TouchSelectionControllerClient,
+      public viz::HitTestRegionObserver {
  public:
   explicit TouchSelectionControllerClientManagerAndroid(
-      RenderWidgetHostViewAndroid* rwhv);
+      RenderWidgetHostViewAndroid* rwhv,
+      viz::HostFrameSinkManager* frame_host_sink_manager);
   ~TouchSelectionControllerClientManagerAndroid() override;
 
   // TouchSelectionControllerClientManager implementation.
@@ -47,8 +58,17 @@ class TouchSelectionControllerClientManagerAndroid
   std::unique_ptr<ui::TouchHandleDrawable> CreateDrawable() override;
   void DidScroll() override;
 
+  // viz::HitTestRegionObserver implementation.
+  void OnAggregatedHitTestRegionListUpdated(
+      const viz::FrameSinkId& frame_sink_id,
+      const std::vector<viz::AggregatedHitTestRegion>& hit_test_data) override;
+
  private:
+  // Neither of the following pointers are owned, and both are assumed to
+  // outlive this object.
   RenderWidgetHostViewAndroid* rwhv_;
+  viz::HostFrameSinkManager* host_frame_sink_manager_;
+
   TouchSelectionControllerClient* active_client_;
   gfx::SelectionBound manager_selection_start_;
   gfx::SelectionBound manager_selection_end_;

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -212,7 +212,8 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
 
   host()->SetView(this);
   touch_selection_controller_client_manager_ =
-      std::make_unique<TouchSelectionControllerClientManagerAndroid>(this);
+      std::make_unique<TouchSelectionControllerClientManagerAndroid>(
+          this, CompositorImpl::GetHostFrameSinkManager());
 
   UpdateNativeViewTree(parent_native_view);
   // This RWHVA may have been created speculatively. We should give any