Commit 37068acd authored by Filip Gorski's avatar Filip Gorski Committed by Commit Bot

Revert "Simplify ActivityTabProvider implementation"

This reverts commit 54ff0ea1.

Original patch created a problem around properly switching themes,
because of timing of switching observer to a new tab (previously
available with a hint only.

Bug: 1056586, 1054072
Change-Id: Iec9267e6eadd87725bf4609117b8d4d31a2fd24e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080692
Commit-Queue: Filip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746038}
parent fcec3175
...@@ -13,6 +13,7 @@ import androidx.annotation.Nullable; ...@@ -13,6 +13,7 @@ import androidx.annotation.Nullable;
import org.chromium.base.BuildInfo; import org.chromium.base.BuildInfo;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.IntentUtils; import org.chromium.base.IntentUtils;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.autofill_assistant.metrics.DropOutReason; import org.chromium.chrome.browser.autofill_assistant.metrics.DropOutReason;
import org.chromium.chrome.browser.directactions.DirectActionHandler; import org.chromium.chrome.browser.directactions.DirectActionHandler;
...@@ -207,15 +208,16 @@ public class AutofillAssistantFacade { ...@@ -207,15 +208,16 @@ public class AutofillAssistantFacade {
} }
// The tab is not yet available. We need to register as listener and wait for it. // The tab is not yet available. We need to register as listener and wait for it.
activity.getActivityTabProvider().addObserver(new Callback<Tab>() { activity.getActivityTabProvider().addObserverAndTrigger(
@Override new ActivityTabProvider.HintlessActivityTabObserver() {
public void onResult(Tab tab) { @Override
if (tab == null) return; public void onActivityTabChanged(Tab tab) {
activity.getActivityTabProvider().removeObserver(this); if (tab == null) return;
assert tab.getWebContents() != null; activity.getActivityTabProvider().removeObserver(this);
callback.onResult(tab); assert tab.getWebContents() != null;
} callback.onResult(tab);
}); }
});
} }
public static boolean isAutofillAssistantEnabled(Intent intent) { public static boolean isAutofillAssistantEnabled(Intent intent) {
......
...@@ -132,7 +132,6 @@ public class ManualFillingControllerTest { ...@@ -132,7 +132,6 @@ public class ManualFillingControllerTest {
private static class MockActivityTabProvider extends ActivityTabProvider { private static class MockActivityTabProvider extends ActivityTabProvider {
public Tab mTab; public Tab mTab;
@Override
public void set(Tab tab) { public void set(Tab tab) {
mTab = tab; mTab = tab;
} }
......
...@@ -7,8 +7,9 @@ package org.chromium.chrome.browser; ...@@ -7,8 +7,9 @@ package org.chromium.chrome.browser;
import androidx.annotation.CallSuper; import androidx.annotation.CallSuper;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback; import org.chromium.base.ObserverList;
import org.chromium.base.supplier.ObservableSupplierImpl; import org.chromium.base.ObserverList.RewindableIterator;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.compositor.layouts.Layout; import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.compositor.layouts.LayoutManager; import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
import org.chromium.chrome.browser.compositor.layouts.SceneChangeObserver; import org.chromium.chrome.browser.compositor.layouts.SceneChangeObserver;
...@@ -17,13 +18,44 @@ import org.chromium.chrome.browser.compositor.layouts.phone.SimpleAnimationLayou ...@@ -17,13 +18,44 @@ import org.chromium.chrome.browser.compositor.layouts.phone.SimpleAnimationLayou
import org.chromium.chrome.browser.tab.EmptyTabObserver; import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabSelectionType; import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver; import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabModelObserver;
/** /**
* A class that provides the current {@link Tab} for various states of the browser's activity. * A class that provides the current {@link Tab} for various states of the browser's activity.
*/ */
public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { public class ActivityTabProvider implements Supplier<Tab> {
/** An interface to track the visible tab for the activity. */
public interface ActivityTabObserver {
/**
* A notification that the activity's tab has changed. This will be triggered whenever a
* different tab is selected by the active {@link TabModel} and when that tab is
* interactive (i.e. not in a tab switching mode). When switching to toolbar swipe or tab
* switcher, this method will be called with {@code null} to indicate that there is no
* single activity tab (observers may or may not choose to ignore this event).
* @param tab The {@link Tab} that became visible or null if not in {@link StaticLayout}.
* @param hint Whether the change event is a hint that a tab change is likely. If true, the
* provided tab may still be frozen and is not yet selected.
*/
void onActivityTabChanged(Tab tab, boolean hint);
}
/** An {@link ActivityTabObserver} that can be used to explicitly watch non-hint events. */
public abstract static class HintlessActivityTabObserver implements ActivityTabObserver {
@Override
public final void onActivityTabChanged(Tab tab, boolean hint) {
// Only pass the event through if it isn't a hint.
if (!hint) onActivityTabChanged(tab);
}
/**
* A notification that the {@link Tab} in the {@link StaticLayout} has changed.
* @param tab The activity's tab.
*/
public abstract void onActivityTabChanged(Tab tab);
}
/** /**
* A utility class for observing the activity tab via {@link TabObserver}. When the activity * A utility class for observing the activity tab via {@link TabObserver}. When the activity
* tab changes, the observer is switched to that tab. * tab changes, the observer is switched to that tab.
...@@ -33,7 +65,7 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -33,7 +65,7 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
private final ActivityTabProvider mTabProvider; private final ActivityTabProvider mTabProvider;
/** An observer to watch for a changing activity tab and move this tab observer. */ /** An observer to watch for a changing activity tab and move this tab observer. */
private final Callback<Tab> mActivityTabObserver; private final ActivityTabObserver mActivityTabObserver;
/** The current activity tab. */ /** The current activity tab. */
private Tab mTab; private Tab mTab;
...@@ -44,12 +76,12 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -44,12 +76,12 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
*/ */
public ActivityTabTabObserver(ActivityTabProvider tabProvider) { public ActivityTabTabObserver(ActivityTabProvider tabProvider) {
mTabProvider = tabProvider; mTabProvider = tabProvider;
mActivityTabObserver = (tab) -> { mActivityTabObserver = (tab, hint) -> {
updateObservedTab(tab); updateObservedTab(tab);
onObservingDifferentTab(tab); onObservingDifferentTab(tab);
}; };
addObserverToTabProvider(); mTabProvider.addObserver(mActivityTabObserver);
updateObservedTabToCurrent(); updateObservedTab(mTabProvider.get());
} }
/** /**
...@@ -78,24 +110,21 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -78,24 +110,21 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
mTab.removeObserver(this); mTab.removeObserver(this);
mTab = null; mTab = null;
} }
removeObserverFromTabProvider(); mTabProvider.removeObserver(mActivityTabObserver);
} }
}
@VisibleForTesting /** The list of observers to send events to. */
protected void updateObservedTabToCurrent() { private final ObserverList<ActivityTabObserver> mObservers = new ObserverList<>();
updateObservedTab(mTabProvider.get());
}
@VisibleForTesting /**
protected void addObserverToTabProvider() { * A single rewindable iterator bound to {@link #mObservers} to prevent constant allocation of
mTabProvider.addObserver(mActivityTabObserver); * new iterators.
} */
private final RewindableIterator<ActivityTabObserver> mRewindableIterator;
@VisibleForTesting /** The {@link Tab} that is considered to be the activity's tab. */
protected void removeObserverFromTabProvider() { private Tab mActivityTab;
mTabProvider.removeObserver(mActivityTabObserver);
}
}
/** A handle to the {@link LayoutManager} to get the active layout. */ /** A handle to the {@link LayoutManager} to get the active layout. */
private LayoutManager mLayoutManager; private LayoutManager mLayoutManager;
...@@ -109,11 +138,25 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -109,11 +138,25 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
/** An observer for watching tab creation and switching events. */ /** An observer for watching tab creation and switching events. */
private TabModelSelectorTabModelObserver mTabModelObserver; private TabModelSelectorTabModelObserver mTabModelObserver;
/** Default constructor. */ /** The last tab ID that was hinted. This is reset when the activity tab actually changes. */
private int mLastHintedTabId;
/**
* Default constructor.
*/
public ActivityTabProvider() { public ActivityTabProvider() {
mRewindableIterator = mObservers.rewindableIterator();
mSceneChangeObserver = new SceneChangeObserver() { mSceneChangeObserver = new SceneChangeObserver() {
@Override @Override
public void onTabSelectionHinted(int tabId) {} public void onTabSelectionHinted(int tabId) {
if (mTabModelSelector == null || mLastHintedTabId == tabId) return;
Tab tab = mTabModelSelector.getTabById(tabId);
mLastHintedTabId = tabId;
mRewindableIterator.rewind();
while (mRewindableIterator.hasNext()) {
mRewindableIterator.next().onActivityTabChanged(tab, true);
}
}
@Override @Override
public void onSceneChange(Layout layout) { public void onSceneChange(Layout layout) {
...@@ -130,6 +173,14 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -130,6 +173,14 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
}; };
} }
/**
* @return The activity's current tab.
*/
@Override
public Tab get() {
return mActivityTab;
}
/** /**
* @param selector A {@link TabModelSelector} for watching for changes in tabs. * @param selector A {@link TabModelSelector} for watching for changes in tabs.
*/ */
...@@ -172,11 +223,50 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> { ...@@ -172,11 +223,50 @@ public class ActivityTabProvider extends ObservableSupplierImpl<Tab> {
return; return;
} }
set(tab); if (mActivityTab == tab) return;
mActivityTab = tab;
mLastHintedTabId = Tab.INVALID_TAB_ID;
mRewindableIterator.rewind();
while (mRewindableIterator.hasNext()) {
mRewindableIterator.next().onActivityTabChanged(tab, false);
}
}
/**
* Add an observer but do not immediately trigger the event. This should only be used in
* extremely specific cases where the observer would trigger an event from the constructor of
* the implementing class (see {@link ActivityTabTabObserver}).
* @param observer The observer to be added.
*
* TODO(fgorski): Find a different way to mock this in tests for {@link LoadProgressMediator}.
*/
@VisibleForTesting
@Deprecated
public void addObserver(ActivityTabObserver observer) {
mObservers.addObserver(observer);
}
/**
* @param observer The {@link ActivityTabObserver} to add to the activity. This will trigger the
* {@link ActivityTabObserver#onActivityTabChanged(Tab, boolean)} event to be
* called on the added observer, providing access to the current tab.
*/
public void addObserverAndTrigger(ActivityTabObserver observer) {
mObservers.addObserver(observer);
observer.onActivityTabChanged(mActivityTab, false);
}
/**
* @param observer The {@link ActivityTabObserver} to remove from the activity.
*/
public void removeObserver(ActivityTabObserver observer) {
mObservers.removeObserver(observer);
} }
/** Clean up and detach any observers this object created. */ /** Clean up and detach any observers this object created. */
public void destroy() { public void destroy() {
mObservers.clear();
if (mLayoutManager != null) mLayoutManager.removeSceneChangeObserver(mSceneChangeObserver); if (mLayoutManager != null) mLayoutManager.removeSceneChangeObserver(mSceneChangeObserver);
mLayoutManager = null; mLayoutManager = null;
if (mTabModelObserver != null) mTabModelObserver.destroy(); if (mTabModelObserver != null) mTabModelObserver.destroy();
......
...@@ -18,6 +18,7 @@ import androidx.browser.trusted.sharing.ShareTarget; ...@@ -18,6 +18,7 @@ import androidx.browser.trusted.sharing.ShareTarget;
import org.chromium.base.IntentUtils; import org.chromium.base.IntentUtils;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ActivityTabProvider.HintlessActivityTabObserver;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.ServiceTabLauncher; import org.chromium.chrome.browser.ServiceTabLauncher;
...@@ -105,6 +106,14 @@ public class CustomTabActivityTabController ...@@ -105,6 +106,14 @@ public class CustomTabActivityTabController
private final CustomTabsSessionToken mSession; private final CustomTabsSessionToken mSession;
private final Intent mIntent; private final Intent mIntent;
@Nullable
private HintlessActivityTabObserver mTabSwapObserver = new HintlessActivityTabObserver() {
@Override
public void onActivityTabChanged(@Nullable Tab tab) {
mTabProvider.swapTab(tab);
}
};
@Inject @Inject
public CustomTabActivityTabController(ChromeActivity<?> activity, public CustomTabActivityTabController(ChromeActivity<?> activity,
Lazy<CustomTabDelegateFactory> customTabDelegateFactory, Lazy<CustomTabDelegateFactory> customTabDelegateFactory,
...@@ -328,7 +337,7 @@ public class CustomTabActivityTabController ...@@ -328,7 +337,7 @@ public class CustomTabActivityTabController
} // else we've already set the initial tab. } // else we've already set the initial tab.
// Listen to tab swapping and closing. // Listen to tab swapping and closing.
mActivityTabProvider.addObserver(mTabProvider::swapTab); mActivityTabProvider.addObserverAndTrigger(mTabSwapObserver);
} }
@Nullable @Nullable
......
...@@ -12,6 +12,7 @@ import org.chromium.base.Callback; ...@@ -12,6 +12,7 @@ import org.chromium.base.Callback;
import org.chromium.base.supplier.ObservableSupplier; import org.chromium.base.supplier.ObservableSupplier;
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.ActivityTabProvider.HintlessActivityTabObserver;
import org.chromium.chrome.browser.ThemeColorProvider; import org.chromium.chrome.browser.ThemeColorProvider;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior; import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.feature_engagement.TrackerFactory; import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
...@@ -140,9 +141,9 @@ public class BrowsingModeBottomToolbarCoordinator { ...@@ -140,9 +141,9 @@ public class BrowsingModeBottomToolbarCoordinator {
* TabSwitcherButtonView} just passes null. * TabSwitcherButtonView} just passes null.
*/ */
void setupIPH(@FeatureConstants String feature, View anchor, OnClickListener listener) { void setupIPH(@FeatureConstants String feature, View anchor, OnClickListener listener) {
mTabProvider.addObserver(new Callback<Tab>() { mTabProvider.addObserverAndTrigger(new HintlessActivityTabObserver() {
@Override @Override
public void onResult(Tab tab) { public void onActivityTabChanged(Tab tab) {
if (tab == null) return; if (tab == null) return;
TabImpl tabImpl = (TabImpl) tab; TabImpl tabImpl = (TabImpl) tab;
final Tracker tracker = TrackerFactory.getTrackerForProfile(tabImpl.getProfile()); final Tracker tracker = TrackerFactory.getTrackerForProfile(tabImpl.getProfile());
......
...@@ -10,6 +10,7 @@ import android.os.Bundle; ...@@ -10,6 +10,7 @@ import android.os.Bundle;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ActivityTabProvider.HintlessActivityTabObserver;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.WebContentsFactory; import org.chromium.chrome.browser.WebContentsFactory;
import org.chromium.chrome.browser.browserservices.BrowserServicesActivityTabController; import org.chromium.chrome.browser.browserservices.BrowserServicesActivityTabController;
...@@ -45,6 +46,13 @@ public class WebappActivityTabController implements BrowserServicesActivityTabCo ...@@ -45,6 +46,13 @@ public class WebappActivityTabController implements BrowserServicesActivityTabCo
private final WebContentsFactory mWebContentsFactory; private final WebContentsFactory mWebContentsFactory;
private final CustomTabActivityTabProvider mTabProvider; private final CustomTabActivityTabProvider mTabProvider;
private HintlessActivityTabObserver mTabSwapObserver = new HintlessActivityTabObserver() {
@Override
public void onActivityTabChanged(@Nullable Tab tab) {
mTabProvider.swapTab(tab);
}
};
@Inject @Inject
public WebappActivityTabController(ChromeActivity<?> activity, public WebappActivityTabController(ChromeActivity<?> activity,
Lazy<CustomTabDelegateFactory> tabDelegateFactory, Lazy<CustomTabDelegateFactory> tabDelegateFactory,
...@@ -131,7 +139,7 @@ public class WebappActivityTabController implements BrowserServicesActivityTabCo ...@@ -131,7 +139,7 @@ public class WebappActivityTabController implements BrowserServicesActivityTabCo
mTabProvider.setInitialTab(tab, mode); mTabProvider.setInitialTab(tab, mode);
// Listen to tab swapping and closing. // Listen to tab swapping and closing.
mActivityTabProvider.addObserver(mTabProvider::swapTab); mActivityTabProvider.addObserverAndTrigger(mTabSwapObserver);
} }
@Nullable @Nullable
......
...@@ -12,6 +12,7 @@ import androidx.annotation.VisibleForTesting; ...@@ -12,6 +12,7 @@ import androidx.annotation.VisibleForTesting;
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.ActivityTabProvider.HintlessActivityTabObserver;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel; import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager; import org.chromium.chrome.browser.compositor.bottombar.OverlayPanelManager;
import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager; import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
...@@ -134,9 +135,6 @@ public class BottomSheetController implements Destroyable { ...@@ -134,9 +135,6 @@ public class BottomSheetController implements Destroyable {
/** The last known activity tab, if available. */ /** The last known activity tab, if available. */
private Tab mLastActivityTab; private Tab mLastActivityTab;
/** A tab observer to watch the last known activity tab. */
private TabObserver mTabObserver;
/** A runnable that initializes the bottom sheet when necessary. */ /** A runnable that initializes the bottom sheet when necessary. */
private Runnable mSheetInitializer; private Runnable mSheetInitializer;
...@@ -256,7 +254,7 @@ public class BottomSheetController implements Destroyable { ...@@ -256,7 +254,7 @@ public class BottomSheetController implements Destroyable {
VrModuleProvider.registerVrModeObserver(mVrModeObserver); VrModuleProvider.registerVrModeObserver(mVrModeObserver);
mTabObserver = new EmptyTabObserver() { final TabObserver tabObserver = new EmptyTabObserver() {
@Override @Override
public void onPageLoadStarted(Tab tab, String url) { public void onPageLoadStarted(Tab tab, String url) {
clearRequestsAndHide(); clearRequestsAndHide();
...@@ -278,8 +276,29 @@ public class BottomSheetController implements Destroyable { ...@@ -278,8 +276,29 @@ public class BottomSheetController implements Destroyable {
} }
}; };
mTabProvider.addObserver(this::setActivityTab); mTabProvider.addObserverAndTrigger(new HintlessActivityTabObserver() {
setActivityTab(mTabProvider.get()); @Override
public void onActivityTabChanged(Tab tab) {
// Temporarily suppress the sheet if entering a state where there is no activity
// tab.
if (tab == null) {
suppressSheet(StateChangeReason.COMPOSITED_UI);
return;
}
// If refocusing the same tab, simply unsuppress the sheet.
if (mLastActivityTab == tab) {
unsuppressSheet();
return;
}
// Move the observer to the new activity tab and clear the sheet.
if (mLastActivityTab != null) mLastActivityTab.removeObserver(tabObserver);
mLastActivityTab = tab;
mLastActivityTab.addObserver(tabObserver);
clearRequestsAndHide();
}
});
ScrimObserver scrimObserver = new ScrimObserver() { ScrimObserver scrimObserver = new ScrimObserver() {
@Override @Override
...@@ -346,26 +365,6 @@ public class BottomSheetController implements Destroyable { ...@@ -346,26 +365,6 @@ public class BottomSheetController implements Destroyable {
mSheetInitializer = null; mSheetInitializer = null;
} }
private void setActivityTab(Tab tab) {
// Temporarily suppress the sheet if entering a state where there is no activity tab.
if (tab == null) {
suppressSheet(StateChangeReason.COMPOSITED_UI);
return;
}
// If refocusing the same tab, simply unsuppress the sheet.
if (mLastActivityTab == tab) {
unsuppressSheet();
return;
}
// Move the observer to the new activity tab and clear the sheet.
if (mLastActivityTab != null) mLastActivityTab.removeObserver(mTabObserver);
mLastActivityTab = tab;
mLastActivityTab.addObserver(mTabObserver);
clearRequestsAndHide();
}
/** /**
* Create a ScrimParams anchoring on the bottom-sheet view. * Create a ScrimParams anchoring on the bottom-sheet view.
* @param scrimObserver The scrimObserver to set for the ScrimParams. * @param scrimObserver The scrimObserver to set for the ScrimParams.
......
...@@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals; ...@@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotEquals;
import android.support.test.InstrumentationRegistry; import android.support.test.InstrumentationRegistry;
import android.support.test.filters.LargeTest;
import android.support.test.filters.SmallTest; import android.support.test.filters.SmallTest;
import org.junit.Before; import org.junit.Before;
...@@ -41,32 +42,22 @@ import java.util.concurrent.TimeoutException; ...@@ -41,32 +42,22 @@ import java.util.concurrent.TimeoutException;
public class ActivityTabProviderTest { public class ActivityTabProviderTest {
/** A test observer that provides access to the tab being observed. */ /** A test observer that provides access to the tab being observed. */
private static class TestActivityTabTabObserver extends ActivityTabTabObserver { private static class TestActivityTabTabObserver extends ActivityTabTabObserver {
/** Callback helper for notification that the observer is watching a different tab. */
private CallbackHelper mObserverMoveHelper;
/** The tab currently being observed. */ /** The tab currently being observed. */
private Tab mObservedTab; private Tab mObservedTab;
public TestActivityTabTabObserver(ActivityTabProvider provider) { public TestActivityTabTabObserver(ActivityTabProvider provider) {
super(provider); super(provider);
TestThreadUtils.runOnUiThreadBlockingNoException(() -> mObservedTab = provider.get()); mObserverMoveHelper = new CallbackHelper();
mObservedTab = provider.get();
} }
@Override @Override
public void onObservingDifferentTab(Tab tab) { public void onObservingDifferentTab(Tab tab) {
mObservedTab = tab; mObservedTab = tab;
} mObserverMoveHelper.notifyCalled();
@Override
protected void updateObservedTabToCurrent() {
TestThreadUtils.runOnUiThreadBlocking(super::updateObservedTabToCurrent);
}
@Override
protected void addObserverToTabProvider() {
TestThreadUtils.runOnUiThreadBlocking(super::addObserverToTabProvider);
}
@Override
protected void removeObserverFromTabProvider() {
TestThreadUtils.runOnUiThreadBlocking(super::removeObserverFromTabProvider);
} }
} }
...@@ -77,21 +68,23 @@ public class ActivityTabProviderTest { ...@@ -77,21 +68,23 @@ public class ActivityTabProviderTest {
private ActivityTabProvider mProvider; private ActivityTabProvider mProvider;
private Tab mActivityTab; private Tab mActivityTab;
private CallbackHelper mActivityTabChangedHelper = new CallbackHelper(); private CallbackHelper mActivityTabChangedHelper = new CallbackHelper();
private CallbackHelper mActivityTabChangedHintHelper = new CallbackHelper();
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
mActivityTestRule.startMainActivityOnBlankPage(); mActivityTestRule.startMainActivityOnBlankPage();
TestThreadUtils.runOnUiThreadBlocking(() -> { mActivity = mActivityTestRule.getActivity();
mActivity = mActivityTestRule.getActivity(); mProvider = mActivity.getActivityTabProvider();
mProvider = mActivity.getActivityTabProvider(); mProvider.addObserverAndTrigger((tab, hint) -> {
mProvider.addObserver(tab -> { if (hint) {
mActivityTabChangedHintHelper.notifyCalled();
} else {
mActivityTab = tab; mActivityTab = tab;
mActivityTabChangedHelper.notifyCalled(); mActivityTabChangedHelper.notifyCalled();
}); }
}); });
mActivityTabChangedHelper.waitForCallback(0); assertEquals("Setup should have only triggered the event once.",
assertEquals("Setup should have only triggered the event once.", 1, mActivityTabChangedHelper.getCallCount(), 1);
mActivityTabChangedHelper.getCallCount());
} }
/** /**
...@@ -110,8 +103,7 @@ public class ActivityTabProviderTest { ...@@ -110,8 +103,7 @@ public class ActivityTabProviderTest {
@Feature({"ActivityTabObserver"}) @Feature({"ActivityTabObserver"})
public void testTriggerOnAddObserver() throws TimeoutException { public void testTriggerOnAddObserver() throws TimeoutException {
CallbackHelper helper = new CallbackHelper(); CallbackHelper helper = new CallbackHelper();
TestThreadUtils.runOnUiThreadBlocking( mProvider.addObserverAndTrigger((tab, hint) -> helper.notifyCalled());
() -> { mProvider.addObserver(tab -> helper.notifyCalled()); });
helper.waitForCallback(0); helper.waitForCallback(0);
assertEquals("Only the added observer should have been triggered.", assertEquals("Only the added observer should have been triggered.",
...@@ -145,6 +137,27 @@ public class ActivityTabProviderTest { ...@@ -145,6 +137,27 @@ public class ActivityTabProviderTest {
mActivityTab); mActivityTab);
} }
/** Test that the hint event triggers when exiting the tab switcher. */
@Test
@LargeTest
@Feature({"ActivityTabObserver"})
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
public void testTriggerHintWithTabSwitcher() throws TimeoutException {
assertEquals("The hint should not yet have triggered.", 0,
mActivityTabChangedHintHelper.getCallCount());
setTabSwitcherModeAndWait(true);
assertEquals("The hint should not yet have triggered.", 0,
mActivityTabChangedHintHelper.getCallCount());
setTabSwitcherModeAndWait(false);
mActivityTabChangedHintHelper.waitForCallback(0);
assertEquals("The hint should have triggerd once.", 1,
mActivityTabChangedHintHelper.getCallCount());
}
/** /**
* Test that onActivityTabChanged is triggered when switching to a new tab without switching * Test that onActivityTabChanged is triggered when switching to a new tab without switching
* layouts. * layouts.
...@@ -220,7 +233,7 @@ public class ActivityTabProviderTest { ...@@ -220,7 +233,7 @@ public class ActivityTabProviderTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"ActivityTabObserver"}) @Feature({"ActivityTabObserver"})
public void testActivityTabTabObserver() throws TimeoutException { public void testActivityTabTabObserver() {
Tab startingTab = getModelSelectedTab(); Tab startingTab = getModelSelectedTab();
TestActivityTabTabObserver tabObserver = new TestActivityTabTabObserver(mProvider); TestActivityTabTabObserver tabObserver = new TestActivityTabTabObserver(mProvider);
......
...@@ -32,7 +32,6 @@ import org.chromium.chrome.test.util.ChromeTabUtils; ...@@ -32,7 +32,6 @@ import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.components.embedder_support.util.UrlConstants; import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.content_public.browser.test.util.Criteria; import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.content_public.browser.test.util.TouchCommon; import org.chromium.content_public.browser.test.util.TouchCommon;
import org.chromium.content_public.common.ContentUrlConstants; import org.chromium.content_public.common.ContentUrlConstants;
import org.chromium.net.test.EmbeddedTestServer; import org.chromium.net.test.EmbeddedTestServer;
...@@ -71,8 +70,7 @@ public class NavigationHandlerTest { ...@@ -71,8 +70,7 @@ public class NavigationHandlerTest {
} }
private Tab currentTab() { private Tab currentTab() {
return TestThreadUtils.runOnUiThreadBlockingNoException( return mActivityTestRule.getActivity().getActivityTabProvider().get();
() -> mActivityTestRule.getActivity().getActivityTabProvider().get());
} }
private void loadNewTabPage() { private void loadNewTabPage() {
......
...@@ -98,13 +98,7 @@ public class BottomSheetControllerTest { ...@@ -98,13 +98,7 @@ public class BottomSheetControllerTest {
/** @return The height of the container view. */ /** @return The height of the container view. */
private int getContainerHeight() { private int getContainerHeight() {
return TestThreadUtils.runOnUiThreadBlockingNoException( return mActivityTestRule.getActivity().getActivityTabProvider().get().getView().getHeight();
()
-> mActivityTestRule.getActivity()
.getActivityTabProvider()
.get()
.getView()
.getHeight());
} }
@Test @Test
......
...@@ -8,6 +8,7 @@ import static org.mockito.ArgumentMatchers.any; ...@@ -8,6 +8,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
...@@ -24,11 +25,11 @@ import org.mockito.Captor; ...@@ -24,11 +25,11 @@ import org.mockito.Captor;
import org.mockito.Mock; import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.chromium.base.Callback;
import org.chromium.base.UserDataHost; import org.chromium.base.UserDataHost;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ActivityTabProvider.ActivityTabObserver;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.WarmupManager; import org.chromium.chrome.browser.WarmupManager;
...@@ -100,7 +101,7 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher { ...@@ -100,7 +101,7 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher {
public final CustomTabActivityTabProvider tabProvider = new CustomTabActivityTabProvider(); public final CustomTabActivityTabProvider tabProvider = new CustomTabActivityTabProvider();
@Captor @Captor
public ArgumentCaptor<Callback<Tab>> activityTabObserverCaptor; public ArgumentCaptor<ActivityTabObserver> activityTabObserverCaptor;
// Captures the WebContents with which tabFromFactory is initialized // Captures the WebContents with which tabFromFactory is initialized
@Captor public ArgumentCaptor<WebContents> webContentsCaptor; @Captor public ArgumentCaptor<WebContents> webContentsCaptor;
...@@ -128,7 +129,9 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher { ...@@ -128,7 +129,9 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher {
when(startupTabPreloader.takeTabIfMatchingOrDestroy(any(), anyInt())).thenReturn(null); when(startupTabPreloader.takeTabIfMatchingOrDestroy(any(), anyInt())).thenReturn(null);
when(reparentingTaskProvider.get(any())).thenReturn(reparentingTask); when(reparentingTaskProvider.get(any())).thenReturn(reparentingTask);
when(activityTabProvider.addObserver(activityTabObserverCaptor.capture())).thenReturn(null); doNothing()
.when(activityTabProvider)
.addObserverAndTrigger(activityTabObserverCaptor.capture());
} }
@Override @Override
...@@ -178,8 +181,8 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher { ...@@ -178,8 +181,8 @@ public class CustomTabActivityContentTestEnvironment extends TestWatcher {
public void changeTab(Tab newTab) { public void changeTab(Tab newTab) {
when(activityTabProvider.get()).thenReturn(newTab); when(activityTabProvider.get()).thenReturn(newTab);
for (Callback<Tab> observer : activityTabObserverCaptor.getAllValues()) { for (ActivityTabObserver observer : activityTabObserverCaptor.getAllValues()) {
observer.onResult(newTab); observer.onActivityTabChanged(newTab, false);
} }
} }
......
...@@ -23,11 +23,10 @@ import org.mockito.MockitoAnnotations; ...@@ -23,11 +23,10 @@ import org.mockito.MockitoAnnotations;
import org.robolectric.Shadows; import org.robolectric.Shadows;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.chromium.base.Callback;
import org.chromium.base.MathUtils; import org.chromium.base.MathUtils;
import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.ActivityTabProvider.ActivityTabObserver;
import org.chromium.chrome.browser.tab.TabImpl; import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tab.TabObserver; import org.chromium.chrome.browser.tab.TabObserver;
import org.chromium.chrome.browser.toolbar.load_progress.LoadProgressProperties.CompletionState; import org.chromium.chrome.browser.toolbar.load_progress.LoadProgressProperties.CompletionState;
...@@ -51,7 +50,7 @@ public class LoadProgressMediatorTest { ...@@ -51,7 +50,7 @@ public class LoadProgressMediatorTest {
@Captor @Captor
public ArgumentCaptor<TabObserver> mTabObserverCaptor; public ArgumentCaptor<TabObserver> mTabObserverCaptor;
@Captor @Captor
public ArgumentCaptor<Callback<Tab>> mActivityTabObserverCaptor; public ArgumentCaptor<ActivityTabObserver> mActivityTabObserverCaptor;
private PropertyModel mModel; private PropertyModel mModel;
private LoadProgressMediator mMediator; private LoadProgressMediator mMediator;
...@@ -94,7 +93,7 @@ public class LoadProgressMediatorTest { ...@@ -94,7 +93,7 @@ public class LoadProgressMediatorTest {
public void switchToLoadingTab() { public void switchToLoadingTab() {
doReturn(true).when(mTab2).isLoading(); doReturn(true).when(mTab2).isLoading();
doReturn(0.1f).when(mTab2).getProgress(); doReturn(0.1f).when(mTab2).getProgress();
mActivityTabObserverCaptor.getValue().onResult(mTab2); mActivityTabObserverCaptor.getValue().onActivityTabChanged(mTab2, false);
verify(mTab2, times(1)).addObserver(any()); verify(mTab2, times(1)).addObserver(any());
assertEquals( assertEquals(
...@@ -111,7 +110,7 @@ public class LoadProgressMediatorTest { ...@@ -111,7 +110,7 @@ public class LoadProgressMediatorTest {
assertEquals(mModel.get(LoadProgressProperties.PROGRESS), assertEquals(mModel.get(LoadProgressProperties.PROGRESS),
LoadProgressMediator.MINIMUM_LOAD_PROGRESS, MathUtils.EPSILON); LoadProgressMediator.MINIMUM_LOAD_PROGRESS, MathUtils.EPSILON);
mActivityTabObserverCaptor.getValue().onResult(mTab2); mActivityTabObserverCaptor.getValue().onActivityTabChanged(mTab2, false);
verify(mTab2, times(1)).addObserver(any()); verify(mTab2, times(1)).addObserver(any());
assertEquals(mModel.get(LoadProgressProperties.COMPLETION_STATE), assertEquals(mModel.get(LoadProgressProperties.COMPLETION_STATE),
CompletionState.FINISHED_DONT_ANIMATE); CompletionState.FINISHED_DONT_ANIMATE);
...@@ -142,7 +141,7 @@ public class LoadProgressMediatorTest { ...@@ -142,7 +141,7 @@ public class LoadProgressMediatorTest {
LoadProgressMediator.MINIMUM_LOAD_PROGRESS, MathUtils.EPSILON); LoadProgressMediator.MINIMUM_LOAD_PROGRESS, MathUtils.EPSILON);
when(mTab2.getUrlString()).thenReturn(NATIVE_PAGE_URL); when(mTab2.getUrlString()).thenReturn(NATIVE_PAGE_URL);
mActivityTabObserverCaptor.getValue().onResult(mTab2); mActivityTabObserverCaptor.getValue().onActivityTabChanged(mTab2, false);
verify(mTab2, times(1)).addObserver(any()); verify(mTab2, times(1)).addObserver(any());
assertEquals(mModel.get(LoadProgressProperties.COMPLETION_STATE), assertEquals(mModel.get(LoadProgressProperties.COMPLETION_STATE),
CompletionState.FINISHED_DONT_ANIMATE); CompletionState.FINISHED_DONT_ANIMATE);
......
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