[TTS] Add a Java Context linked to existing native

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

Index: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
index 9028aea2fc796e27b5b9b4030e533338e0be72da..4b8af8287a9941df91378019bb760b109ee850e4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
@@ -120,6 +120,9 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     private boolean mWasActivatedByTap;
     private boolean mIsInitialized;
 
+    // The current search context, or null.
+    private ContextualSearchContext mContext;
+
     /**
      * This boolean is used for loading content after a long-press when content is not immediately
      * loaded.
@@ -303,8 +306,9 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     /**
      * @return The Base Page's {@link ContentViewCore}.
      */
-    @Nullable private ContentViewCore getBaseContentView() {
-        return mSelectionController.getBaseContentView();
+    @Nullable
+    private WebContents getBaseWebContents() {
+        return mSelectionController.getBaseWebContents();
     }
 
     /**
@@ -326,6 +330,8 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
      * @param reason The {@link StateChangeReason} for hiding Contextual Search.
      */
     public void hideContextualSearch(StateChangeReason reason) {
+        if (mContext != null) mContext.destroy();
+        mContext = null;
         if (mSearchPanel == null) return;
 
         if (mSearchPanel.isShowing()) {
@@ -419,6 +425,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
         // TODO(pedrosimonetti): Fix for M47. Replace this with a better delayed load approach.
         mSearchPanel.destroyContent();
 
+        String selection = mSelectionController.getSelectedText();
         boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP;
         boolean didRequestSurroundings = false;
         if (isTap) {
@@ -426,42 +433,40 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
             mShouldLoadDelayedSearch = false;
         }
         if (isTap && mPolicy.shouldPreviousTapResolve()) {
-            mNetworkCommunicator.startSearchTermResolutionRequest(
-                    mSelectionController.getSelectedText());
+            mContext = new ContextualSearchContext(

[Theresa, 2017/03/08 01:54:38]

showContextualSearch() can be called due to a selection modification. I believe
that it may even be called twice with isTap if a selection is being modified due
to a second tap, but I'm not entirely sure.

Should we be destroying the old context?

[Donn Denman, 2017/03/09 17:35:04]

We should have already destroyed any old context and set it to null.  Added an
assert here to verify that's been done.

Also updated the Icing case when constructing below (line 467).

+                    selection, mPolicy.getHomeCountry(mActivity), mPolicy.maySendBasePageUrl());
+            nativeGatherSurroundingText(
+                    mNativeContextualSearchManagerPtr, mContext, getBaseWebContents());
             didRequestSurroundings = true;
             // Cache the native translate data, so JNI calls won't be made when time-critical.
             mTranslateController.cacheNativeTranslateData();
         } else {
             boolean shouldPrefetch = mPolicy.shouldPrefetchSearchResult();
-            mSearchRequest = createContextualSearchRequest(
-                    mSelectionController.getSelectedText(), null, null, shouldPrefetch);
+            mSearchRequest = createContextualSearchRequest(selection, null, null, shouldPrefetch);
             mTranslateController.forceAutoDetectTranslateUnlessDisabled(mSearchRequest);
             mDidStartLoadingResolvedSearchRequest = false;
-            mSearchPanel.setSearchTerm(mSelectionController.getSelectedText());
+            mSearchPanel.setSearchTerm(selection);
             if (shouldPrefetch) loadSearchUrl();
 
             // Record metrics for manual refinement of the search term from long-press.
             // TODO(donnd): remove this section once metrics have been analyzed.
-            if (mSelectionController.getSelectionType() == SelectionType.LONG_PRESS
-                    && mSearchPanel.isPeeking()) {
+            if (!isTap && mSearchPanel.isPeeking()) {
                 boolean isSingleWord =
-                        !CONTAINS_WHITESPACE_PATTERN
-                                 .matcher(mSelectionController.getSelectedText().trim())
-                                 .find();
+                        !CONTAINS_WHITESPACE_PATTERN.matcher(selection.trim()).find();
                 RecordUserAction.record(isSingleWord ? "ContextualSearch.ManualRefineSingleWord"
                                                      : "ContextualSearch.ManualRefineMultiWord");
             }
         }
 
-        if (!didRequestSurroundings) {
+        if (!didRequestSurroundings && getBaseWebContents() != null) {
             // Gather surrounding text for Icing integration, which will make the selection and
             // a shorter version of the surroundings available for Conversational Search.
             // Although the surroundings are extracted, they will not be sent to the server as
             // part of search term resolution, just sent to Icing which keeps them local until
             // the user activates a Voice Search.
-            nativeGatherSurroundingText(mNativeContextualSearchManagerPtr,
-                    mSelectionController.getSelectedText(), NO_HOME_COUNTRY,
-                    getBaseContentView().getWebContents(), mPolicy.maySendBasePageUrl());
+            mContext = new ContextualSearchContext(selection);
+            nativeGatherSurroundingText(
+                    mNativeContextualSearchManagerPtr, mContext, getBaseWebContents());
         }
 
         mWereSearchResultsSeen = false;
@@ -492,8 +497,6 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
             mSearchPanel.setDidSearchInvolvePromo();
         }
 
-        // TODO(donnd): although we are showing the bar here, we have not yet set the text!
-        // Refactor to show the bar and set the text at the same time!
         // TODO(donnd): If there was a previously ongoing contextual search, we should ensure
         // it's registered as closed.
         mSearchPanel.requestPanelShow(stateChangeReason);
@@ -504,21 +507,21 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
 
     @Override
     public void startSearchTermResolutionRequest(String selection) {
-        ContentViewCore baseContentView = getBaseContentView();
-        if (baseContentView != null) {
-            nativeStartSearchTermResolutionRequest(mNativeContextualSearchManagerPtr, selection,
-                    mPolicy.getHomeCountry(mActivity), getBaseContentView().getWebContents(),
-                    mPolicy.maySendBasePageUrl());
+        WebContents baseWebContents = getBaseWebContents();
+        if (baseWebContents != null) {
+            assert mContext != null;
+            nativeStartSearchTermResolutionRequest(
+                    mNativeContextualSearchManagerPtr, mContext, getBaseWebContents());
         }
     }
 
     @Override
     @Nullable public URL getBasePageUrl() {
-        ContentViewCore baseContentViewCore = getBaseContentView();
-        if (baseContentViewCore == null) return null;
+        WebContents baseWebContents = getBaseWebContents();
+        if (baseWebContents == null) return null;
 
         try {
-            return new URL(baseContentViewCore.getWebContents().getUrl());
+            return new URL(baseWebContents.getUrl());
         } catch (MalformedURLException e) {
             return null;
         }
@@ -615,10 +618,11 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
      */
     @CalledByNative
     private void onSurroundingTextAvailable(final String afterText) {
+        String selection = mSelectionController.getSelectedText();
         // TODO(donnd): check if panel has been requested to show.
         // We used to call mSearchPanel.isShowing() here, but that's unreliable (crbug.com/669600).
-        mSearchPanel.setSearchContext(
-                mSelectionController.getSelectedText(), afterText);
+        mSearchPanel.setSearchContext(selection, afterText);

[Theresa, 2017/03/08 01:54:38]

CSPanel#setSearchContext() probably needs to be renamed since it collides with
the new Java ContextualSearchContext class.

[Donn Denman, 2017/03/09 17:35:04]

Done.

Changed to setContextDetails and cascaded that name change through to the
Context Control.

+        mNetworkCommunicator.startSearchTermResolutionRequest(selection);
     }
 
     /**
@@ -748,7 +752,11 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
         }
 
         if (selectionStartAdjust != 0 || selectionEndAdjust != 0) {
-            mSelectionController.adjustSelection(selectionStartAdjust, selectionEndAdjust);
+            if (mSelectionController.getSelectedText() == mContext.getSelection()) {

[Theresa, 2017/03/08 01:54:38]

I'm not quite sure what's happening here -- when would this be false?

Also, findbugs is complaining about the == comparison for Strings.

[Donn Denman, 2017/03/09 17:35:04]

This can happen when the user changes the selection while the resolve is in
progress.  When the selection has changed we shouldn't apply the adjustment, we
should just abort the search.

[Theresa, 2017/03/09 18:29:44]

That makes sense. In this scenario, should be do nothing here rather than
calling hideContextualSearch() (unless the selection is null)? We should be
doing a verbatim search for the new selection, but hopefully that's already
happened before this method is called.

+                mSelectionController.adjustSelection(selectionStartAdjust, selectionEndAdjust);
+            } else {
+                hideContextualSearch(StateChangeReason.UNKNOWN);
+            }
         }
     }
 
@@ -1291,16 +1299,21 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
         if (mIsAccessibilityModeEnabled) return;
 
         if (isTapSupported()) {
-            // Here we are starting a new Contextual Search with a Tap gesture, therefore
-            // we need to clear to properly reflect that a search just started and we don't
-            // have the resolved search term yet.
+            // Here we are probably starting a new Contextual Search with a Tap gesture (or we'll
+            // ignore the tap), therefore we need to clear to properly reflect that a search just
+            // started and we don't have the resolved search term yet.
             mSearchRequest = null;
 
-            // Let the policy know that a tap gesture has been received.
-            mPolicy.registerTap();
+            if (mContext != null) mContext.destroy();
+            mContext = null;
 
-            ContentViewCore baseContentView = getBaseContentView();
-            if (baseContentView != null) baseContentView.getWebContents().selectWordAroundCaret();
+            WebContents baseWebContents = getBaseWebContents();
+            if (baseWebContents != null) {
+                // Let the policy know that a tap gesture has been received.
+                mPolicy.registerTap();
+
+                baseWebContents.selectWordAroundCaret();
+            }
         }
     }
 
@@ -1313,7 +1326,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
             StateChangeReason stateChangeReason = type == SelectionType.TAP
                     ? StateChangeReason.TEXT_SELECT_TAP : StateChangeReason.TEXT_SELECT_LONG_PRESS;
             ContextualSearchUma.logSelectionIsValid(selectionValid);
-            if (selectionValid) {
+            if (selectionValid && mSearchPanel != null) {
                 mSearchPanel.updateBasePageSelectionYPx(y);
                 if (!mSearchPanel.isShowing()) {
                     mSearchPanel.getPanelMetrics().onSelectionEstablished(selection);
@@ -1329,7 +1342,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     public void handleSelectionDismissal() {
         if (mIsAccessibilityModeEnabled) return;
 
-        if (mSearchPanel.isShowing()
+        if (mSearchPanel != null && mSearchPanel.isShowing()
                 && !mIsPromotingToTab
                 // If the selection is dismissed when the Panel is not peeking anymore,
                 // which means the Panel is at least partially expanded, then it means
@@ -1346,7 +1359,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
             String selection, boolean selectionValid, float x, float y) {
         if (mIsAccessibilityModeEnabled) return;
 
-        if (mSearchPanel.isShowing()) {
+        if (mSearchPanel != null && mSearchPanel.isShowing()) {
             if (selectionValid) {
                 mSearchPanel.setSearchTerm(selection);
             } else {
@@ -1359,7 +1372,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     public void handleSelectionSuppression(BlacklistReason reason) {
         if (mIsAccessibilityModeEnabled) return;
 
-        mSearchPanel.getPanelMetrics().setBlacklistReason(reason);
+        if (mSearchPanel != null) mSearchPanel.getPanelMetrics().setBlacklistReason(reason);
     }
 
     // ============================================================================================
@@ -1431,11 +1444,9 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
     private native long nativeInit();
     private native void nativeDestroy(long nativeContextualSearchManager);
     private native void nativeStartSearchTermResolutionRequest(long nativeContextualSearchManager,
-            String selection, String homeCountry, WebContents baseWebContents,
-            boolean maySendBasePageUrl);
+            ContextualSearchContext contextualSearchContext, WebContents baseWebContents);
     protected native void nativeGatherSurroundingText(long nativeContextualSearchManager,
-            String selection, String homeCountry, WebContents baseWebContents,
-            boolean maySendBasePageUrl);
+            ContextualSearchContext contextualSearchContext, WebContents baseWebContents);
     private native void nativeEnableContextualSearchJsApiForOverlay(
             long nativeContextualSearchManager, WebContents overlayWebContents);
     // Don't call these directly, instead call the private methods that cache the results.