Commit a4995f45 authored by Patrick Noland's avatar Patrick Noland Committed by Commit Bot

[ToolbarMVC] Remove setOverviewModeBehavior from LocationBarCoordinator

It's something of a layering violation for LBC to use
OverviewModeBehavior, since it's not at all obvious why the omnibox
would need to know about overview mode state directly. Since this is
just used to prefetch when entering an omnibox-having overview mode, we
can have ToolbarManager call a new prefetch method directly.

Bug: 1139481
Change-Id: Iefbb3c9665b084ad5870fc3643c945022cc3e4dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504088Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarTomasz Wiszkowski <ender@google.com>
Reviewed-by: default avatarBrandon Wylie <wylieb@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825138}
parent dbf9b94d
...@@ -13,7 +13,6 @@ import org.chromium.base.supplier.ObservableSupplier; ...@@ -13,7 +13,6 @@ import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.Supplier; import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher; import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.Destroyable; import org.chromium.chrome.browser.lifecycle.Destroyable;
import org.chromium.chrome.browser.lifecycle.NativeInitObserver; import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
...@@ -285,9 +284,9 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat ...@@ -285,9 +284,9 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat
return (LocationBarCoordinatorTablet) mSubCoordinator; return (LocationBarCoordinatorTablet) mSubCoordinator;
} }
/** Sets the {@link LayoutStateProvider}. */ /** Initiates a pre-fetch of autocomplete suggestions. */
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) { public void startAutocompletePrefetch() {
mLocationBarLayout.setLayoutStateProvider(layoutStateProvider); mLocationBarLayout.startPrefetch();
} }
/** /**
......
...@@ -73,9 +73,6 @@ public interface LocationBarDataProvider { ...@@ -73,9 +73,6 @@ public interface LocationBarDataProvider {
*/ */
boolean isInOverviewAndShowingOmnibox(); boolean isInOverviewAndShowingOmnibox();
/** Returns whether the location bar should show when in overview mode. */
boolean shouldShowLocationBarInOverviewMode();
/** Returns the current {@link Profile}. */ /** Returns the current {@link Profile}. */
Profile getProfile(); Profile getProfile();
......
...@@ -45,7 +45,6 @@ import org.chromium.chrome.browser.AppHooks; ...@@ -45,7 +45,6 @@ import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.gsa.GSAState; import org.chromium.chrome.browser.gsa.GSAState;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.locale.LocaleManager; import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.native_page.NativePageFactory; import org.chromium.chrome.browser.native_page.NativePageFactory;
import org.chromium.chrome.browser.ntp.FakeboxDelegate; import org.chromium.chrome.browser.ntp.FakeboxDelegate;
...@@ -352,8 +351,15 @@ public class LocationBarLayout ...@@ -352,8 +351,15 @@ public class LocationBarLayout
return mAutocompleteCoordinator; return mAutocompleteCoordinator;
} }
/**
* Runs logic that can't be invoked until after native is initialized but shouldn't be on the
* critical path, e.g. pre-fetching autocomplete suggestions. Contrast with
* onFinishNativeInitialization, which is for logic that should be on the critical path and need
* native to be initialized. This method must be called after onFinishNativeInitialization.
*/
public void onDeferredStartup() { public void onDeferredStartup() {
mAutocompleteCoordinator.prefetchZeroSuggestResults(); assert mNativeInitialized;
startPrefetch();
} }
public void onFinishNativeInitialization() { public void onFinishNativeInitialization() {
...@@ -385,6 +391,13 @@ public class LocationBarLayout ...@@ -385,6 +391,13 @@ public class LocationBarLayout
setProfile(mProfileSupplier.get()); setProfile(mProfileSupplier.get());
} }
/** Initiates a prefetch of autocomplete suggestions. */
public void startPrefetch() {
if (!mNativeInitialized) return;
mAutocompleteCoordinator.prefetchZeroSuggestResults();
}
public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) { public void setProfileSupplier(ObservableSupplier<Profile> profileSupplier) {
assert profileSupplier != null; assert profileSupplier != null;
assert mProfileSupplier == null; assert mProfileSupplier == null;
...@@ -879,10 +892,6 @@ public class LocationBarLayout ...@@ -879,10 +892,6 @@ public class LocationBarLayout
mUrlCoordinator.setAllowFocus(focusable); mUrlCoordinator.setAllowFocus(focusable);
} }
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
mAutocompleteCoordinator.setLayoutStateProvider(layoutStateProvider);
}
@CallSuper @CallSuper
public void setUrlFocusChangeFraction(float fraction) { public void setUrlFocusChangeFraction(float fraction) {
mUrlFocusChangeFraction = fraction; mUrlFocusChangeFraction = fraction;
......
...@@ -20,7 +20,6 @@ import org.chromium.base.StrictModeContext; ...@@ -20,7 +20,6 @@ import org.chromium.base.StrictModeContext;
import org.chromium.base.supplier.Supplier; import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider; import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
import org.chromium.chrome.browser.omnibox.UrlBar.UrlTextChangeListener; import org.chromium.chrome.browser.omnibox.UrlBar.UrlTextChangeListener;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider; import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
...@@ -221,14 +220,6 @@ public class AutocompleteCoordinator implements UrlFocusChangeListener, UrlTextC ...@@ -221,14 +220,6 @@ public class AutocompleteCoordinator implements UrlFocusChangeListener, UrlTextC
mMediator.setLocationBarDataProvider(locationBarDataProvider); mMediator.setLocationBarDataProvider(locationBarDataProvider);
} }
/**
* @param layoutStateProvider A means of accessing the current Layout state and a way to
* listen to state changes.
*/
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
mMediator.setLayoutStateProvider(layoutStateProvider);
}
/** /**
* Updates the profile used for generating autocomplete suggestions. * Updates the profile used for generating autocomplete suggestions.
* @param profile The profile to be used. * @param profile The profile to be used.
......
...@@ -34,8 +34,6 @@ import org.chromium.chrome.browser.app.tabmodel.TabWindowManagerSingleton; ...@@ -34,8 +34,6 @@ import org.chromium.chrome.browser.app.tabmodel.TabWindowManagerSingleton;
import org.chromium.chrome.browser.document.ChromeIntentUtil; import org.chromium.chrome.browser.document.ChromeIntentUtil;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.init.AsyncInitializationActivity; import org.chromium.chrome.browser.init.AsyncInitializationActivity;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher; import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver; import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider; import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
...@@ -94,8 +92,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -94,8 +92,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
private AutocompleteResult mAutocompleteResult; private AutocompleteResult mAutocompleteResult;
private LocationBarDataProvider mDataProvider; private LocationBarDataProvider mDataProvider;
private LayoutStateProvider mLayoutStateProvider;
private LayoutStateProvider.LayoutStateObserver mLayoutStateObserver;
private boolean mNativeInitialized; private boolean mNativeInitialized;
private AutocompleteController mAutocomplete; private AutocompleteController mAutocomplete;
...@@ -161,17 +157,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -161,17 +157,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
mAutocompleteResult = new AutocompleteResult(null, null); mAutocompleteResult = new AutocompleteResult(null, null);
mDropdownViewInfoListBuilder = new DropdownItemViewInfoListBuilder(mAutocomplete); mDropdownViewInfoListBuilder = new DropdownItemViewInfoListBuilder(mAutocomplete);
mDropdownViewInfoListManager = new DropdownItemViewInfoListManager(mSuggestionModels); mDropdownViewInfoListManager = new DropdownItemViewInfoListManager(mSuggestionModels);
mLayoutStateObserver = new LayoutStateProvider.LayoutStateObserver() {
@Override
public void onStartedShowing(@LayoutType int layoutType, boolean showToolbar) {
if (!mNativeInitialized || layoutType != LayoutType.TAB_SWITCHER) return;
if (mDataProvider.shouldShowLocationBarInOverviewMode()) {
AutocompleteControllerJni.get().prefetchZeroSuggestResults();
}
}
};
} }
/** /**
...@@ -196,10 +181,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -196,10 +181,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
if (mTabObserver != null) { if (mTabObserver != null) {
mTabObserver.destroy(); mTabObserver.destroy();
} }
if (mLayoutStateProvider != null) {
mLayoutStateProvider.removeObserver(mLayoutStateObserver);
mLayoutStateProvider = null;
}
} }
@Override @Override
...@@ -266,17 +247,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi ...@@ -266,17 +247,6 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
mDataProvider = provider; mDataProvider = provider;
} }
/**
* @param layoutStateProvider A means of accessing the current Layout state and a way to
* listen to state changes.
*/
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
assert mLayoutStateProvider == null;
mLayoutStateProvider = layoutStateProvider;
mLayoutStateProvider.addObserver(mLayoutStateObserver);
}
/** Set the WindowAndroid instance associated with the containing Activity. */ /** Set the WindowAndroid instance associated with the containing Activity. */
void setWindowAndroid(WindowAndroid window) { void setWindowAndroid(WindowAndroid window) {
if (mLifecycleDispatcher != null) { if (mLifecycleDispatcher != null) {
......
...@@ -55,11 +55,6 @@ class SearchBoxDataProvider implements LocationBarDataProvider { ...@@ -55,11 +55,6 @@ class SearchBoxDataProvider implements LocationBarDataProvider {
return false; return false;
} }
@Override
public boolean shouldShowLocationBarInOverviewMode() {
return false;
}
@Override @Override
public Profile getProfile() { public Profile getProfile() {
return mTab != null ? Profile.fromWebContents(mTab.getWebContents()) : null; return mTab != null ? Profile.fromWebContents(mTab.getWebContents()) : null;
......
...@@ -660,6 +660,10 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve ...@@ -660,6 +660,10 @@ public class ToolbarManager implements UrlFocusChangeListener, ThemeColorObserve
if (layoutType == LayoutType.TAB_SWITCHER) { if (layoutType == LayoutType.TAB_SWITCHER) {
mToolbar.setTabSwitcherMode(true, showToolbar, false); mToolbar.setTabSwitcherMode(true, showToolbar, false);
updateButtonStatus(); updateButtonStatus();
if (mLocationBarModel.shouldShowLocationBarInOverviewMode()) {
assert mLocationBar instanceof LocationBarCoordinator;
((LocationBarCoordinator) mLocationBar).startAutocompletePrefetch();
}
} }
} }
......
...@@ -28,7 +28,6 @@ import org.chromium.base.TraceEvent; ...@@ -28,7 +28,6 @@ import org.chromium.base.TraceEvent;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.compositor.overlays.toolbar.TopToolbarOverlayCoordinator; import org.chromium.chrome.browser.compositor.overlays.toolbar.TopToolbarOverlayCoordinator;
import org.chromium.chrome.browser.findinpage.FindToolbar; import org.chromium.chrome.browser.findinpage.FindToolbar;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.LocationBarCoordinator; import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
import org.chromium.chrome.browser.omnibox.OmniboxFocusReason; import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
...@@ -587,8 +586,6 @@ public abstract class ToolbarLayout ...@@ -587,8 +586,6 @@ public abstract class ToolbarLayout
void setLayoutUpdater(Runnable layoutUpdater) {} void setLayoutUpdater(Runnable layoutUpdater) {}
void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {}
/** /**
* @param attached Whether or not the web content is attached to the view heirarchy. * @param attached Whether or not the web content is attached to the view heirarchy.
*/ */
......
...@@ -50,7 +50,6 @@ import org.chromium.base.TraceEvent; ...@@ -50,7 +50,6 @@ import org.chromium.base.TraceEvent;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.LocationBarCoordinator; import org.chromium.chrome.browser.omnibox.LocationBarCoordinator;
import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils; import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils;
...@@ -1547,11 +1546,6 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC ...@@ -1547,11 +1546,6 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
mLayoutUpdater = layoutUpdater; mLayoutUpdater = layoutUpdater;
} }
@Override
public void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
mLocationBar.setLayoutStateProvider(layoutStateProvider);
}
@Override @Override
public void finishAnimations() { public void finishAnimations() {
mClipRect = null; mClipRect = null;
......
...@@ -14,7 +14,6 @@ import androidx.annotation.Nullable; ...@@ -14,7 +14,6 @@ import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.CallbackController;
import org.chromium.base.supplier.ObservableSupplier; import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.OneShotCallback; import org.chromium.base.supplier.OneShotCallback;
import org.chromium.base.supplier.OneshotSupplier; import org.chromium.base.supplier.OneshotSupplier;
...@@ -87,7 +86,6 @@ public class TopToolbarCoordinator implements Toolbar { ...@@ -87,7 +86,6 @@ public class TopToolbarCoordinator implements Toolbar {
private MenuButtonCoordinator mMenuButtonCoordinator; private MenuButtonCoordinator mMenuButtonCoordinator;
private ObservableSupplier<AppMenuButtonHelper> mAppMenuButtonHelperSupplier; private ObservableSupplier<AppMenuButtonHelper> mAppMenuButtonHelperSupplier;
private CallbackController mCallbackController = new CallbackController();
private ObservableSupplier<TabModelSelector> mTabModelSelectorSupplier; private ObservableSupplier<TabModelSelector> mTabModelSelectorSupplier;
private TopToolbarOverlayCoordinator mOverlayCoordinator; private TopToolbarOverlayCoordinator mOverlayCoordinator;
...@@ -138,9 +136,6 @@ public class TopToolbarCoordinator implements Toolbar { ...@@ -138,9 +136,6 @@ public class TopToolbarCoordinator implements Toolbar {
controlContainer.getProgressBarDrawingInfo(info); controlContainer.getProgressBarDrawingInfo(info);
}; };
layoutStateProviderSupplier.onAvailable(
mCallbackController.makeCancelable(this::setLayoutStateProvider));
mTabModelSelectorSupplier = tabModelSelectorSupplier; mTabModelSelectorSupplier = tabModelSelectorSupplier;
if (mToolbarLayout instanceof ToolbarPhone) { if (mToolbarLayout instanceof ToolbarPhone) {
...@@ -230,11 +225,6 @@ public class TopToolbarCoordinator implements Toolbar { ...@@ -230,11 +225,6 @@ public class TopToolbarCoordinator implements Toolbar {
} }
} }
private void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
assert layoutStateProvider != null;
mToolbarLayout.setLayoutStateProvider(layoutStateProvider);
}
/** /**
* @param urlExpansionObserver The observer that observes URL expansion progress change. * @param urlExpansionObserver The observer that observes URL expansion progress change.
*/ */
...@@ -272,11 +262,6 @@ public class TopToolbarCoordinator implements Toolbar { ...@@ -272,11 +262,6 @@ public class TopToolbarCoordinator implements Toolbar {
mStartSurfaceToolbarCoordinator.destroy(); mStartSurfaceToolbarCoordinator.destroy();
} }
if (mCallbackController != null) {
mCallbackController.destroy();
mCallbackController = null;
}
if (mOptionalButtonController != null) { if (mOptionalButtonController != null) {
mOptionalButtonController.destroy(); mOptionalButtonController.destroy();
mOptionalButtonController = null; mOptionalButtonController = null;
......
...@@ -231,11 +231,6 @@ public class VoiceRecognitionHandlerTest { ...@@ -231,11 +231,6 @@ public class VoiceRecognitionHandlerTest {
return mIncognito; return mIncognito;
} }
@Override
public boolean shouldShowLocationBarInOverviewMode() {
return false;
}
@Override @Override
public boolean isInOverviewAndShowingOmnibox() { public boolean isInOverviewAndShowingOmnibox() {
return false; return false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment