Add piping of ShowUnhandledTapUIIfNeeded from Blink.

This change picks up notification from Blink that there was an unhandled tap that may require some UI response, analyzes some signals, and conditionally propagates it through Chrome. The notification is done through a new interface: ContextualSearchClient instead of using ContentViewClient. The methods in ContentViewClient have been deprecated and will be removed soon. Also update the ContentViewCore#onSelectionEvent position parameters to use integer device pixels instead of float dips. The associated Blink CL is 819563004. This feature will be used by Contextual Search. BUG=412642, 395128, 383502, 403001 Review URL: https://codereview.chromium.org/816953004 Cr-Commit-Position: refs/heads/master@{#311938}
Denger-Network · Jan 16, 2015 · a070f3c · a070f3c
1 parent 3ab6a57
commit a070f3c
Show file tree

Hide file tree

Showing 11 changed files with 149 additions and 32 deletions.
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
@@ -596,8 +596,9 @@ void ContentViewCoreImpl::OnSelectionEvent(ui::SelectionEventType event,
   ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
   if (j_obj.is_null())
     return;
-  Java_ContentViewCore_onSelectionEvent(
-      env, j_obj.obj(), event, position.x(), position.y());
+  Java_ContentViewCore_onSelectionEvent(env, j_obj.obj(), event,
+                                        position.x() * dpi_scale(),
+                                        position.y() * dpi_scale());
 }
 
 scoped_ptr<ui::TouchHandleDrawable>
@@ -624,9 +625,9 @@ void ContentViewCoreImpl::ShowPastePopup(int x_dip, int y_dip) {
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
-  Java_ContentViewCore_showPastePopupWithFeedback(env, obj.obj(),
-                                                  static_cast<jint>(x_dip),
-                                                  static_cast<jint>(y_dip));
+  Java_ContentViewCore_showPastePopupWithFeedback(
+      env, obj.obj(), static_cast<jint>(x_dip * dpi_scale()),
+      static_cast<jint>(y_dip * dpi_scale()));
 }
 
 void ContentViewCoreImpl::GetScaledContentBitmap(
@@ -1319,6 +1320,16 @@ void ContentViewCoreImpl::RequestTextSurroundingSelection(
   }
 }
 
+void ContentViewCoreImpl::OnShowUnhandledTapUIIfNeeded(int x_dip, int y_dip) {
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
+  if (obj.is_null())
+    return;
+  Java_ContentViewCore_onShowUnhandledTapUIIfNeeded(
+      env, obj.obj(), static_cast<jint>(x_dip * dpi_scale()),
+      static_cast<jint>(y_dip * dpi_scale()));
+}
+
 void ContentViewCoreImpl::OnSmartClipDataExtracted(
     const base::string16& text,
     const base::string16& html,

diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h
@@ -279,6 +279,8 @@ class ContentViewCoreImpl : public ContentViewCore,
   void SelectBetweenCoordinates(const gfx::PointF& base,
                                 const gfx::PointF& extent);
 
+  void OnShowUnhandledTapUIIfNeeded(int x_dip, int y_dip);
+
  private:
   class ContentViewUserData;
 

diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -386,6 +386,8 @@ bool RenderWidgetHostViewAndroid::OnMessageReceived(
                         OnTextInputStateChanged)
     IPC_MESSAGE_HANDLER(ViewHostMsg_SmartClipDataExtracted,
                         OnSmartClipDataExtracted)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_ShowUnhandledTapUIIfNeeded,
+                        OnShowUnhandledTapUIIfNeeded)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -648,6 +650,18 @@ void RenderWidgetHostViewAndroid::OnTextSurroundingSelectionResponse(
   text_surrounding_selection_callback_.Reset();
 }
 
+void RenderWidgetHostViewAndroid::OnShowUnhandledTapUIIfNeeded(int x_dip,
+                                                               int y_dip) {
+  if (!content_view_core_)
+    return;
+  // Validate the coordinates are within the viewport.
+  gfx::Size viewport_size = content_view_core_->GetViewportSizeDip();
+  if (x_dip < 0 || x_dip > viewport_size.width() ||
+      y_dip < 0 || y_dip > viewport_size.height())
+    return;
+  content_view_core_->OnShowUnhandledTapUIIfNeeded(x_dip, y_dip);
+}
+
 void RenderWidgetHostViewAndroid::ReleaseLocksOnSurface() {
   if (!frame_evictor_->HasFrame()) {
     DCHECK_EQ(locks_on_frame_count_, 0u);

diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h
@@ -256,6 +256,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   void DismissTextHandles();
   void SetTextHandlesTemporarilyHidden(bool hidden);
   void OnShowingPastePopup(const gfx::PointF& point);
+  void OnShowUnhandledTapUIIfNeeded(int x_dip, int y_dip);
 
   void SynchronousFrameMetadata(
       const cc::CompositorFrameMetadata& frame_metadata);

diff --git a/content/common/view_messages.h b/content/common/view_messages.h
@@ -1624,6 +1624,12 @@ IPC_MESSAGE_ROUTED3(ViewHostMsg_SmartClipDataExtracted,
                     base::string16 /* html */,
                     gfx::Rect /* rect */)
 
+// Notifies that an unhandled tap has occurred at the specified x,y position
+// and that the UI may need to be triggered.
+IPC_MESSAGE_ROUTED2(ViewHostMsg_ShowUnhandledTapUIIfNeeded,
+                    int /* x */,
+                    int /* y */)
+
 #elif defined(OS_MACOSX)
 // Request that the browser load a font into shared memory for us.
 IPC_SYNC_MESSAGE_CONTROL1_3(ViewHostMsg_LoadFont,

diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
@@ -122,31 +122,21 @@ public boolean doesPerformWebSearch() {
 
     /**
      * Notification that the selection has changed.
-     * TODO(donnd): Remove this and instead expose a ContextualSearchClient, crbug.com/403001.
+     * TODO(donnd): Remove this ASAP, once downstream no longer calls this. crbug.com/403001.
      * @param selection The newly established selection.
      */
-    public void onSelectionChanged(String selection) {
-    }
-
-    /**
-     * Notification that a selection or insertion-related event has occurred.
-     * TODO(pedrosimonetti): remove this method once downstream code has been updated to use
-     * the other signature.
-     * @param eventType The selection event type, see {@link SelectionEventType}.
-     */
-    public void onSelectionEvent(int eventType) {
-    }
+    @Deprecated
+    public void onSelectionChanged(String selection) {}
 
     /**
      * Notification that a selection or insertion-related event has occurred.
-     * TODO(donnd): Remove this and instead expose a ContextualSearchClient, crbug.com/403001.
+     * TODO(donnd): Remove this ASAP, once downstream no longer calls this. crbug.com/403001.
      * @param eventType The selection event type, see {@link SelectionEventType}.
      * @param posXPix The x coordinate of the selection start handle.
      * @param posYPix The y coordinate of the selection start handle.
      */
-    public void onSelectionEvent(int eventType, float posXPix, float posYPix) {
-        onSelectionEvent(eventType);
-    }
+    @Deprecated
+    public void onSelectionEvent(int eventType, float posXPix, float posYPix) {}
 
     /**
      * Called when a new content intent is requested to be started.

diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -519,6 +519,9 @@ public static Activity activityFromContext(Context context) {
     // A flag to determine if we enable hover feature or not.
     private Boolean mEnableTouchHover;
 
+    // The client that implements Contextual Search functionality, or null if none exists.
+    private ContextualSearchClient mContextualSearchClient;
+
     /**
      * Constructs a new ContentViewCore. Embedders must call initialize() after constructing
      * a ContentViewCore and before using it.
@@ -1239,6 +1242,14 @@ private void onSingleTapEventAck(boolean consumed, int x, int y) {
         }
     }
 
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onShowUnhandledTapUIIfNeeded(int x, int y) {
+        if (mContextualSearchClient != null) {
+            mContextualSearchClient.showUnhandledTapUIIfNeeded(x, y);
+        }
+    }
+
     /**
      * Called just prior to a tap or press gesture being forwarded to the renderer.
      */
@@ -2108,7 +2119,7 @@ private void hidePastePopup() {
     }
 
     @CalledByNative
-    private void onSelectionEvent(int eventType, float posXDip, float posYDip) {
+    private void onSelectionEvent(int eventType, int x, int y) {
         switch (eventType) {
             case SelectionEventType.SELECTION_SHOWN:
                 mHasSelection = true;
@@ -2138,7 +2149,7 @@ private void onSelectionEvent(int eventType, float posXDip, float posYDip) {
             case SelectionEventType.INSERTION_MOVED:
                 if (mPastePopupMenu == null) break;
                 if (!isScrollInProgress() && mPastePopupMenu.isShowing()) {
-                    showPastePopup((int) posXDip, (int) posYDip);
+                    showPastePopup(x, y);
                 } else {
                     hidePastePopup();
                 }
@@ -2148,7 +2159,7 @@ private void onSelectionEvent(int eventType, float posXDip, float posYDip) {
                 if (mWasPastePopupShowingOnInsertionDragStart)
                     hidePastePopup();
                 else
-                    showPastePopup((int) posXDip, (int) posYDip);
+                    showPastePopup(x, y);
                 break;
 
             case SelectionEventType.INSERTION_CLEARED:
@@ -2166,8 +2177,12 @@ private void onSelectionEvent(int eventType, float posXDip, float posYDip) {
                 assert false : "Invalid selection event type.";
         }
 
-        final float scale = mRenderCoordinates.getDeviceScaleFactor();
-        getContentViewClient().onSelectionEvent(eventType, posXDip * scale, posYDip * scale);
+        // TODO(donnd): remove this line once downstream changes from using the ContentViewClient to
+        // using the CoontextualSearchClient.  See crbug.com/403001.
+        getContentViewClient().onSelectionEvent(eventType, x, y);
+        if (mContextualSearchClient != null) {
+            mContextualSearchClient.onSelectionEvent(eventType, x, y);
+        }
     }
 
     private void dismissTextHandles() {
@@ -2402,25 +2417,28 @@ public boolean isScrollInProgress() {
     @CalledByNative
     private void onSelectionChanged(String text) {
         mLastSelectedText = text;
+        // TODO(donnd): remove this line once downstream changes from using the ContentViewClient to
+        // using the CoontextualSearchClient.  See crbug.com/403001.
         getContentViewClient().onSelectionChanged(text);
+        if (mContextualSearchClient != null) {
+            mContextualSearchClient.onSelectionChanged(text);
+        }
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
-    private void showPastePopupWithFeedback(int xDip, int yDip) {
+    private void showPastePopupWithFeedback(int x, int y) {
         // TODO(jdduke): Remove this when there is a better signal that long press caused
         // showing of the paste popup. See http://crbug.com/150151.
-        if (showPastePopup(xDip, yDip)) {
+        if (showPastePopup(x, y)) {
             mContainerView.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS);
         }
     }
 
-    private boolean showPastePopup(int xDip, int yDip) {
+    private boolean showPastePopup(int x, int y) {
         if (!mHasInsertion || !canPaste()) return false;
         final float contentOffsetYPix = mRenderCoordinates.getContentOffsetYPix();
-        getPastePopup().showAt(
-                (int) mRenderCoordinates.fromDipToPix(xDip),
-                (int) (mRenderCoordinates.fromDipToPix(yDip) + contentOffsetYPix));
+        getPastePopup().showAt(x, (int) (y + contentOffsetYPix));
         return true;
     }
 
@@ -3039,6 +3057,14 @@ private boolean isFullscreenRequiredForOrientationLock() {
         return mFullscreenRequiredForOrientationLock;
     }
 
+    /**
+     * Sets the client to use for Contextual Search functionality.
+     * @param contextualSearchClient The client to notify for Contextual Search operations.
+     */
+    public void setContextualSearchClient(ContextualSearchClient contextualSearchClient) {
+        mContextualSearchClient = contextualSearchClient;
+    }
+
     private native WebContents nativeGetWebContentsAndroid(long nativeContentViewCoreImpl);
 
     private native void nativeOnJavaContentViewCoreDestroyed(long nativeContentViewCoreImpl);

diff --git a/content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java b/content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java
@@ -0,0 +1,31 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.content.browser;
+
+/**
+ * Interface to a client that implements Contextual Search handling for the content layer.
+ */
+public interface ContextualSearchClient {
+    /**
+     * Notification that the web content selection has changed, regardless of the causal action.
+     * @param selection The newly established selection.
+     */
+    void onSelectionChanged(String selection);
+
+    /**
+     * Notification that a user-triggered selection or insertion-related event has occurred.
+     * @param eventType The selection event type, see {@link SelectionEventType}.
+     * @param posXPix The x coordinate of the selection start handle.
+     * @param posYPix The y coordinate of the selection start handle.
+     */
+    void onSelectionEvent(int eventType, float posXPix, float posYPix);
+
+    /**
+     * Requests to show the UI for an unhandled tap, if needed.
+     * @param x The x coordinate of the tap.
+     * @param y The y coordinate of the tap.
+     */
+    void showUnhandledTapUIIfNeeded(int x, int y);
+}
diff --git a/...ent/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java b/...ent/public/android/java/src/org/chromium/content_public/browser/GestureStateListener.java
@@ -67,4 +67,10 @@ public void onScrollOffsetOrExtentChanged(int scrollOffsetY, int scrollExtentY)
      * indicating whether or not the gesture was consumed.
      */
     public void onSingleTap(boolean consumed, int x, int y) {}
+
+    /*
+     * Called after a single-tap gesture event was processed by the renderer,
+     * but was not handled.
+     */
+    public void onShowUnhandledTapUIIfNeeded(int x, int y) {}
 }
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
@@ -55,11 +55,13 @@
 #include "skia/ext/platform_canvas.h"
 #include "third_party/WebKit/public/platform/WebCursorInfo.h"
 #include "third_party/WebKit/public/platform/WebGraphicsContext3D.h"
+#include "third_party/WebKit/public/platform/WebPoint.h"
 #include "third_party/WebKit/public/platform/WebRect.h"
 #include "third_party/WebKit/public/platform/WebScreenInfo.h"
 #include "third_party/WebKit/public/platform/WebSize.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/web/WebDeviceEmulationParams.h"
+#include "third_party/WebKit/public/web/WebNode.h"
 #include "third_party/WebKit/public/web/WebPagePopup.h"
 #include "third_party/WebKit/public/web/WebPopupMenu.h"
 #include "third_party/WebKit/public/web/WebPopupMenuInfo.h"
@@ -97,7 +99,9 @@ using blink::WebKeyboardEvent;
 using blink::WebMouseEvent;
 using blink::WebMouseWheelEvent;
 using blink::WebNavigationPolicy;
+using blink::WebNode;
 using blink::WebPagePopup;
+using blink::WebPoint;
 using blink::WebPopupMenu;
 using blink::WebPopupMenuInfo;
 using blink::WebPopupType;
@@ -2127,6 +2131,21 @@ void RenderWidget::resetInputMethod() {
   UpdateCompositionInfo(true);
 }
 
+#if defined(OS_ANDROID)
+void RenderWidget::showUnhandledTapUIIfNeeded(
+    const WebPoint& tapped_position,
+    const WebNode& tapped_node,
+    bool page_changed) {
+  DCHECK(handling_input_event_);
+  bool should_trigger = !page_changed && tapped_node.isTextNode() &&
+                        !tapped_node.isContentEditable();
+  if (should_trigger) {
+    Send(new ViewHostMsg_ShowUnhandledTapUIIfNeeded(routing_id_,
+        tapped_position.x, tapped_position.y));
+  }
+}
+#endif
+
 void RenderWidget::didHandleGestureEvent(
     const WebGestureEvent& event,
     bool event_cancelled) {

diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h
@@ -57,6 +57,8 @@ struct WebDeviceEmulationParams;
 class WebGestureEvent;
 class WebKeyboardEvent;
 class WebMouseEvent;
+class WebNode;
+struct WebPoint;
 class WebTouchEvent;
 }
 
@@ -171,6 +173,15 @@ class CONTENT_EXPORT RenderWidget
                                      bool event_cancelled);
   virtual void showImeIfNeeded();
 
+#if defined(OS_ANDROID)
+  // Notifies that a tap was not consumed, so showing a UI for the unhandled
+  // tap may be needed.
+  virtual void showUnhandledTapUIIfNeeded(
+      const blink::WebPoint& tapped_position,
+      const blink::WebNode& tapped_node,
+      bool page_changed) override;
+#endif
+
   // Begins the compositor's scheduler to start producing frames.
   void StartCompositor();