Revert of [TTS] Gather surrounding text on Tap before any UX. (patchset #13...

Revert of [TTS] Gather surrounding text on Tap before any UX. (patchset #13 id:240001 of https://codereview.chromium.org/2211353002/ )

Reason for revert:
Suspected of causing try flakes in ContextualSearchManagerTest#testPromoTapCount:

https://bugs.chromium.org/p/chromium/issues/detail?id=647210

Original issue's description:
> [TTS] Gather surrounding text on Tap before any UX.
>
> Extract the text tapped on to use as a signal in Tap Suppression.
> The text is extracted before any UX is displayed in order to allow the
> tap to be totally ignored when appropriate.  Feeding the surrounding
> text into the logic of TTS will be done separately.
>
> This adds several files that are part of the 2016-refactoring.
> See crbug.com/624609 and go/cs-refactoring-2016.
>
> This CL is part of the refactoring-2016 effort, see go/cs-refactoring-2016
> for details.
>
> BUG=634136, 624609
>
> Committed: https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae
> Cr-Commit-Position: refs/heads/master@{#418464}

TBR=twellington@chromium.org,pedrosimonetti@chromium.org,tedchoc@chromium.org,donnd@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=634136, 624609

Review-Url: https://codereview.chromium.org/2348443002
Cr-Commit-Position: refs/heads/master@{#418873}

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java

-// Copyright 2016 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.chrome.browser.contextualsearch.action;
-
-import org.chromium.chrome.browser.contextualsearch.gesture.SearchGestureHost;
-
-/**
- * Represents a Search Action that will gather and examine surrounding text in order to
- * "resolve" what to search for.
- * Right now this class is just being used to extract surrounding context for a tap
- * action before issuing the resolution request to determine if we want to suppress
- * the tap. Longer term it will actually do the resolution request & search request.
- * This is part of the 2016-refactoring (crbug.com/624609, go/cs-refactoring-2016).
- */
-public class ResolvedSearchAction extends SearchAction {
-    // ============================================================================================
-    // Constructor
-    // ============================================================================================
-
-    /**
-     * Constructs a Search Action that extracts context for a Tap, using the given listener and
-     * host. Current implementation is limited to gathering the text surrounding a Tap gesture in
-     * order to determine whether a search should be done or not.
-     * @param listener The object to notify when the {@link SearchAction} state changes.
-     * @param host The host that provides environment data to the {@link SearchAction}.
-     */
-    public ResolvedSearchAction(SearchActionListener listener, SearchGestureHost host) {
-        super(listener, host);
-    }
-
-    // ============================================================================================
-    // Abstract implementations
-    // ============================================================================================
-
-    @Override
-    public void extractContext() {
-        requestSurroundingText();
-    }
-
-    // ============================================================================================
-    // State handling
-    // ============================================================================================
-
-    @Override
-    protected void onSurroundingTextReady() {
-        super.onSurroundingTextReady();
-
-        notifyContextReady();
-    }
-}

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchAction.java

-// Copyright 2016 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.chrome.browser.contextualsearch.action;
-
-import org.chromium.base.annotations.CalledByNative;
-import org.chromium.chrome.browser.contextualsearch.gesture.SearchGestureHost;
-import org.chromium.content_public.browser.WebContents;
-
-/**
- * Represents an abstract action to do a Contextual Search, and supports native C++ functionality.
- * Subclasses will exist for a Resolved search action that determines the search based on page text,
- * and Verbatim search action that just searches for the literal selection without providing
- * context.
- * This is part of the 2016-refactoring (crbug.com/624609, go/cs-refactoring-2016).
- */
-public abstract class SearchAction {
-    private long mNativePointer;
-
-    protected final SearchActionListener mListener;
-    protected final SearchGestureHost mHost;
-
-    // ============================================================================================
-    // Constructor
-    // ============================================================================================
-
-    /**
-     * Constructs an action that knows how to Search.  Current implementation is limited to
-     * gathering the text on a Tap gesture in order to determine whether the Tap should be
-     * suppressed or a search should be done or not, implemented by the
-     * {@class ResolvedSearchAction} subclass.
-     * @param listener The object to notify when the {@link SearchAction} state changes.
-     * @param host The host object, which provides environment data.
-     */
-    public SearchAction(SearchActionListener listener, SearchGestureHost host) {
-        mHost = host;
-        mNativePointer = nativeInit();
-
-        mListener = listener;
-    }
-
-    // ============================================================================================
-    // Abstract
-    // ============================================================================================
-
-    /**
-     * Extracts the context for the current search -- text surrounding the location of the Tap
-     * gesture.
-     */
-    public abstract void extractContext();
-
-    // ============================================================================================
-    //
-    // ============================================================================================
-
-    /**
-     * Called when the system determines that this action will not be acted upon.
-     */
-    public void dismissAction() {
-        mHost.dismissGesture();
-    }
-
-    /**
-     * Should be called when this object is no longer needed to clean up storage.
-     */
-    public void destroyAction() {
-        onActionEnded();
-
-        if (mNativePointer != 0L) {
-            nativeDestroy(mNativePointer);
-        }
-    }
-
-    // ============================================================================================
-    // Suppression
-    // ============================================================================================
-
-    /**
-     * @return Whether this action should be suppressed.
-     */
-    protected boolean shouldSuppressAction() {
-        // TODO(donnd): integrate with native tap suppression.
-        return false;
-    }
-
-    // ============================================================================================
-    // State notification
-    // ============================================================================================
-
-    /**
-     * Sends notification that the context is ready for use now.
-     */
-    protected void notifyContextReady() {
-        onContextReady();
-    }
-
-    // ============================================================================================
-    // Surrounding Text
-    // ============================================================================================
-
-    /**
-     * Requests text surrounding the location of the caret.
-     */
-    protected void requestSurroundingText() {
-        WebContents webContents = mHost.getTabWebContents();
-        if (webContents != null) {
-            nativeRequestSurroundingText(mNativePointer, webContents);
-            // TODO(donnd): consider reusing this surrounding text for the resolve action too.
-            // Currently we make an additional request for the surroundings after the UX selects the
-            // word tapped, in order to resolve the search term based on that selection.
-        } else {
-            notifyContextReady();
-        }
-    }
-
-    @CalledByNative
-    protected void onSurroundingTextReady() {
-        // No base class action here, subclass may override and take action.
-    }
-
-    // ============================================================================================
-    // SearchAction states
-    // ============================================================================================
-
-    /**
-     * Called to notify that the current context is ready.
-     */
-    private void onContextReady() {
-        mListener.onContextReady(this);
-
-        if (shouldSuppressAction()) {
-            onActionSuppressed();
-        } else {
-            onActionAccepted();
-        }
-    }
-
-    /**
-     * Called when an action has been accepted to notify the listener.
-     */
-    private void onActionAccepted() {
-        mListener.onActionAccepted(this);
-    }
-
-    /**
-     * Called when an action has been suppressed to notify the listener.
-     */
-    private void onActionSuppressed() {
-        mListener.onActionSuppressed(this);
-
-        dismissAction();
-    }
-
-    /**
-     * Called when an action has ended to notify the listener.
-     */
-    private void onActionEnded() {
-        mListener.onActionEnded(this);
-    }
-
-    // ============================================================================================
-    // Internals
-    // ============================================================================================
-
-    @CalledByNative
-    private void clearNativePointer() {
-        assert mNativePointer != 0;
-        mNativePointer = 0;
-    }
-
-    // ============================================================================================
-    // Native methods.
-    // ============================================================================================
-
-    // Native calls.
-    private native long nativeInit();
-    private native void nativeDestroy(long nativeSearchAction);
-
-    private native void nativeRequestSurroundingText(
-            long nativeSearchAction, WebContents webContents);
-}

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchActionListener.java

-// Copyright 2016 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.chrome.browser.contextualsearch.action;
-
-/**
- * Listens to notifications sent by the {@link SearchAction} class.
- * This is part of the 2016-refactoring (crbug.com/624609, go/cs-refactoring-2016).
- */
-public class SearchActionListener {
-    /**
-     * Indicates that the Contextual Search Context, which includes the selected word
-     * and surrounding text, is ready to be accessed.
-     * @param action The {@link SearchAction} whose context is ready.
-     */
-    protected void onContextReady(SearchAction action) {}
-
-    /**
-     * Indicates that the {@code SearchAction} has been accepted (not suppressed).
-     * @param action The {@link SearchAction} that has been accepted.
-     */
-    protected void onActionAccepted(SearchAction action) {}
-
-    /**
-     * Indicates that the {@code SearchAction} has been suppressed (not accepted).
-     * @param action The {@link SearchAction} that has been suppressed.
-     */
-    protected void onActionSuppressed(SearchAction action) {}
-
-    /**
-     * Indicates that the {@code SearchAction} has ended.
-     * @param action The {@link SearchAction} that has ended.
-     */
-    protected void onActionEnded(SearchAction action) {}
-}

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/gesture/SearchGestureHost.java

-// Copyright 2016 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.chrome.browser.contextualsearch.gesture;
-
-import org.chromium.content_public.browser.WebContents;
-
-/**
- * Provides the host interface for Search Gestures, which can create Search Actions.
- * When a gesture is recognized that it should trigger a search, this interface is used
- * to communicate back to the host
- * This is part of the 2016-refactoring (crbug.com/624609, go/cs-refactoring-2016).
- */
-public interface SearchGestureHost {
-    /**
-     * @return the {@link WebContents} for the current base tab.
-     */
-    WebContents getTabWebContents();
-
-    /**
-     * Tells the host that the current gesture should be dismissed because processing is complete.
-     */
-    void dismissGesture();
-}

chrome/android/java_sources.gni

@@ -221,10 +221,6 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/contextmenu/ContextMenuParams.java",
   "java/src/org/chromium/chrome/browser/contextmenu/ContextMenuPopulator.java",
   "java/src/org/chromium/chrome/browser/contextmenu/ContextMenuTitleView.java",
-  "java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java",
-  "java/src/org/chromium/chrome/browser/contextualsearch/action/SearchAction.java",
-  "java/src/org/chromium/chrome/browser/contextualsearch/action/SearchActionListener.java",
-  "java/src/org/chromium/chrome/browser/contextualsearch/gesture/SearchGestureHost.java",
   "java/src/org/chromium/chrome/browser/contextualsearch/BarOverlapTapSuppression.java",
   "java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchBlacklist.java",
   "java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java",

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java

@@ -806,15 +806,48 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
     }
 
     /**
-     * Click to cause the panel to show, tap the Bar to expand, then close.
+     * Taps the base page near the top.
+     */
+    private void tapBasePageToClosePanel() throws InterruptedException {
+        // TODO(pedrosimonetti): This is not reliable. Find a better approach.
+        // We use the far right side (x == 0.9f) to prevent simulating a tap on top of an
+        // existing long-press selection (the pins are a tap target). This might not work on RTL.
+        // We are using y == 0.35f because otherwise it will fail for long press cases.
+        // It might be better to get the position of the Panel and tap just about outside
+        // the Panel. I suspect some Flaky tests are caused by this problem (ones involving
+        // long press and trying to close with the bar peeking, with a long press selection
+        // established).
+        tapBasePage(0.9f, 0.35f);
+        waitForPanelToClose();
+    }
+
+    /**
+     * Taps the base page at the given x, y position.
+     */
+    private void tapBasePage(float x, float y) {
+        View root = getActivity().getWindow().getDecorView().getRootView();
+        x *= root.getWidth();
+        y *= root.getHeight();
+        TouchCommon.singleClickView(root, (int) x, (int) y);
+    }
+
+    /**
+     * Click various places to cause the panel to show, expand, then close.
      */
     private void clickToExpandAndClosePanel() throws InterruptedException, TimeoutException {
         clickWordNode("states");
-        tapPeekingBarToExpandAndAssert();
-        closePanel();
+        tapBarToExpandAndClosePanel();
         waitForSelectionDissolved();
     }
 
+    /**
+     * Tap on the peeking Bar to expand the panel, then taps on the base page to close it.
+     */
+    private void tapBarToExpandAndClosePanel() throws InterruptedException {
+        tapPeekingBarToExpandAndAssert();
+        tapBasePageToClosePanel();
+    }
+
     /**
      * Generate a click in the panel's bar.
      * TODO(donnd): Replace this method with panelBarClick since this appears to be unreliable.
@@ -1077,8 +1110,8 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertLoadedNormalPriorityUrl();
         assertEquals(1, mFakeServer.getLoadedUrlCount());
 
-        // close the panel.
-        closePanel();
+        // tap the base page to close.
+        tapBasePageToClosePanel();
         assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertNoContentViewCore();
     }
@@ -1703,7 +1736,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         pressAppMenuKey();
         assertAppMenuVisibility(false);
 
-        closePanel();
+        tapBasePageToClosePanel();
 
         pressAppMenuKey();
         assertAppMenuVisibility(true);
@@ -1869,7 +1902,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
 
     /**
      * Tests that ContextualSearchObserver gets notified when user brings up contextual search
-     * panel via long press and when the panel is dismissed.
+     * panel via long press and then dismisses the panel by tapping on the base page.
      */
     @SmallTest
     @Feature({"ContextualSearch"})
@@ -1882,13 +1915,13 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         longPressNode("states");
         assertEquals(0, observer.hideCount);
 
-        closePanel();
+        tapBasePageToClosePanel();
         assertEquals(1, observer.hideCount);
     }
 
     /**
      * Tests that ContextualSearchObserver gets notified when user brings up contextual search
-     * panel via tap and when the panel is dismissed.
+     * panel via tap and then dismisses the panel by tapping on the base page.
      */
     @SmallTest
     @Feature({"ContextualSearch"})
@@ -1899,7 +1932,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         clickWordNode("states");
         assertEquals(0, observer.hideCount);
 
-        closePanel();
+        tapBasePageToClosePanel();
         assertEquals(1, observer.hideCount);
     }
 
@@ -2122,7 +2155,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertFalse(mPanel.isPeekPromoVisible());
 
         // After closing the Panel the Promo should still be invisible.
-        closePanel();
+        tapBasePageToClosePanel();
         assertFalse(mPanel.isPeekPromoVisible());
 
         // Click elsewhere to clear the selection.
@@ -2156,7 +2189,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertContentViewCoreVisible();
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
     }
 
@@ -2179,7 +2212,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertContentViewCoreVisible();
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
     }
 
@@ -2213,7 +2246,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
         assertEquals(1, mFakeServer.getLoadedUrlCount());
     }
@@ -2249,7 +2282,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
         assertEquals(1, mFakeServer.getLoadedUrlCount());
     }
@@ -2288,7 +2321,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertNotSame(cvc2, cvc3);
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
         assertEquals(3, mFakeServer.getLoadedUrlCount());
     }
@@ -2334,7 +2367,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertNotSame(cvc1, cvc2);
 
         // Closing the Panel should destroy the Content.
-        closePanel();
+        tapBasePageToClosePanel();
         assertNoContentViewCore();
         assertEquals(2, mFakeServer.getLoadedUrlCount());
     }
@@ -2389,7 +2422,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         String url = mFakeServer.getLoadedUrl();
 
         // Close the Panel without seeing the Content.
-        closePanel();
+        tapBasePageToClosePanel();
 
         // Now check that the URL has been removed from history.
         assertTrue(mFakeServer.hasRemovedUrl(url));
@@ -2412,7 +2445,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         tapPeekingBarToExpandAndAssert();
 
         // Close the Panel.
-        closePanel();
+        tapBasePageToClosePanel();
 
         // Now check that the URL has not been removed from history, since the Content was seen.
         assertFalse(mFakeServer.hasRemovedUrl(url));
@@ -2447,7 +2480,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertNotSame(url2, url3);
 
         // Close the Panel without seeing any Content.
-        closePanel();
+        tapBasePageToClosePanel();
 
         // Now check that all three URLs have been removed from history.
         assertEquals(3, mFakeServer.getLoadedUrlCount());
@@ -2565,14 +2598,15 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
 
     /**
      * Tests that Contextual Search works in fullscreen. Specifically, tests that tapping a word
-     * peeks the panel, expanding the bar results in the bar ending at the correct spot in the page.
+     * peeks the panel, expanding the bar results in the bar ending at the correct spot in the page
+     * and tapping the base page closes the panel.
      */
     @SmallTest
     @Feature({"ContextualSearch"})
     @Restriction({ChromeRestriction.RESTRICTION_TYPE_PHONE, RESTRICTION_TYPE_NON_LOW_END_DEVICE})
     public void testTapContentAndExpandPanelInFullscreen()
             throws InterruptedException, TimeoutException {
-        // Toggle tab to fullscreen.
+        // Toggle tab to fulllscreen.
         FullscreenTestUtils.togglePersistentFullscreenAndAssert(getActivity().getActivityTab(),
                 true, getActivity());
 
@@ -2583,6 +2617,9 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         tapPeekingBarToExpandAndAssert();
         assertEquals(mManager.getContextualSearchPanel().getHeight(),
                 mManager.getContextualSearchPanel().getPanelHeightFromState(PanelState.EXPANDED));
+
+        // Tap the base page and assert that the panel is closed.
+        tapBasePageToClosePanel();
     }
 
     /**

chrome/browser/BUILD.gn

@@ -2941,8 +2941,6 @@ split_static_library("browser") {
       "android/contextualsearch/contextual_search_tab_helper.h",
       "android/contextualsearch/resolved_search_term.cc",
       "android/contextualsearch/resolved_search_term.h",
-      "android/contextualsearch/search_action.cc",
-      "android/contextualsearch/search_action.h",
       "android/cookies/cookies_fetcher.cc",
       "android/cookies/cookies_fetcher.h",
       "android/data_usage/data_use_matcher.cc",
@@ -3742,7 +3740,6 @@ if (android_java_ui) {
       "../android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuParams.java",
       "../android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java",
       "../android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTabHelper.java",
-      "../android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchAction.java",
       "../android/java/src/org/chromium/chrome/browser/cookies/CookiesFetcher.java",
       "../android/java/src/org/chromium/chrome/browser/database/SQLiteCursor.java",
       "../android/java/src/org/chromium/chrome/browser/datausage/DataUseTabUIManager.java",

chrome/browser/android/chrome_jni_registrar.cc

@@ -35,7 +35,6 @@
 #include "chrome/browser/android/compositor/tab_content_manager.h"
 #include "chrome/browser/android/contextualsearch/contextual_search_manager.h"
 #include "chrome/browser/android/contextualsearch/contextual_search_tab_helper.h"
-#include "chrome/browser/android/contextualsearch/search_action.h"
 #include "chrome/browser/android/cookies/cookies_fetcher.h"
 #include "chrome/browser/android/data_usage/data_use_tab_ui_manager_android.h"
 #include "chrome/browser/android/data_usage/external_data_use_observer_bridge.h"
@@ -333,7 +332,6 @@ static base::android::RegistrationMethod kChromeRegisteredMethods[] = {
      remote_media::RemoteMediaPlayerBridge::RegisterRemoteMediaPlayerBridge},
     {"RevenueStats", RegisterRevenueStats},
     {"SafeBrowsingApiBridge", safe_browsing::RegisterSafeBrowsingApiBridge},
-    {"SearchAction", RegisterSearchAction},
     {"SceneLayer", chrome::android::RegisterSceneLayer},
     {"ScreenshotTask", RegisterScreenshotTask},
     {"ServiceTabLauncher", ServiceTabLauncher::Register},

chrome/browser/android/contextualsearch/contextual_search_context.cc

@@ -9,18 +9,11 @@ ContextualSearchContext::ContextualSearchContext(
     const bool use_resolved_search_term,
     const GURL& page_url,
     const std::string& encoding)
-    : use_resolved_search_term(use_resolved_search_term),
+    : selected_text(selected_text),
+      use_resolved_search_term(use_resolved_search_term),
       page_url(page_url),
-      encoding(encoding),
-      selected_text(selected_text) {}
-
-ContextualSearchContext::ContextualSearchContext(
-    const bool use_resolved_search_term,
-    const GURL& page_url,
-    const std::string& encoding)
-    : use_resolved_search_term(use_resolved_search_term),
-      page_url(page_url),
-      encoding(encoding) {}
+      encoding(encoding) {
+}
 
 ContextualSearchContext::~ContextualSearchContext() {
 }

chrome/browser/android/contextualsearch/contextual_search_context.h

@@ -18,16 +18,13 @@ struct ContextualSearchContext {
                           const bool use_resolved_search_term,
                           const GURL& page_url,
                           const std::string& encoding);
-  ContextualSearchContext(const bool use_resolved_search_term,
-                          const GURL& page_url,
-                          const std::string& encoding);
   ~ContextualSearchContext();
 
+  const std::string selected_text;
   const bool use_resolved_search_term;
   const GURL page_url;
   const std::string encoding;
 
-  std::string selected_text;
   base::string16 surrounding_text;
   int start_offset;
   int end_offset;

chrome/browser/android/contextualsearch/search_action.cc

-// Copyright 2016 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.
-
-#include "chrome/browser/android/contextualsearch/search_action.h"
-
-#include <set>
-
-#include "base/android/jni_string.h"
-#include "base/bind.h"
-#include "base/memory/weak_ptr.h"
-#include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/android/contextualsearch/contextual_search_context.h"
-#include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/web_contents.h"
-#include "jni/SearchAction_jni.h"
-#include "url/gurl.h"
-
-using base::android::ConvertUTF8ToJavaString;
-using base::android::JavaParamRef;
-using base::android::ScopedJavaGlobalRef;
-using content::WebContents;
-
-namespace {
-// Context length is set to 1.5K characters for historical reasons:
-// This provided the largest context that would fit in a GET request along with
-// our other required parameters."
-const int kSurroundingTextSize = 1536;
-}
-
-SearchAction::SearchAction(JNIEnv* env, jobject obj) {
-  java_object_.Reset(env, obj);
-}
-
-SearchAction::~SearchAction() {
-  // The Java object can be null in tests only, by calling the default
-  // constructor.
-  if (!java_object_.is_null()) {
-    JNIEnv* env = base::android::AttachCurrentThread();
-    Java_SearchAction_clearNativePointer(env, java_object_.obj());
-  }
-}
-
-void SearchAction::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
-  delete this;
-}
-
-void SearchAction::RequestSurroundingText(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& obj,
-    const JavaParamRef<jobject>& j_web_contents) {
-  WebContents* web_contents = WebContents::FromJavaWebContents(j_web_contents);
-  DCHECK(web_contents);
-
-  GURL url(web_contents->GetURL());
-  std::string encoding(web_contents->GetEncoding());
-  search_context_.reset(new ContextualSearchContext(true, url, encoding));
-
-  content::RenderFrameHost* focused_frame = web_contents->GetFocusedFrame();
-  if (!focused_frame) {
-    OnSurroundingTextResponse(base::string16(), 0, 0);
-  } else {
-    focused_frame->RequestTextSurroundingSelection(
-        base::Bind(&SearchAction::OnSurroundingTextResponse, AsWeakPtr()),
-        kSurroundingTextSize);
-  }
-}
-
-void SearchAction::OnSurroundingTextResponse(
-    const base::string16& surrounding_text,
-    int focus_start,
-    int focus_end) {
-  // Record the context, which can be accessed later on demand.
-  search_context_->surrounding_text = surrounding_text;
-  search_context_->start_offset = focus_start;
-  search_context_->end_offset = focus_end;
-
-  // Notify Java.
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SearchAction_onSurroundingTextReady(env, java_object_.obj());
-}
-
-std::string SearchAction::FindFocusedWord() {
-  DCHECK(search_context_);
-
-  const base::string16& surrounding_text = search_context_->surrounding_text;
-  int focus_start = search_context_->start_offset;
-  int focus_end = search_context_->end_offset;
-  int surrounding_text_length = surrounding_text.length();
-
-  // First, we need to find the focused word boundaries.
-  int focused_word_start = focus_start;
-  int focused_word_end = focus_end;
-
-  if (surrounding_text_length > 0 &&
-      IsValidCharacter(surrounding_text[focused_word_start])) {
-    // Find focused word start (inclusive)
-    for (int i = focus_start; i >= 0; i--) {
-      focused_word_start = i;
-
-      wchar_t character;
-      if (i > 0) {
-        character = surrounding_text[i - 1];
-
-        if (!IsValidCharacter(character))
-          break;
-      }
-    }
-
-    // Find focused word end (non inclusive)
-    for (int i = focus_end; i < surrounding_text_length; i++) {
-      wchar_t character = surrounding_text[i];
-
-      if (IsValidCharacter(character)) {
-        focused_word_end = i + 1;
-      } else {
-        focused_word_end = i;
-        break;
-      }
-    }
-  }
-
-  return base::UTF16ToUTF8(surrounding_text.substr(
-      focused_word_start, focused_word_end - focused_word_start));
-}
-
-std::string SearchAction::GetSampleText(int sample_length) {
-  DCHECK(search_context_);
-
-  const base::string16& surrounding_text = search_context_->surrounding_text;
-  int focus_start = search_context_->start_offset;
-  int focus_end = search_context_->end_offset;
-  int surrounding_text_length = surrounding_text.length();
-
-  //---------------------------------------------------------------------------
-  // Cut the surrounding size so it can be sent to the Java side.
-
-  // Start equally distributing the size around the focal point.
-  int focal_point_size = focus_end - focus_start;
-  int sample_margin = (sample_length - focal_point_size) / 2;
-  int sample_start = focus_start - sample_margin;
-  int sample_end = focus_end + sample_margin;
-
-  // If the the start is out of bounds, compensate the end side.
-  if (sample_start < 0) {
-    sample_end -= sample_start;
-    sample_start = 0;
-  }
-
-  // If the the end is out of bounds, compensate the start side.
-  if (sample_end > surrounding_text_length) {
-    int diff = sample_end - surrounding_text_length;
-    sample_end = surrounding_text_length;
-    sample_start -= diff;
-  }
-
-  // Trim the start and end to make sure they are within bounds.
-  sample_start = std::max(0, sample_start);
-  sample_end = std::min(surrounding_text_length, sample_end);
-  return base::UTF16ToUTF8(
-      surrounding_text.substr(sample_start, sample_end - sample_start));
-}
-
-bool SearchAction::IsValidCharacter(int char_code) {
-  // See http://www.unicode.org/Public/UCD/latest/ucd/NamesList.txt
-  // See http://jrgraphix.net/research/unicode_blocks.php
-
-  // TODO(donnd): should not include CJK characters!
-  // TODO(donnd): consider using regular expressions.
-
-  if ((char_code >= 11264 && char_code <= 55295) ||  // Asian language symbols
-      (char_code >= 192 && char_code <= 8191) ||  // Letters in many languages
-      (char_code >= 97 && char_code <= 122) ||    // Lowercase letters
-      (char_code >= 65 && char_code <= 90) ||     // Uppercase letters
-      (char_code >= 48 && char_code <= 57)) {     // Numbers
-    return true;
-  }
-
-  return false;
-}
-
-// Testing
-
-SearchAction::SearchAction() {}
-
-void SearchAction::SetContext(std::string surrounding_text,
-                              int focus_start,
-                              int focus_end) {
-  search_context_.reset(new ContextualSearchContext(false, GURL(), ""));
-  search_context_->surrounding_text = base::UTF8ToUTF16(surrounding_text);
-  search_context_->start_offset = focus_start;
-  search_context_->end_offset = focus_end;
-}
-
-// Boilerplate
-
-bool RegisterSearchAction(JNIEnv* env) {
-  return RegisterNativesImpl(env);
-}
-
-jlong Init(JNIEnv* env, const JavaParamRef<jobject>& obj) {
-  SearchAction* content = new SearchAction(env, obj);
-  return reinterpret_cast<intptr_t>(content);
-}

chrome/browser/android/contextualsearch/search_action.h

-// Copyright 2016 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.
-
-#ifndef CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_SEARCH_ACTION_H_
-#define CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_SEARCH_ACTION_H_
-
-#include <memory>
-
-#include "base/android/jni_android.h"
-#include "base/gtest_prod_util.h"
-#include "base/memory/weak_ptr.h"
-#include "base/strings/string_util.h"
-
-struct ContextualSearchContext;
-
-// Represents the native side of a Java Search Action, which knows how to do a
-// Contextual Search.
-// This is part of the 2016-refactoring (crbug.com/624609,
-// go/cs-refactoring-2016).
-// Gathers text surrounding the selection from the page and makes it accessible
-// to Java.
-// TODO(donnd): add capability to "resolve" the best search term for the section
-// of the page based on a server request or local text analysis.
-class SearchAction : public base::SupportsWeakPtr<SearchAction> {
- public:
-  // Constructs a new Search Action linked to the given Java object.
-  SearchAction(JNIEnv* env, jobject obj);
-  virtual ~SearchAction();
-
-  // Should be called when this native object is no longer needed (calls the
-  // destructor).
-  void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
-
-  // Requests the text surrounding the selection for the given WebContents Java
-  // object.  The surrounding text will be made available through a call to
-  // |OnSurroundingTextResponse|.
-  void RequestSurroundingText(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj,
-      const base::android::JavaParamRef<jobject>& j_web_contents);
-
-  // Finds and returns the focused word within the current surrounding text.
-  // RequestSurroundingText must be called first and the associated response
-  // received.
-  std::string FindFocusedWord();
-
-  // Returns a sample of the current surrounding text of the given
-  // |sample_length| or shorter if insufficient sample text is available.
-  // RequestSurroundingText must be called first and the associated response
-  // received.
-  std::string GetSampleText(int sample_length);
-
- private:
-  friend class SearchActionTest;
-  FRIEND_TEST_ALL_PREFIXES(SearchActionTest, IsValidCharacterTest);
-  FRIEND_TEST_ALL_PREFIXES(SearchActionTest, FindFocusedWordTest);
-  FRIEND_TEST_ALL_PREFIXES(SearchActionTest, SampleSurroundingsTest);
-
-  // Analyzes the surrounding text and makes it available to the Java
-  // SearchAction object in a call to SearchAction#onSurroundingTextResponse.
-  void OnSurroundingTextResponse(const base::string16& surrounding_text,
-                                 int focus_start,
-                                 int focus_end);
-
-  // Determines if the given character is a valid part of a word to search for.
-  bool IsValidCharacter(int char_code);
-
-  // Testing support
-  SearchAction();
-  void SetContext(std::string surrounding_text, int focus_start, int focus_end);
-
-  // The linked Java object.
-  base::android::ScopedJavaGlobalRef<jobject> java_object_;
-
-  // The current search context, or null.
-  std::shared_ptr<ContextualSearchContext> search_context_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchAction);
-};
-
-bool RegisterSearchAction(JNIEnv* env);
-
-#endif  // CHROME_BROWSER_ANDROID_CONTEXTUALSEARCH_SEARCH_ACTION_H_

chrome/browser/android/contextualsearch/search_action_unittest.cc

-// Copyright 2016 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.
-
-#include "chrome/browser/android/contextualsearch/search_action.h"
-
-#include "base/gtest_prod_util.h"
-#include "base/strings/utf_string_conversions.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using base::string16;
-using base::UTF8ToUTF16;
-using std::pair;
-
-// Tests parts of the SearchAction class.
-// This is part of the 2016-refactoring (crbug.com/624609,
-// go/cs-refactoring-2016).
-class SearchActionTest : public testing::Test {
- public:
-  SearchActionTest() {}
-  ~SearchActionTest() override {}
-
-  // The class under test.
-  std::unique_ptr<SearchAction> search_action_;
-
- protected:
-  void SetUp() override { search_action_.reset(new SearchAction()); }
-  void TearDown() override {}
-
-  // Helper to set the context to the given sample and focus on |focus|.
-  void SetContext(std::string sample, std::string focus);
-};
-
-void SearchActionTest::SetContext(std::string sample, std::string focus) {
-  size_t offset = sample.find(focus);
-  ASSERT_NE(offset, std::string::npos);
-  search_action_->SetContext(sample, offset, offset + focus.length());
-}
-
-TEST_F(SearchActionTest, IsValidCharacterTest) {
-  EXPECT_TRUE(search_action_->IsValidCharacter('a'));
-  EXPECT_TRUE(search_action_->IsValidCharacter('A'));
-  EXPECT_TRUE(search_action_->IsValidCharacter('0'));
-
-  EXPECT_FALSE(search_action_->IsValidCharacter(','));
-  EXPECT_FALSE(search_action_->IsValidCharacter(' '));
-  EXPECT_FALSE(search_action_->IsValidCharacter('-'));
-}
-
-TEST_F(SearchActionTest, FindFocusedWordTest) {
-  // Test finding "word" within this sample string.
-  std::string sample = "Sample word, text";
-
-  // Any range inside the word but before the end should return the word.
-  search_action_->SetContext(sample, 7, 7);
-  EXPECT_EQ("word", search_action_->FindFocusedWord());
-  search_action_->SetContext(sample, 10, 10);
-  EXPECT_EQ("word", search_action_->FindFocusedWord());
-  search_action_->SetContext(sample, 7, 11);
-  EXPECT_EQ("word", search_action_->FindFocusedWord());
-
-  // A range just past the word returns an empty string.
-  search_action_->SetContext(sample, 11, 11);
-  EXPECT_EQ("", search_action_->FindFocusedWord());
-}
-
-TEST_F(SearchActionTest, SampleSurroundingsTest) {
-  std::string focus = "focus";
-  std::string sample = "987654321focus123456789";
-
-  // Sample big enough to include both ends.
-  SetContext(sample, focus);
-  EXPECT_EQ(sample, search_action_->GetSampleText(100));
-
-  // Must trim both ends, trimming 6 off each end.
-  EXPECT_EQ("321focus123", search_action_->GetSampleText(12));
-
-  // With focus near the beginning, extra is shifted to the end.
-  SetContext("321focus123456789", focus);
-  EXPECT_EQ("321focus12345", search_action_->GetSampleText(13));
-
-  // With focus near the end, extra is shifted to the beginning.
-  SetContext("987654321focus123", focus);
-  EXPECT_EQ("54321focus123", search_action_->GetSampleText(13));
-
-  // Requesting less than the focus.
-  SetContext("focus", focus);
-  EXPECT_EQ("c", search_action_->GetSampleText(1));
-}