[TTS] Pass "exact resolve" param through to server

Pipes a boolean through Contextual Search to a server request parameter
to determine whether to make the Resolve an "exact resolve" request that
cannot expand the selection. Uses TemplateUrl to add the parameter
to the Resolve request.

This additional parameter is needed for the Long-press triggering
experiment.

Also includes cleanup of initializers in C++ ContextualSearchContext.

BUG=956277

Change-Id: Iaa0e1988035ed49d13e7a9e77e20c0c2ef04b4d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995842
Commit-Queue: Theresa  <twellington@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Auto-Submit: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736584}

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchContext.java

@@ -279,14 +279,10 @@ public abstract class ContextualSearchContext {
     }
 
     /**
-     * Specifies that this resolve must return a non-expanding result.
+     * Specifies that this search must be exact so the resolve must return a non-expanding result.
      */
-    void setRestrictedResolve() {
-        // TODO(donnd): Improve by sending full context plus a boolean.
-        mSurroundingText = mInitialSelectedWord;
-        mSelectionStartOffset = 0;
-        mSelectionEndOffset = mSurroundingText.length();
-        ContextualSearchContextJni.get().restrictResolve(mNativePointer, this);
+    void setExactResolve() {
+        ContextualSearchContextJni.get().setExactResolve(mNativePointer, this);
     }
 
     /**
@@ -584,6 +580,6 @@ public abstract class ContextualSearchContext {
         void setContent(long nativeContextualSearchContext, ContextualSearchContext caller,
                 String content, int selectionStart, int selectionEnd);
         String detectLanguage(long nativeContextualSearchContext, ContextualSearchContext caller);
-        void restrictResolve(long nativeContextualSearchContext, ContextualSearchContext caller);
+        void setExactResolve(long nativeContextualSearchContext, ContextualSearchContext caller);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

@@ -507,10 +507,10 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     }
 
     @Override
-    public void startSearchTermResolutionRequest(String selection, boolean isRestrictedResolve) {
+    public void startSearchTermResolutionRequest(String selection, boolean isExactResolve) {
         WebContents baseWebContents = getBaseWebContents();
         if (baseWebContents != null && mContext != null && mContext.canResolve()) {
-            if (isRestrictedResolve) mContext.setRestrictedResolve();
+            if (isExactResolve) mContext.setExactResolve();
             ContextualSearchManagerJni.get().startSearchTermResolutionRequest(
                     mNativeContextualSearchManagerPtr, this, mContext, getBaseWebContents());
         } else {

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchNetworkCommunicator.java

@@ -16,10 +16,10 @@ interface ContextualSearchNetworkCommunicator {
      * Starts a Search Term Resolution request.
      * When the response comes back {@link #handleSearchTermResolutionResponse} will be called.
      * @param selection the current selected text.
-     * @param isRestrictedResolve Whether the resolution should be restricted to an exact match with
-     *        the given selection.
+     * @param isExactResolve Whether the resolution should be restricted to an exact match with
+     *        the given selection that cannot be expanded based on the response.
      */
-    void startSearchTermResolutionRequest(String selection, boolean isRestrictedResolve);
+    void startSearchTermResolutionRequest(String selection, boolean isExactResolve);
 
     /**
      * Handles a Search Term Resolution response.

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

@@ -59,6 +59,7 @@ class ContextualSearchFakeServer
     private String mSearchTermRequested;
     private boolean mShouldUseHttps;
     private boolean mIsOnline = true;
+    private boolean mIsExactResolve;
 
     private boolean mDidEverCallWebContentsOnShow;
 
@@ -585,6 +586,7 @@ class ContextualSearchFakeServer
         mIsOnline = true;
         mLoadedUrlCount = 0;
         mUseInvalidLowPriorityPath = false;
+        mIsExactResolve = false;
     }
 
     /**
@@ -603,6 +605,11 @@ class ContextualSearchFakeServer
         return mUseInvalidLowPriorityPath && mLoadedUrl.contains("invalid");
     }
 
+    @VisibleForTesting
+    boolean getIsExactResolve() {
+        return mIsExactResolve;
+    }
+
     //============================================================================================
     // History Removal Helpers
     //============================================================================================
@@ -620,9 +627,10 @@ class ContextualSearchFakeServer
     //============================================================================================
 
     @Override
-    public void startSearchTermResolutionRequest(String selection, boolean isRestrictedResolve) {
+    public void startSearchTermResolutionRequest(String selection, boolean isExactResolve) {
         mLoadedUrl = null;
         mSearchTermRequested = selection;
+        mIsExactResolve = isExactResolve;
 
         if (mActiveFakeTapSearch != null) {
             mActiveFakeTapSearch.notifySearchTermResolutionStarted();

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

@@ -106,8 +106,8 @@ import org.chromium.ui.touch_selection.SelectionEventType;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeoutException;
 
-// TODO(pedrosimonetti): Create class with limited API to encapsulate the internals of simulations.
-// TODO(pedrosimonetti): Separate tests into different classes grouped by type of tests. Examples:
+// TODO(donnd): Create class with limited API to encapsulate the internals of simulations.
+// TODO(donnd): Separate tests into different classes grouped by type of tests. Examples:
 // Gestures (Tap, LongPress), Search Term Resolution (resolves, expand selection, prevent preload,
 // translation), Panel interaction (tap, fling up/down, close), Content (creation, loading,
 // visibility, history, delayed load), Tab Promotion, Policy (add tests to check if policies
@@ -132,7 +132,6 @@ public class ContextualSearchManagerTest {
             "/chrome/test/data/android/contextualsearch/tap_test.html";
     private static final int TEST_TIMEOUT = 15000;
     private static final int TEST_EXPECTED_FAILURE_TIMEOUT = 1000;
-    private static final int PLENTY_OF_TAPS = 1000;
 
     // TODO(donnd): get these from TemplateURL once the low-priority or Contextual Search API
     // is fully supported.
@@ -289,6 +288,53 @@ public class ContextualSearchManagerTest {
         waitForPanelToPeek();
     }
 
+    /**
+     * Long-press a node without completing the action, by keeping the touch down by not letting up.
+     * @param nodeId The ID of the node to touch
+     * @return A time stamp to use with {@link #longPressExtendSelection}
+     * @throws TimeoutException
+     * @see #longPressExtendSelection
+     */
+    public long longPressNodeWithoutUp(String nodeId) throws TimeoutException {
+        long downTime = SystemClock.uptimeMillis();
+        Tab tab = mActivityTestRule.getActivity().getActivityTab();
+        DOMUtils.longPressNodeWithoutUp(tab.getWebContents(), nodeId, downTime);
+        waitForSelectActionBarVisible();
+        waitForPanelToPeek();
+        return downTime;
+    }
+
+    /**
+     * Extends a Long-press selection by completing a drag action.
+     * @param startNodeId The ID of the node that has already been touched
+     * @param endNodeId The ID of the node that the touch should be extended to
+     * @param downTime A time stamp returned by {@link #longPressNodeWithoutUp}
+     * @throws TimeoutException
+     * @see #longPressNodeWithoutUp
+     */
+    public void longPressExtendSelection(String startNodeId, String endNodeId, long downTime)
+            throws TimeoutException {
+        // TODO(donnd): figure out why we need this one line here, and why the selection does not
+        // match our expected nodes!
+        longPressNodeWithoutUp("term");
+
+        // Drag to the specified position by a DOM node id.
+        int stepCount = 100;
+        Tab tab = mActivityTestRule.getActivity().getActivityTab();
+        DOMUtils.dragNodeTo(tab.getWebContents(), startNodeId, endNodeId, stepCount, downTime);
+        DOMUtils.dragNodeEnd(tab.getWebContents(), endNodeId, downTime);
+
+        // Make sure the selection controller knows we did a drag.
+        // TODO(donnd): figure out how to reliably simulate a drag on all platforms.
+        float unused = 0.0f;
+        @SelectionEventType
+        int dragStoppedEvent = SelectionEventType.SELECTION_HANDLE_DRAG_STOPPED;
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> mSelectionController.handleSelectionEvent(dragStoppedEvent, unused, unused));
+
+        waitForSelectActionBarVisible();
+    }
+
     /**
      * Simulates a click on the given node.
      * @param nodeId A string containing the node ID.
@@ -435,7 +481,7 @@ public class ContextualSearchManagerTest {
 
     //============================================================================================
     // Fake Response
-    // TODO(pedrosimonetti): remove these methods and use the new infrastructure instead.
+    // TODO(donnd): remove these methods and use the new infrastructure instead.
     //============================================================================================
 
     /**
@@ -560,7 +606,7 @@ public class ContextualSearchManagerTest {
 
     //============================================================================================
     // Other Helpers
-    // TODO(pedrosimonetti): organize into sections.
+    // TODO(donnd): organize into sections.
     //============================================================================================
 
     /**
@@ -710,6 +756,14 @@ public class ContextualSearchManagerTest {
         assertLoadedNoUrl();
     }
 
+    /**
+     * Asserts that a Search Term has been requested.
+     * @param isExactResolve Whether the Resolve request must be exact (non-expanding).
+     */
+    private void assertExactResolve(boolean isExactResolve) {
+        Assert.assertEquals(isExactResolve, mFakeServer.getIsExactResolve());
+    }
+
     /**
      * Waits for the Search Panel (the Search Bar) to peek up from the bottom, and asserts that it
      * did peek.
@@ -874,7 +928,6 @@ public class ContextualSearchManagerTest {
      * Flings the panel up to its expanded state.
      */
     private void flingPanelUp() {
-        // TODO(pedrosimonetti): Consider using a swipe method instead.
         fling(0.5f, 0.95f, 0.5f, 0.55f, 1000);
     }
 
@@ -889,7 +942,6 @@ public class ContextualSearchManagerTest {
      * Flings the panel up to its maximized state.
      */
     private void flingPanelUpToTop() {
-        // TODO(pedrosimonetti): Consider using a swipe method instead.
         fling(0.5f, 0.95f, 0.5f, 0.05f, 1000);
     }
 
@@ -897,7 +949,6 @@ public class ContextualSearchManagerTest {
      * Scrolls the base page.
      */
     private void scrollBasePage() {
-        // TODO(pedrosimonetti): Consider using a swipe method instead.
         fling(0.f, 0.75f, 0.f, 0.7f, 100);
     }
 
@@ -905,7 +956,7 @@ public class ContextualSearchManagerTest {
      * Taps the base page near the top.
      */
     private void tapBasePageToClosePanel() {
-        // TODO(pedrosimonetti): This is not reliable. Find a better approach.
+        // TODO(donnd): This is not reliable. Find a better approach.
         // This taps on the panel in an area that will be selected if the "intelligence" node has
         // been tap-selected, and that will cause it to be long-press selected.
         // We use the far right side to prevent simulating a tap on top of an
@@ -991,6 +1042,13 @@ public class ContextualSearchManagerTest {
         assertLoadedNoUrl();
         assertSearchTermRequested();
 
+        fakeAResponse();
+    }
+
+    /**
+     * Fakes a response to the Resove request.
+     */
+    private void fakeAResponse() {
         fakeResponse(false, 200, "states", "United States Intelligence", "alternate-term", false);
         waitForPanelToPeek();
         assertLoadedLowPriorityUrl();
@@ -3140,4 +3198,41 @@ public class ContextualSearchManagerTest {
         assertPanelClosedOrUndefined();
         assertLoadedNoUrl();
     }
+
+    @Test
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Features.EnableFeatures("ContextualSearchLongpressResolve")
+    public void testLongpressResolveEnabled() throws TimeoutException {
+        longPressNode("states");
+        assertLoadedNoUrl();
+        assertSearchTermRequested();
+
+        fakeResponse(false, 200, "states", "United States Intelligence", "alternate-term", false);
+        waitForPanelToPeek();
+        assertLoadedLowPriorityUrl();
+        assertContainsParameters("states", "alternate-term");
+    }
+
+    @Test
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Features.EnableFeatures("ContextualSearchLongpressResolve")
+    public void testLongpressExtendinSelectionExactResolve() throws TimeoutException {
+        // First test regular long-press.  It should not require an exact resolve.
+        longPressNode("search");
+        fakeAResponse();
+        assertSearchTermRequested();
+        assertExactResolve(false);
+
+        // Long press a node without release so we can simulate the user extending the selection.
+        long downTime = longPressNodeWithoutUp("search");
+
+        // Extend the selection to the nearby word.
+        longPressExtendSelection("term", "resolution", downTime);
+        waitForSelectActionBarVisible();
+        fakeAResponse();
+        assertSearchTermRequested();
+        assertExactResolve(true);
+    }
 }

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

@@ -106,8 +106,7 @@ public class ContextualSearchTapEventTest {
         }
 
         @Override
-        public void startSearchTermResolutionRequest(
-                String selection, boolean isRestrictedResolve) {
+        public void startSearchTermResolutionRequest(String selection, boolean isExactResolve) {
             // Skip native calls and immediately "resolve" the search term.
             onSearchTermResolutionResponse(true, 200, selection, selection, "", "", false, 0, 10,
                     "", "", "", "", QuickActionCategory.NONE, 0, "", "", 0);

chrome/browser/android/contextualsearch/contextual_search_context.cc

@@ -10,14 +10,7 @@
 #include "components/translate/core/language_detection/language_detection_util.h"
 #include "content/public/browser/browser_thread.h"
 
-ContextualSearchContext::ContextualSearchContext(JNIEnv* env, jobject obj)
-    : can_resolve(false),
-      can_send_base_page_url(false),
-      home_country(std::string()),
-      base_page_url(GURL()),
-      surrounding_text(base::string16()),
-      start_offset(0),
-      end_offset(0) {
+ContextualSearchContext::ContextualSearchContext(JNIEnv* env, jobject obj) {
   java_object_.Reset(env, obj);
 }
 
@@ -25,7 +18,9 @@ ContextualSearchContext::ContextualSearchContext(
     const std::string& home_country,
     const GURL& page_url,
     const std::string& encoding)
-    : home_country(home_country),
+    : can_resolve(true),
+      can_send_base_page_url(true),
+      home_country(home_country),
       base_page_url(page_url),
       base_page_encoding(encoding) {
   java_object_ = nullptr;
@@ -147,15 +142,14 @@ int ContextualSearchContext::GetEndOffset() const {
   return end_offset;
 }
 
-void ContextualSearchContext::RestrictResolve(
+void ContextualSearchContext::SetExactResolve(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj) {
-  // TODO(donnd): improve on this cheap implementation by sending this bit to
-  // the server instead of destroying our valuable context!
-  int start = this->start_offset;
-  int end = this->end_offset;
-  SetSelectionSurroundings(0, end - start,
-                           this->surrounding_text.substr(start, end - start));
+  is_exact_resolve = true;
+}
+
+bool ContextualSearchContext::GetExactResolve() {
+  return is_exact_resolve;
 }
 
 base::android::ScopedJavaLocalRef<jstring>

chrome/browser/android/contextualsearch/contextual_search_context.h

@@ -92,11 +92,15 @@ struct ContextualSearchContext {
   int64_t GetPreviousEventId() const;
   int GetPreviousEventResults() const;
 
-  // Causes the next resolve request to be for an exact match instead of an
-  // expandable term.
-  void RestrictResolve(JNIEnv* env,
+  // Causes resolve requests to be for an exact match instead of an expandable
+  // term.
+  void SetExactResolve(JNIEnv* env,
                        const base::android::JavaParamRef<jobject>& obj);
 
+  // Returns whether the resolve request is for an exact match instead of an
+  // expandable term.
+  bool GetExactResolve();
+
   // Detects the language of the context using CLD from the translate utility.
   base::android::ScopedJavaLocalRef<jstring> DetectLanguage(
       JNIEnv* env,
@@ -106,17 +110,18 @@ struct ContextualSearchContext {
   base::WeakPtr<ContextualSearchContext> GetWeakPtr();
 
  private:
-  bool can_resolve;
-  bool can_send_base_page_url;
+  bool can_resolve = false;
+  bool can_send_base_page_url = false;
 
   std::string home_country;
   GURL base_page_url;
   std::string base_page_encoding;
   base::string16 surrounding_text;
-  int start_offset;
-  int end_offset;
-  int64_t previous_event_id;
-  int previous_event_results;
+  int start_offset = 0;
+  int end_offset = 0;
+  int64_t previous_event_id = 0L;
+  int previous_event_results = 0;
+  bool is_exact_resolve = false;
 
   // The linked Java object.
   base::android::ScopedJavaGlobalRef<jobject> java_object_;

chrome/browser/android/contextualsearch/contextual_search_delegate.cc

@@ -290,12 +290,10 @@ std::string ContextualSearchDelegate::BuildRequestUrl(
     contextual_cards_version = field_trial_->GetContextualCardsVersion();
   }
 
-  // TODO(donnd): use a passed-in value for is_search_exact ASAP (CL pending).
-  bool is_exact_search = false;
   TemplateURLRef::SearchTermsArgs::ContextualSearchParams params(
       kContextualSearchRequestVersion, contextual_cards_version,
       context->GetHomeCountry(), context->GetPreviousEventId(),
-      context->GetPreviousEventResults(), is_exact_search);
+      context->GetPreviousEventResults(), context->GetExactResolve());
 
   search_terms_args.contextual_search_params = params;