Overlay content is its own class

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 d6144848ec55fe7689e731dc24640f5b93b3b21a..21fa3e371b4a21dbea409e6142f54b4e0e72b10a 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
@@ -18,6 +18,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.compositor.bottombar.OverlayContentDelegate;
 import org.chromium.chrome.browser.compositor.bottombar.contextualsearch.ContextualSearchPanel.PanelState;
 import org.chromium.chrome.browser.compositor.bottombar.contextualsearch.ContextualSearchPanel.StateChangeReason;
 import org.chromium.chrome.browser.compositor.bottombar.contextualsearch.ContextualSearchPanelDelegate;
@@ -106,6 +107,12 @@ public class ContextualSearchManager extends ContextualSearchObservable
     private boolean mWasActivatedByTap;
     private boolean mIsInitialized;
 
+    /**
+     * This boolean is used for loading content after a long-press when content is not immediately
+     * loaded.
+     */
+    private boolean mShouldLoadDelayedSearch;
+
     private boolean mIsShowingPromo;
     private boolean mDidLogPromoOutcome;
 
@@ -466,8 +473,16 @@ public class ContextualSearchManager extends ContextualSearchObservable
             removeLastSearchVisit();
         }
 
+        mSearchPanelDelegate.setSearchContentViewVisibility(false);
+
         boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP;
         boolean didRequestSurroundings = false;
+
+        if (isTap) {
+            // If the user action was not a long-press, immediately start loading content.
+            mShouldLoadDelayedSearch = false;

[pedro (no code reviews), 2015/09/23 00:00:12]

I'm not very happy with adding a new boolean, because the logic in the Manager
is already pretty complicated, but I cannot think of a better solution at the
moment.

Things to keep in mind:

1) If we decide to keep the boolean, it should actually be placed inside the
block below, because not all taps will load content before the user pulls the
Panel up.

2) Also note that even when it does, a tap will not load it *immediately*, as
stated in comment. Instead if will be loaded after a round trip to the server to
find out the resolved search term. For this reason, please update the phrasing
of the comment.

I will try to find a better solution, but I cannot apply your patch due to merge
conflicts :(

[mdjones, 2015/09/23 00:29:31]

Acknowledged.

+        }
+
         if (isTap && mPolicy.shouldPreviousTapResolve(
                 mNetworkCommunicator.getBasePageUrl())) {
             mNetworkCommunicator.startSearchTermResolutionRequest(
@@ -757,6 +772,73 @@ public class ContextualSearchManager extends ContextualSearchObservable
         return mPolicy.isTapSupported();
     }
 
+    // ============================================================================================
+    // OverlayContentDelegate
+    // ============================================================================================
+
+    @Override
+    public OverlayContentDelegate getOverlayContentDelegate() {
+        return new SearchOverlayContentDelegate();
+    }
+
+    /**
+     * Implementation of OverlayContentDelegate. Made public for testing purposes.
+     */
+    public class SearchOverlayContentDelegate extends OverlayContentDelegate {
+
+        public SearchOverlayContentDelegate() {}
+
+        @Override
+        public void onMainFrameLoadStarted(String url) {
+            onExternalNavigation(url);
+        }
+
+        @Override
+        public void onMainFrameNavigation(String url, boolean isFailure) {
+            handleDidNavigateMainFrame(url, isFailure);
+        }
+
+        @Override
+        public void onContentLoadStarted(String url) {
+            onStartedLoading();
+        }
+
+        @Override
+        public void onContentLoadFinished() {
+            onSearchResultsLoaded();
+        }
+
+        @Override
+        public void onVisibilityChanged(boolean isVisible) {
+            onContentViewVisibilityChanged(isVisible);
+        }
+
+        @Override
+        public void onContentViewCreated(ContentViewCore contentViewCore) {
+            // TODO: implemented in manager
+            ContextualSearchManager.this.onContentViewCreated(contentViewCore);
+        }
+
+        @Override
+        public void onContentViewDestroyed() {
+            // TODO: implemented in manager
+            ContextualSearchManager.this.onContentViewDestroyed();
+        }
+
+        @Override
+        public void onContentViewSeen() {
+            mSearchPanelDelegate.setWasSearchContentViewSeen();
+        }
+
+        @Override
+        public boolean shouldInterceptNavigation(ExternalNavigationHandler externalNavHandler,
+                NavigationParams navigationParams) {
+            return ContextualSearchManager.this.shouldInterceptNavigation(externalNavHandler,
+                    navigationParams);
+        }
+    }
+
+
     // --------------------------------------------------------------------------------------------
     // Search Content View
     // --------------------------------------------------------------------------------------------
@@ -835,7 +917,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
      * Called when the Search Content view has finished loading to record how long it takes the SERP
      * to load after opening the panel.
      */
-    @Override
     public void onSearchResultsLoaded() {
         if (mSearchRequest == null) return;
 
@@ -855,23 +936,33 @@ public class ContextualSearchManager extends ContextualSearchObservable
         }
     }
 
-    @Override
-    public void handleDidNavigateMainFrame(String url, int httpResultCode) {
+    /**
+     * Handles the WebContentsObserver#didNavigateMainFrame callback.
+     * @param url The URL of the navigation.
+     * @param isFailure True if the navigation resulted in a failure.
+     */
+    public void handleDidNavigateMainFrame(String url, boolean isFailure) {
         if (shouldPromoteSearchNavigation()) {
             onExternalNavigation(url);
         } else {
             // Could be just prefetching, check if that failed.
-            boolean isFailure = isHttpFailureCode(httpResultCode);
             onContextualSearchRequestNavigation(isFailure);
         }
     }
 
-    @Override
+    /**
+     * Called when the WebContents for the panel starts loading.
+     */
     public void onStartedLoading() {
         mDidPromoteSearchNavigation = false;
     }
 
-    @Override
+    /**
+     * Determine if the panel should intercept a particular navigation.
+     * @param externamNavHandler External navigation handler.
+     * @param navigationParams Params associated with the navigation.
+     * @return True if the navigation should be intecepted.
+     */
     public boolean shouldInterceptNavigation(
             ExternalNavigationHandler externalNavHandler, NavigationParams navigationParams) {
         mTabRedirectHandler.updateNewUrlLoading(navigationParams.pageTransitionType,
@@ -889,12 +980,12 @@ public class ContextualSearchManager extends ContextualSearchObservable
                 != OverrideUrlLoadingResult.NO_OVERRIDE) {
             mSearchPanelDelegate.maximizePanelThenPromoteToTab(StateChangeReason.TAB_PROMOTION,
                     INTERCEPT_NAVIGATION_PROMOTION_ANIMATION_DURATION_MS);
-            return true;
+            return false;
         }
         if (navigationParams.isExternalProtocol) {
-            return true;
+            return false;
         }
-        return false;
+        return true;
     }
 
     /**
@@ -920,7 +1011,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
      * Auto-promotes the panel into a separate tab if that's not already being done.
      * @param url The URL we are navigating to.
      */
-    @Override
     public void onExternalNavigation(String url) {
         mSearchPanelDelegate.updateTopControlState();
 
@@ -938,13 +1028,18 @@ public class ContextualSearchManager extends ContextualSearchObservable
         }
     }
 
-    @Override
+    /**
+     * This method is called when the panel's ContentViewCore is created.
+     * @param contentView The created ContentViewCore.
+     */
     public void onContentViewCreated(ContentViewCore contentView) {
         // TODO(mdjones): Move SearchContentViewDelegate ownership to panel.
         mSearchContentViewDelegate.setContextualSearchContentViewCore(contentView);
     }
 
-    @Override
+    /**
+     * This method is called when the panel's ContentViewCore is destroyed.
+     */
     public void onContentViewDestroyed() {
         if (mSearchContentViewDelegate != null) {
             mSearchContentViewDelegate.releaseContextualSearchContentViewCore();
@@ -1026,7 +1121,10 @@ public class ContextualSearchManager extends ContextualSearchObservable
                 ? mSearchPanelDelegate.getContentViewCore().computeVerticalScrollOffset() : -1.f;
     }
 
-    @Override
+    /**
+     * This is called when the search panel is shown or is hidden.
+     * @param isVisible True if the panel is now visible.
+     */
     public void onContentViewVisibilityChanged(boolean isVisible) {
         if (isVisible) {
             mWereSearchResultsSeen = true;
@@ -1039,10 +1137,12 @@ public class ContextualSearchManager extends ContextualSearchObservable
                         mSelectionController.getSelectedText());
                 mDidLoadResolvedSearchRequest = false;
             }
-            if (mSearchRequest != null && !mDidLoadResolvedSearchRequest) {
+            if ((mSearchRequest != null && !mDidLoadResolvedSearchRequest)
+                    || mShouldLoadDelayedSearch) {

[pedro (no code reviews), 2015/09/23 00:00:12]

Could you please include a comment here detailing why exactly we need the new
boolean?

[mdjones, 2015/09/23 00:29:31]

Done.

                 mSearchRequest.setNormalPriority();
                 loadSearchUrl();
             }
+            mShouldLoadDelayedSearch = true;
             mPolicy.updateCountersForOpen();
         }
     }