Commit 38e66853 authored by Matt Jones's avatar Matt Jones Committed by Commit Bot

Replace ViewportSupplier with layout check

This patch removes the viewport type check for the TopToolbarOverlay
in place of checking specific layouts. The format in this patch is
much closer to what we want longer-term, especially with the
deprecation of the ViewportMode concept.

Bug: 1100332
Change-Id: Ief878a1c3b5b499cd411ab979d748cfd1477297a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350391
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarMei Liang <meiliang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805356}
parent 70901442
......@@ -18,7 +18,6 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.ObserverList;
import org.chromium.base.TraceEvent;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
......@@ -416,13 +415,9 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
// If fullscreen is disabled, don't bother creating this overlay; only the android view will
// ever be shown.
if (DeviceClassManager.enableFullscreen()) {
Supplier<Integer> viewportModeSupplier = ()
-> getActiveLayout() != null ? getActiveLayout().getViewportMode()
: Layout.ViewportMode.ALWAYS_FULLSCREEN;
mToolbarOverlay = new TopToolbarOverlayCoordinator(mContext, mFrameRequestSupplier,
this, controlContainer, tabProvider, getBrowserControlsManager(),
viewportModeSupplier, mAndroidViewShownSupplier,
() -> renderHost.getResourceManager());
mAndroidViewShownSupplier, () -> renderHost.getResourceManager());
}
// Initialize Layouts
......
......@@ -44,7 +44,6 @@ public class TopToolbarOverlayCoordinator implements SceneOverlay {
LayoutManager layoutManager, ControlContainer controlContainer,
ActivityTabProvider tabSupplier,
BrowserControlsStateProvider browserControlsStateProvider,
Supplier<Integer> viewportModeSupplier,
ObservableSupplier<Boolean> androidViewShownSupplier,
Supplier<ResourceManager> resourceManagerSupplier) {
mModel = new PropertyModel.Builder(TopToolbarOverlayProperties.ALL_KEYS)
......@@ -60,8 +59,7 @@ public class TopToolbarOverlayCoordinator implements SceneOverlay {
mModel, mSceneLayer, TopToolbarSceneLayer::bind, frameRequestSupplier, true);
mMediator = new TopToolbarOverlayMediator(mModel, context, layoutManager, controlContainer,
tabSupplier, browserControlsStateProvider, viewportModeSupplier,
androidViewShownSupplier);
tabSupplier, browserControlsStateProvider, androidViewShownSupplier);
}
/** Clean up this component. */
......
......@@ -11,13 +11,14 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
import org.chromium.chrome.browser.compositor.layouts.SceneChangeObserver;
import org.chromium.chrome.browser.compositor.layouts.ToolbarSwipeLayout;
import org.chromium.chrome.browser.compositor.layouts.phone.StackLayout;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabObserver;
......@@ -58,9 +59,6 @@ public class TopToolbarOverlayMediator {
/** An observer of the browser controls offsets. */
private final BrowserControlsStateProvider.Observer mBrowserControlsObserver;
/** A means of accessing the current viewport mode. */
private final Supplier<Integer> mViewportModeSupplier;
/** A means of checking whether the toolbar android view is being force-hidden or shown. */
private final ObservableSupplier<Boolean> mAndroidViewShownSupplier;
......@@ -73,17 +71,18 @@ public class TopToolbarOverlayMediator {
/** The last non-null tab. */
private Tab mLastActiveTab;
/** Whether the active layout has its own toolbar to display instead of this one. */
private boolean mLayoutHasOwnToolbar;
TopToolbarOverlayMediator(PropertyModel model, Context context, LayoutManager layoutManager,
ControlContainer controlContainer, ActivityTabProvider tabSupplier,
BrowserControlsStateProvider browserControlsStateProvider,
Supplier<Integer> viewportModeSupplier,
ObservableSupplier<Boolean> androidViewShownSupplier) {
mContext = context;
mLayoutManager = layoutManager;
mToolbarContainer = controlContainer;
mTabSupplier = tabSupplier;
mBrowserControlsStateProvider = browserControlsStateProvider;
mViewportModeSupplier = viewportModeSupplier;
mAndroidViewShownSupplier = androidViewShownSupplier;
mModel = model;
......@@ -93,6 +92,11 @@ public class TopToolbarOverlayMediator {
@Override
public void onSceneChange(Layout layout) {
// TODO(1100332): Use layout IDs instead of type checking when they are available.
// TODO(1100332): Once ToolbarSwipeLayout uses a SceneLayer that does not include
// its own toolbar, only check for the vertical tab switcher.
mLayoutHasOwnToolbar =
layout instanceof StackLayout || layout instanceof ToolbarSwipeLayout;
updateVisibility();
}
};
......@@ -238,7 +242,7 @@ public class TopToolbarOverlayMediator {
private void updateVisibility() {
mModel.set(TopToolbarOverlayProperties.VISIBLE,
!BrowserControlsUtils.areBrowserControlsOffScreen(mBrowserControlsStateProvider)
&& mViewportModeSupplier.get() != Layout.ViewportMode.ALWAYS_FULLSCREEN);
&& !mLayoutHasOwnToolbar);
}
/** @return Whether this overlay should be attached to the tree. */
......
......@@ -20,12 +20,10 @@ import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.compositor.layouts.Layout.ViewportMode;
import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabImpl;
......@@ -51,9 +49,6 @@ public class TopToolbarOverlayMediatorTest {
@Mock
private BrowserControlsStateProvider mBrowserControlsProvider;
@Mock
private ObservableSupplier<Integer> mViewportModeSupplier;
@Mock
private TabImpl mTab;
......@@ -81,8 +76,6 @@ public class TopToolbarOverlayMediatorTest {
mAndroidViewShownSupplier = new ObservableSupplierImpl<>();
mAndroidViewShownSupplier.set(true);
when(mViewportModeSupplier.get()).thenReturn(ViewportMode.DYNAMIC_BROWSER_CONTROLS);
TopToolbarOverlayMediator.setToolbarBackgroundColorForTesting(Color.RED);
TopToolbarOverlayMediator.setUrlBarColorForTesting(Color.BLUE);
TopToolbarOverlayMediator.setIsTabletForTesting(false);
......@@ -98,9 +91,9 @@ public class TopToolbarOverlayMediatorTest {
.with(TopToolbarOverlayProperties.PROGRESS_BAR_INFO, null)
.build();
mMediator = new TopToolbarOverlayMediator(mModel, mContext, mLayoutManager,
mControlContainer, mTabSupplier, mBrowserControlsProvider, mViewportModeSupplier,
mAndroidViewShownSupplier);
mMediator =
new TopToolbarOverlayMediator(mModel, mContext, mLayoutManager, mControlContainer,
mTabSupplier, mBrowserControlsProvider, mAndroidViewShownSupplier);
// Ensure the observer is added to the initial tab.
verify(mTabSupplier).addObserverAndTrigger(mActivityTabObserverCaptor.capture());
......
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