Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3035)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

Issue 1304013002: Move functionality for ContentViewCore to ContextualSearchPanel (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@simple-move-cvc-to-panel
Patch Set: Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 93f9955f85b32fc885ac97c306f1d189f50a12ec..e796411573a8dbbf3fc9d6bcc4a75eed2980d1bc 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
@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.contextualsearch;
import android.app.Activity;
import android.view.View;
-import android.view.View.MeasureSpec;
import android.view.ViewGroup;
import android.view.ViewTreeObserver;
import android.view.ViewTreeObserver.OnGlobalFocusChangeListener;
@@ -19,7 +18,6 @@ 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.WebContentsFactory;
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;
@@ -42,18 +40,13 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;
import org.chromium.chrome.browser.widget.findinpage.FindToolbarManager;
import org.chromium.chrome.browser.widget.findinpage.FindToolbarObserver;
-import org.chromium.components.navigation_interception.InterceptNavigationDelegate;
import org.chromium.components.navigation_interception.NavigationParams;
-import org.chromium.components.web_contents_delegate_android.WebContentsDelegateAndroid;
-import org.chromium.content.browser.ContentView;
-import org.chromium.content.browser.ContentViewClient;
import org.chromium.content.browser.ContentViewCore;
import org.chromium.content.browser.ContextualSearchClient;
import org.chromium.content_public.browser.GestureStateListener;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.NavigationEntry;
import org.chromium.content_public.browser.WebContentsObserver;
-import org.chromium.content_public.common.ConsoleMessageLevel;
import org.chromium.content_public.common.TopControlsState;
import org.chromium.ui.base.WindowAndroid;
@@ -95,7 +88,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
private final WindowAndroid mWindowAndroid;
private WebContentsObserver mSearchWebContentsObserver;
- private final WebContentsDelegateAndroid mWebContentsDelegate;
private ContextualSearchContentViewDelegate mSearchContentViewDelegate;
private final ContextualSearchTabPromotionDelegate mTabPromotionDelegate;
private TabModelSelectorTabObserver mTabModelSelectorTabObserver;
@@ -136,10 +128,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
private TabRedirectHandler mTabRedirectHandler;
- // http://crbug.com/522266 : An instance of InterceptNavigationDelegateImpl should be kept in
- // java layer. Otherwise, the instance could be garbage-collected unexpectedly.
- private InterceptNavigationDelegateImpl mInterceptNavigationDelegate;
-
/**
* The delegate that is notified when the Search Panel ContentViewCore is ready to be rendered.
*/
@@ -185,26 +173,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
mSelectionController = new ContextualSearchSelectionController(activity, this);
- mWebContentsDelegate = new WebContentsDelegateAndroid() {
- @Override
- public void onLoadStarted() {
- super.onLoadStarted();
- mSearchPanelDelegate.onLoadStarted();
- }
-
- @Override
- public void onLoadStopped() {
- super.onLoadStopped();
- mSearchPanelDelegate.onLoadStopped();
- }
-
- @Override
- public void onLoadProgressChanged(int progress) {
- super.onLoadProgressChanged(progress);
- mSearchPanelDelegate.onLoadProgressChanged(progress);
- }
- };
-
final View controlContainer = mActivity.findViewById(R.id.control_container);
mOnFocusChangeListener = new OnGlobalFocusChangeListener() {
@Override
@@ -300,6 +268,8 @@ public class ContextualSearchManager extends ContextualSearchObservable
@Override
public void setContextualSearchPanelDelegate(ContextualSearchPanelDelegate delegate) {
mSearchPanelDelegate = delegate;
+
+ mSearchPanelDelegate.setChromeActivity(mActivity);
}
@Override
@@ -415,7 +385,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
mLoadedSearchUrlTimeMs = 0L;
mWereSearchResultsSeen = false;
- mNetworkCommunicator.destroySearchContentView();
mSearchRequest = null;
if (mIsShowingPromo && !mDidLogPromoOutcome) {
@@ -494,9 +463,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
removeLastSearchVisit();
}
- // Make sure we'll create a new Content View when needed.
- mNetworkCommunicator.destroySearchContentView();
-
boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP;
boolean didRequestSurroundings = false;
if (isTap && mPolicy.shouldPreviousTapResolve(
@@ -743,10 +709,10 @@ public class ContextualSearchManager extends ContextualSearchObservable
boolean shouldPreload = !doPreventPreload && mPolicy.shouldPrefetchSearchResult(true);
mSearchRequest = new ContextualSearchRequest(searchTerm, alternateTerm, shouldPreload);
mDidLoadResolvedSearchRequest = false;
- if (mIsSearchContentViewShowing) {
+ if (mSearchPanelDelegate.isContentViewShowing()) {
mSearchRequest.setNormalPriority();
}
- if (mIsSearchContentViewShowing || shouldPreload) {
+ if (mSearchPanelDelegate.isContentViewShowing() || shouldPreload) {
loadSearchUrl();
}
mPolicy.logSearchTermResolutionDetails(searchTerm,
@@ -763,7 +729,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
*/
private void loadSearchUrl() {
mLoadedSearchUrlTimeMs = System.currentTimeMillis();
- mNetworkCommunicator.loadUrl(mSearchRequest.getSearchUrl());
+ mSearchPanelDelegate.loadUrlInPanel(mSearchRequest.getSearchUrl());
mDidLoadResolvedSearchRequest = true;
// TODO(pedrosimonetti): If the user taps on a word and quickly after that taps on the
@@ -772,7 +738,8 @@ public class ContextualSearchManager extends ContextualSearchObservable
// to coordinate with Chrome-Android folks to come up with a proper fix for this.
// For now, we force the ContentView to be displayed by calling onShow() again
// when a URL is being loaded. See: crbug.com/398206
- if (mIsSearchContentViewShowing && mSearchPanelDelegate.getContentViewCore() != null) {
+ if (mSearchPanelDelegate.isContentViewShowing()
+ && mSearchPanelDelegate.getContentViewCore() != null) {
mSearchPanelDelegate.getContentViewCore().onShow();
}
}
@@ -824,7 +791,8 @@ public class ContextualSearchManager extends ContextualSearchObservable
* a load of a user-visible search result.
* @param isFailure Whether the navigation failed.
*/
- private void onContextualSearchRequestNavigation(boolean isFailure) {
+ @Override
+ public void onContextualSearchRequestNavigation(boolean isFailure) {
if (mSearchRequest == null) return;
if (mSearchRequest.isUsingLowPriority()) {
@@ -846,7 +814,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
mSearchRequest.setHasFailed();
mSearchRequest.setNormalPriority();
// If the content view is showing, load at normal priority now.
- if (mIsSearchContentViewShowing) {
+ if (mSearchPanelDelegate.isContentViewShowing()) {
loadSearchUrl();
} else {
mDidLoadResolvedSearchRequest = false;
@@ -864,116 +832,27 @@ 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.
*/
- private void onSearchResultsLoaded() {
+ @Override
+ public void onSearchResultsLoaded() {
if (mSearchRequest == null) return;
mSearchPanelDelegate.onSearchResultsLoaded(mSearchRequest.wasPrefetch());
- }
- /**
- * Creates a new Content View Core to display search results, if needed.
- */
- private void createNewSearchContentViewCoreIfNeeded() {
- if (mSearchPanelDelegate.getContentViewCore() == null) {
- mNetworkCommunicator.createNewSearchContentView();
+ // Any time we place a page in a ContentViewCore, clear history if needed.
+ // This prevents error URLs from appearing in the Tab's history stack.
+ // Also please note that clearHistory() will not
+ // clear the current entry (search results page in this case),
+ // and it will not work properly if there are pending navigations.
+ // That's why we need to clear the history here, after the navigation
+ // is completed.
+ boolean shouldClearHistory = mSearchRequest.getHasFailed();
+ if (shouldClearHistory && mSearchPanelDelegate.getContentViewCore() != null) {
+ mSearchPanelDelegate.getContentViewCore().getWebContents().getNavigationController()
+ .clearHistory();
}
}
@Override
- public void loadUrl(String url) {
- createNewSearchContentViewCoreIfNeeded();
- if (mSearchPanelDelegate.getContentViewCore() != null
- && mSearchPanelDelegate.getContentViewCore().getWebContents() != null) {
- mDidLoadAnyUrl = true;
- mSearchPanelDelegate.getContentViewCore().getWebContents()
- .getNavigationController().loadUrl(new LoadUrlParams(url));
- }
- }
-
- @Override
- public void createNewSearchContentView() {
- if (mSearchPanelDelegate.getContentViewCore() != null) {
- mNetworkCommunicator.destroySearchContentView();
- }
-
- final ContentViewCore cvc = new ContentViewCore(mActivity);
-
- // Adds a ContentViewClient to override the default fullscreen size.
- if (!mSearchPanelDelegate.isFullscreenSizePanel()) {
- cvc.setContentViewClient(new ContentViewClient() {
- @Override
- public int getDesiredWidthMeasureSpec() {
- return MeasureSpec.makeMeasureSpec(
- mSearchPanelDelegate.getSearchContentViewWidthPx(),
- MeasureSpec.EXACTLY);
- }
-
- @Override
- public int getDesiredHeightMeasureSpec() {
- return MeasureSpec.makeMeasureSpec(
- mSearchPanelDelegate.getSearchContentViewHeightPx(),
- MeasureSpec.EXACTLY);
- }
- });
- }
-
- ContentView cv = new ContentView(mActivity, cvc);
- // Creates an initially hidden WebContents which gets shown when the panel is opened.
- cvc.initialize(cv, cv,
- WebContentsFactory.createWebContents(false, true), mWindowAndroid);
-
- // Transfers the ownership of the WebContents to the native ContextualSearchPanel.
- mSearchPanelDelegate.setWebContents(cvc, mWebContentsDelegate);
-
- mSearchWebContentsObserver =
- new WebContentsObserver(cvc.getWebContents()) {
- @Override
- public void didStartLoading(String url) {
- mDidPromoteSearchNavigation = false;
- }
-
- @Override
- public void didStartProvisionalLoadForFrame(long frameId, long parentFrameId,
- boolean isMainFrame, String validatedUrl, boolean isErrorPage,
- boolean isIframeSrcdoc) {
- if (isMainFrame) onExternalNavigation(validatedUrl);
- }
-
- @Override
- public void didNavigateMainFrame(String url, String baseUrl,
- boolean isNavigationToDifferentPage, boolean isNavigationInPage,
- int httpResultCode) {
- mNetworkCommunicator.handleDidNavigateMainFrame(url, httpResultCode);
- }
-
- @Override
- public void didFinishLoad(long frameId, String validatedUrl,
- boolean isMainFrame) {
- onSearchResultsLoaded();
-
- // Any time we place a page in a ContentViewCore, clear history if needed.
- // This prevents error URLs from appearing in the Tab's history stack.
- // Also please note that clearHistory() will not
- // clear the current entry (search results page in this case),
- // and it will not work properly if there are pending navigations.
- // That's why we need to clear the history here, after the navigation
- // is completed.
- boolean shouldClearHistory =
- mSearchRequest != null && mSearchRequest.getHasFailed();
- if (shouldClearHistory && cvc != null) {
- cvc.getWebContents().getNavigationController()
- .clearHistory();
- }
- }
- };
-
- mSearchContentViewDelegate.setContextualSearchContentViewCore(cvc);
- mInterceptNavigationDelegate = new InterceptNavigationDelegateImpl();
- mSearchPanelDelegate.setInterceptNavigationDelegate(mInterceptNavigationDelegate,
- cvc.getWebContents());
- }
-
- @Override
public void handleDidNavigateMainFrame(String url, int httpResultCode) {
if (shouldPromoteSearchNavigation()) {
onExternalNavigation(url);
@@ -982,7 +861,34 @@ public class ContextualSearchManager extends ContextualSearchObservable
boolean isFailure = isHttpFailureCode(httpResultCode);
onContextualSearchRequestNavigation(isFailure);
}
- mDidLoadAnyUrl = false;
+ }
+
+ @Override
+ public void onStartedLoading() {
+ mDidPromoteSearchNavigation = false;
+ }
+
+
+ @Override
+ public boolean shouldInterceptNavigation(ExternalNavigationHandler externalNavHandler,
+ NavigationParams navigationParams) {
+ ExternalNavigationParams params = new ExternalNavigationParams.Builder(
+ navigationParams.url, false, navigationParams.referrer,
+ navigationParams.pageTransitionType, navigationParams.isRedirect)
+ .setApplicationMustBeInForeground(true)
+ .setRedirectHandler(mTabRedirectHandler)
+ .setIsMainFrame(navigationParams.isMainFrame)
+ .build();
+ if (externalNavHandler.shouldOverrideUrlLoading(params)
+ != OverrideUrlLoadingResult.NO_OVERRIDE) {
+ mSearchPanelDelegate.maximizePanelThenPromoteToTab(StateChangeReason.TAB_PROMOTION,
+ INTERCEPT_NAVIGATION_PROMOTION_ANIMATION_DURATION_MS);
+ return false;
+ }
+ if (navigationParams.isExternalProtocol) {
+ return false;
+ }
+ return true;
}
/**
@@ -999,7 +905,8 @@ public class ContextualSearchManager extends ContextualSearchObservable
// A navigation can be due to us loading a URL, or a touch in the search content view.
// Require a touch, but no recent loading, in order to promote to a separate tab.
// Note that tapping the opt-in button requires checking for recent loading.
- return mSearchPanelDelegate.didTouchSearchContentView() && !mDidLoadAnyUrl;
+ return mSearchPanelDelegate.didTouchSearchContentView()
+ && !mSearchPanelDelegate.didLoadAnyUrl();
}
/**
@@ -1007,21 +914,9 @@ 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.
*/
- private void onExternalNavigation(String url) {
- if (mSearchPanelDelegate.isFullscreenSizePanel()) {
- // Consider the ContentView height to be fullscreen, and inform the system that
- // the Toolbar is always visible (from the Compositor's perspective), even though
- // the Toolbar and Base Page might be offset outside the screen. This means the
- // renderer will consider the ContentView height to be the fullscreen height
- // minus the Toolbar height.
- //
- // This is necessary to fix the bugs: crbug.com/510205 and crbug.com/510206
- mSearchPanelDelegate.getContentViewCore().getWebContents()
- .updateTopControlsState(false, true, false);
- } else {
- mSearchPanelDelegate.getContentViewCore().getWebContents()
- .updateTopControlsState(true, false, false);
- }
+ @Override
+ public void onExternalNavigation(String url) {
+ mSearchPanelDelegate.updateTopControlState();
if (!mDidPromoteSearchNavigation
&& !BLACKLISTED_URL.equals(url)
@@ -1038,27 +933,16 @@ public class ContextualSearchManager extends ContextualSearchObservable
}
@Override
- public void destroySearchContentView() {
- if (mSearchPanelDelegate.getContentViewCore() != null
- && mSearchContentViewDelegate != null) {
- mSearchPanelDelegate.destroyWebContents();
+ public void onContentViewCreated(ContentViewCore contentView) {
+ // TODO(mdjones): Move SearchContentViewDelegate ownership to panel.
+ mSearchContentViewDelegate.setContextualSearchContentViewCore(contentView);
+ }
+
+ @Override
+ public void onContentViewDestroyed() {
+ if (mSearchContentViewDelegate != null) {
mSearchContentViewDelegate.releaseContextualSearchContentViewCore();
- mSearchPanelDelegate.getContentViewCore().getWebContents().destroy();
- mSearchPanelDelegate.getContentViewCore().destroy();
- mSearchPanelDelegate.resetContentViewCore();
- if (mSearchWebContentsObserver != null) {
- mSearchWebContentsObserver.destroy();
- mSearchWebContentsObserver = null;
- }
}
-
- // This should be called last here. The setSearchContentViewVisibility method
- // will change the visibility the SearchContentView but also set the value of the
- // internal property mIsSearchContentViewShowing. If we call this after deleting
- // the SearchContentView, it will be faster, because only the internal property
- // will be changed, since there will be no need to change the visibility of the
- // SearchContentView.
- setSearchContentViewVisibility(false);
}
@Override
@@ -1131,23 +1015,13 @@ public class ContextualSearchManager extends ContextualSearchObservable
}
@Override
- public void resetSearchContentViewScroll() {
- if (mSearchPanelDelegate.getContentViewCore() != null) {
- mSearchPanelDelegate.getContentViewCore().scrollTo(0, 0);
- }
- }
-
- @Override
public float getSearchContentViewVerticalScroll() {
return mSearchPanelDelegate.getContentViewCore() != null
? mSearchPanelDelegate.getContentViewCore().computeVerticalScrollOffset() : -1.f;
}
@Override
- public void setSearchContentViewVisibility(boolean isVisible) {
- if (mIsSearchContentViewShowing == isVisible) return;
-
- mIsSearchContentViewShowing = isVisible;
+ public void onContentViewVisibilityChanged(boolean isVisible) {
if (isVisible) {
mWereSearchResultsSeen = true;
// If there's no current request, then either a search term resolution
@@ -1163,18 +1037,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
mSearchRequest.setNormalPriority();
loadSearchUrl();
}
- // The CVC is created with the search request, but if none was made we'll need
- // one in order to display an empty panel.
- createNewSearchContentViewCoreIfNeeded();
- if (mSearchPanelDelegate.getContentViewCore() != null) {
- mSearchPanelDelegate.getContentViewCore().onShow();
- }
- mSearchPanelDelegate.setWasSearchContentViewSeen();
mPolicy.updateCountersForOpen();
- } else {
- if (mSearchPanelDelegate.getContentViewCore() != null) {
- mSearchPanelDelegate.getContentViewCore().onHide();
- }
}
}
@@ -1191,50 +1054,6 @@ public class ContextualSearchManager extends ContextualSearchObservable
hideContextualSearch(StateChangeReason.UNKNOWN);
}
- // Used to intercept intent navigations.
- // TODO(jeremycho): Consider creating a Tab with the Panel's ContentViewCore,
- // which would also handle functionality like long-press-to-paste.
- private class InterceptNavigationDelegateImpl implements InterceptNavigationDelegate {
- final ExternalNavigationHandler mExternalNavHandler = new ExternalNavigationHandler(
- mActivity);
- @Override
- public boolean shouldIgnoreNavigation(NavigationParams navigationParams) {
- mTabRedirectHandler.updateNewUrlLoading(navigationParams.pageTransitionType,
- navigationParams.isRedirect,
- navigationParams.hasUserGesture || navigationParams.hasUserGestureCarryover,
- mActivity.getLastUserInteractionTime(), TabRedirectHandler.INVALID_ENTRY_INDEX);
-
- ExternalNavigationParams params = new ExternalNavigationParams.Builder(
- navigationParams.url, false, navigationParams.referrer,
- navigationParams.pageTransitionType, navigationParams.isRedirect)
- .setApplicationMustBeInForeground(true)
- .setRedirectHandler(mTabRedirectHandler)
- .setIsMainFrame(navigationParams.isMainFrame)
- .build();
- if (mExternalNavHandler.shouldOverrideUrlLoading(params)
- != OverrideUrlLoadingResult.NO_OVERRIDE) {
- mSearchPanelDelegate.maximizePanelThenPromoteToTab(
- StateChangeReason.TAB_PROMOTION,
- INTERCEPT_NAVIGATION_PROMOTION_ANIMATION_DURATION_MS);
- return true;
- }
- if (navigationParams.isExternalProtocol) {
- ContentViewCore baseContentView = getBaseContentView();
- if (baseContentView != null) {
- int resId = mExternalNavHandler.canExternalAppHandleUrl(navigationParams.url)
- ? R.string.blocked_navigation_warning
- : R.string.unreachable_navigation_warning;
- String message = mActivity.getApplicationContext().getString(
- resId, navigationParams.url);
- baseContentView.getWebContents().addMessageToDevToolsConsole(
- ConsoleMessageLevel.WARNING, message);
- }
- return true;
- }
- return false;
- }
- }
-
// --------------------------------------------------------------------------------------------
// ContextualSearchClient -- interface used by ContentViewCore.
// --------------------------------------------------------------------------------------------

Powered by Google App Engine
This is Rietveld 408576698