Implement disambiguation popup

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 4cf9502c982b22a15050cb76d59a1897072d0215..a17177b67249ac5cd14035b91733a5c1002884e9 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -355,6 +355,21 @@ static const float kScalingIncrement = 0.1f;
 
 static const float kScalingIncrementForGesture = 0.01f;
 
+// The amount of padding to add to the disambiguation popup to show
+// content around the possible elements, adding some context.
+static const int kDisambiguationPopupPadding = 8;
+
+// Constants used for fitting the disambiguation popup inside the bounds of
+// the view. Note that there are mirror constants in PopupZoomer.java.
+static const int kDisambiguationPopupBoundsMargin = 25;
+
+// The smallest allowable touch target used for disambiguation popup.
+// This value is used to determine the minimum amount we need to scale to
+// make all targets touchable.
+static const int kDisambiguationPopupMinimumTouchSize = 40;
+static const float kDisambiguationPopupMaxScale = 5.0;
+static const float kDisambiguationPopupMinScale = 2.0;
+
 #if defined(OS_ANDROID)
 // Delay between tapping in content and launching the associated android intent.
 // Used to allow users see what has been recognized as content.
@@ -1007,6 +1022,8 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_EnableViewSourceMode, OnEnableViewSourceMode)
     IPC_MESSAGE_HANDLER(JavaBridgeMsg_Init, OnJavaBridgeInit)
     IPC_MESSAGE_HANDLER(ViewMsg_SetAccessibilityMode, OnSetAccessibilityMode)
+    IPC_MESSAGE_HANDLER(ViewMsg_ReleaseDisambiguationPopupDIB,
+                        OnReleaseDisambiguationPopupDIB)
 
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(handled = RenderWidget::OnMessageReceived(message))
@@ -5892,3 +5909,101 @@ void RenderViewImpl::OnJavaBridgeInit() {
   java_bridge_dispatcher_ = new JavaBridgeDispatcher(this);
 #endif
 }
+
+// Compute the scaling factor to ensure the smallest touch candidate reaches

[darin (slow to review), 2012/08/30 17:39:23]

render_view_impl.cc is already packed with code for too many different things. 
it is a cross-roads of hell type of file.

can you please break this implementation code out into a helper file?  maybe it
is just all of these calculations that would move out.  ideally, you'd reduce
the RenderViewImpl::handleDisambiguationPopup down to the bits of code that deal
with webwidget_ and send the IPC.

[trchen, 2012/08/30 21:15:22]

That sounds like a great idea. I think splitting into a helper class also helps
individual platform to override popup size calculation logic. (the current one
is too Android-specific)

+// a certain clickable size after zooming
+static float FindOptimalScaleFactor(const WebVector<WebRect>& target_rects) {
+  using std::min;
+  using std::max;
+  if (!target_rects.size()) // shall never reach
+    return kDisambiguationPopupMinScale;
+  int smallest_target = min(target_rects[0].width, target_rects[0].height);
+  for (size_t i = 1; i < target_rects.size(); i++) {
+    smallest_target = min(smallest_target, target_rects[i].width);
+    smallest_target = min(smallest_target, target_rects[i].height);
+  }
+  smallest_target = max(smallest_target, 1);
+  return min(kDisambiguationPopupMaxScale, max(kDisambiguationPopupMinScale,
+      static_cast<float>(kDisambiguationPopupMinimumTouchSize)
+          / smallest_target));
+}
+
+static void TrimEdges(int *e1, int *e2, int max_combined) {
+  if (*e1 + *e2 <= max_combined)
+    return;
+
+  if (std::min(*e1, *e2) * 2 >= max_combined) {
+    *e1 = *e2 = max_combined / 2;
+  } else if (*e1 > *e2) {
+    *e1 = max_combined - *e2;
+  } else
+    *e2 = max_combined - *e1;
+}
+
+// Ensure the disambiguation popup fits inside the screen,
+// clip the edges farthest to the touch point if needed.
+static gfx::Rect CropZoomArea(const gfx::Rect& zoom_rect,
+                               const gfx::Size& viewport_size,
+                               const gfx::Point& touch_point,
+                               float scale) {
+    gfx::Size max_size = viewport_size;
+    max_size.Enlarge(-2 * kDisambiguationPopupBoundsMargin,
+                     -2 * kDisambiguationPopupBoundsMargin);
+    max_size = max_size.Scale(1.0 / scale);
+
+    int left = touch_point.x() - zoom_rect.x();
+    int right = zoom_rect.right() - touch_point.x();
+    int top = touch_point.y() - zoom_rect.y();
+    int bottom = zoom_rect.bottom() - touch_point.y();
+    TrimEdges(&left, &right, max_size.width());
+    TrimEdges(&top, &bottom, max_size.height());
+
+    return gfx::Rect(touch_point.x() - left,
+                      touch_point.y() - top,
+                      left + right,
+                      top + bottom);
+}
+
+bool RenderViewImpl::handleDisambiguationPopup(

[darin (slow to review), 2012/08/30 17:39:23]

why is this function named "handleDisambiguationPopup"?  maybe it should be
named "showDisambiguationPopup"?  maybe the IPC message should be similarly
named?

what i'm not clear about is whether you are trying to say that a popup must be
shown or if you are just trying to tell the browser-side that multiple targets
were touched.  if the latter, then the WebKit API method should be named
consistently.

[trchen, 2012/08/30 21:15:22]

Yea... I agree the naming is confusing here.

Up to this point, the disambiguation popup can still be cancelled. If the
embedder doesn't want to implement the popup, it can simply return false in the
handler. (The default behavior provided by WebViewClient)

After the IPC has been sent to the browser process, there is no way to fallback
to default behavior, since the gesture event has already been swallowed.

I think you're right that the WebKit API should be named multipleTargetTapped
and the IPC should be renamed to ViewHostMsg_ShowDisambiguationPopup. The
confusing name is inherited from our old code.

+    const WebKit::WebGestureEvent& event,
+    const WebVector<WebRect>& target_rects) {
+  gfx::Rect zoom_rect = event.boundingBox;
+  for (size_t i = 0; i < target_rects.size(); i++)
+    zoom_rect = zoom_rect.Union(gfx::Rect(target_rects[i]));
+  zoom_rect.Inset(-kDisambiguationPopupPadding, -kDisambiguationPopupPadding);
+  zoom_rect = zoom_rect.Intersect(gfx::Rect(GetSize()));
+
+  float scale = FindOptimalScaleFactor(target_rects);
+  zoom_rect = CropZoomArea(
+      zoom_rect, GetSize(), gfx::Point(event.x, event.y), scale);
+
+  gfx::Size canvas_size = zoom_rect.size();
+  canvas_size = canvas_size.Scale(scale);
+
+  TransportDIB* transport_dib = NULL;
+  {
+    scoped_ptr<skia::PlatformCanvas> canvas(
+        RenderProcess::current()->GetDrawingCanvas(&transport_dib,
+                                                   gfx::Rect(canvas_size)));
+    if (!canvas.get())
+      return false;
+
+    canvas->scale(scale, scale);
+
+    canvas->translate(-zoom_rect.x(), -zoom_rect.y());
+    webwidget_->paint(webkit_glue::ToWebCanvas(canvas.get()), zoom_rect,
+        WebWidget::ForceSoftwareRenderingAndIgnoreGPUResidentContent);
+  }
+  Send(new ViewHostMsg_MultipleTargetsTouched(routing_id_,
+                                              zoom_rect,
+                                              canvas_size,
+                                              transport_dib->id()));
+
+  return true;
+}
+
+void RenderViewImpl::OnReleaseDisambiguationPopupDIB(
+    TransportDIB::Handle dib_handle) {
+  TransportDIB* dib = TransportDIB::CreateWithHandle(dib_handle);
+  RenderProcess::current()->ReleaseTransportDIB(dib);
+}