Commit 0b926446 authored by Henrique Nakashima's avatar Henrique Nakashima Committed by Commit Bot

Modularize TabWindowManager to chrome/browser/tabmodel

Split TabWindowManagerImpl out of interface, then move the interface,
the impl and the factory to the tabmodel module.

Narrow down the interface to be a bit smaller and more generic.

Bug: 1112922
Change-Id: I1c76e4ad86fae7435e31211ae857a8d772366b80
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461950Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817080}
parent 321a5026
......@@ -1048,6 +1048,7 @@ android_library("chrome_test_java") {
"//chrome/browser/share/android:java_resources",
"//chrome/browser/tab:java",
"//chrome/browser/tabmodel:java",
"//chrome/browser/tabmodel/internal:java",
"//chrome/browser/tabpersistence:java",
"//chrome/browser/thumbnail:java",
"//chrome/browser/thumbnail:javatests",
......
......@@ -1536,7 +1536,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/tabmodel/TabPersistencePolicy.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabPersistentStore.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabPersister.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabWindowManager.java",
"java/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicy.java",
"java/src/org/chromium/chrome/browser/tabmodel/document/TabDelegate.java",
"java/src/org/chromium/chrome/browser/tasks/ConditionalTabStripUtils.java",
......
......@@ -4,14 +4,19 @@
package org.chromium.chrome.browser.app.tabmodel;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorFactory;
import org.chromium.chrome.browser.tabmodel.TabWindowManager;
import org.chromium.chrome.browser.tabmodel.TabWindowManagerFactory;
/**
* Glue-level singleton instance of {@link TabWindowManager}.
*/
public class TabWindowManagerSingleton {
private static TabWindowManager sInstance;
private static TabModelSelectorFactory sSelectorFactoryForTesting;
/**
* @return The singleton instance of {@link TabWindowManager}.
......@@ -19,9 +24,23 @@ public class TabWindowManagerSingleton {
public static TabWindowManager getInstance() {
ThreadUtils.assertOnUiThread();
if (sInstance == null) {
sInstance = new TabWindowManager(new DefaultTabModelSelectorFactory(),
AsyncTabParamsManagerSingleton.getInstance());
TabModelSelectorFactory selectorFactory = sSelectorFactoryForTesting == null
? new DefaultTabModelSelectorFactory()
: sSelectorFactoryForTesting;
sInstance = TabWindowManagerFactory.createInstance(
selectorFactory, AsyncTabParamsManagerSingleton.getInstance());
}
return sInstance;
}
/**
* Allows overriding the default {@link TabModelSelectorFactory} with another one. Typically
* for testing.
* @param factory A {@link TabModelSelectorFactory} instance.
*/
@VisibleForTesting
public static void setTabModelSelectorFactoryForTesting(TabModelSelectorFactory factory) {
assert sInstance == null;
sSelectorFactoryForTesting = factory;
}
}
\ No newline at end of file
......@@ -398,7 +398,7 @@ public class TabbedModeTabPersistencePolicy implements TabPersistencePolicy {
}
private boolean shouldDeleteTabFile(int tabId, TabWindowManager tabWindowManager) {
return !tabWindowManager.tabExistsInAnySelector(tabId) && !mOtherTabIds.get(tabId);
return tabWindowManager.getTabById(tabId) == null && !mOtherTabIds.get(tabId);
}
/**
......
......@@ -80,18 +80,16 @@ public class ContextMenuLoadUrlParamsTest {
// Plant RecordingTabModelSelector as the TabModelSelector used in Main. The factory has to
// be set before super.setUp(), as super.setUp() creates Main and consequently the
// TabModelSelector.
TestThreadUtils.runOnUiThreadBlocking(() -> {
TabWindowManagerSingleton.getInstance().setTabModelSelectorFactory(
new TabModelSelectorFactory() {
@Override
public TabModelSelector buildSelector(Activity activity,
TabCreatorManager tabCreatorManager,
NextTabPolicySupplier nextTabPolicySupplier, int selectorIndex) {
return new RecordingTabModelSelector(activity, tabCreatorManager,
new ChromeTabModelFilterFactory(), selectorIndex);
}
});
});
TabWindowManagerSingleton.setTabModelSelectorFactoryForTesting(
new TabModelSelectorFactory() {
@Override
public TabModelSelector buildSelector(Activity activity,
TabCreatorManager tabCreatorManager,
NextTabPolicySupplier nextTabPolicySupplier, int selectorIndex) {
return new RecordingTabModelSelector(activity, tabCreatorManager,
new ChromeTabModelFilterFactory(), selectorIndex);
}
});
mActivityTestRule.startMainActivityOnBlankPage();
TestThreadUtils.runOnUiThreadBlocking(
() -> { FirstRunStatus.setFirstRunFlowComplete(true); });
......
......@@ -219,10 +219,15 @@ public class TabPersistentStoreTest {
private TestTabModelDirectory mMockDirectory;
private AdvancedMockContext mAppContext;
private SharedPreferencesManager mPreferences;
private TabWindowManagerImpl mTabWindowManager;
@Before
public void setUp() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
TabWindowManagerSingleton.setTabModelSelectorFactoryForTesting(
mMockTabModelSelectorFactory);
mTabWindowManager = (TabWindowManagerImpl) TabWindowManagerSingleton.getInstance();
mChromeActivity = new ChromeActivity() {
@Override
protected boolean handleBackPressed() {
......@@ -266,8 +271,7 @@ public class TabPersistentStoreTest {
@After
public void tearDown() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
TabWindowManagerSingleton.getInstance().onActivityStateChange(
mChromeActivity, ActivityState.DESTROYED);
mTabWindowManager.onActivityStateChange(mChromeActivity, ActivityState.DESTROYED);
});
mMockDirectory.tearDown();
}
......@@ -697,13 +701,11 @@ public class TabPersistentStoreTest {
TestThreadUtils.runOnUiThreadBlocking(new Callable<TestTabModelSelector>() {
@Override
public TestTabModelSelector call() {
TabWindowManager tabWindowManager = TabWindowManagerSingleton.getInstance();
tabWindowManager.setTabModelSelectorFactory(mMockTabModelSelectorFactory);
// Clear any existing TestTabModelSelector (required when
// createAndRestoreRealTabModelImpls is called multiple times in one test).
tabWindowManager.onActivityStateChange(
mTabWindowManager.onActivityStateChange(
mChromeActivity, ActivityState.DESTROYED);
return (TestTabModelSelector) tabWindowManager.requestSelector(
return (TestTabModelSelector) mTabWindowManager.requestSelector(
mChromeActivity, mChromeActivity, null, 0);
}
});
......
......@@ -10,6 +10,7 @@ import androidx.test.filters.SmallTest;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
......@@ -32,8 +33,10 @@ import java.util.ArrayList;
import java.util.List;
/**
* Test for {@link TabWindowManager} APIs. Makes sure the class handles multiple {@link Activity}s
* requesting {@link TabModelSelector}s, {@link Activity}s getting destroyed, etc..
* Test for {@link TabWindowManagerImpl} through {@link TabWindowManagerSingleton}.
*
* Makes sure the class handles multiple {@link Activity}s requesting {@link TabModelSelector}s,
* {@link Activity}s getting destroyed, etc.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
public class TabWindowManagerTest {
......@@ -48,6 +51,12 @@ public class TabWindowManagerTest {
}
};
@Before
public void setUp() {
TabWindowManagerSingleton.setTabModelSelectorFactoryForTesting(
mMockTabModelSelectorFactory);
}
private ChromeActivity buildActivity() {
ChromeActivity activity = new CustomTabActivity();
mActivities.add(activity);
......@@ -62,7 +71,6 @@ public class TabWindowManagerTest {
private MockTabModelSelector requestSelector(ChromeActivity activity, int requestedIndex) {
final TabWindowManager manager = TabWindowManagerSingleton.getInstance();
manager.setTabModelSelectorFactory(mMockTabModelSelectorFactory);
return (MockTabModelSelector) manager.requestSelector(
activity, activity, () -> NextTabPolicy.HIERARCHICAL, requestedIndex);
}
......@@ -186,7 +194,8 @@ public class TabWindowManagerTest {
}
/**
* Test that a destroyed {@link Activity} properly gets removed from {@link TabWindowManager}.
* Test that a destroyed {@link Activity} properly gets removed from {@link
* TabWindowManagerImpl}.
*/
@Test
@SmallTest
......@@ -290,20 +299,20 @@ public class TabWindowManagerTest {
Tab tab1 = selector0.addMockTab();
Tab tab2 = selector1.addMockIncognitoTab();
Assert.assertFalse(manager.tabExistsInAnySelector(tab1.getId() - 1));
Assert.assertTrue(manager.tabExistsInAnySelector(tab1.getId()));
Assert.assertTrue(manager.tabExistsInAnySelector(tab2.getId()));
Assert.assertFalse(manager.tabExistsInAnySelector(tab2.getId() + 1));
Assert.assertNull(manager.getTabById(tab1.getId() - 1));
Assert.assertNotNull(manager.getTabById(tab1.getId()));
Assert.assertNotNull(manager.getTabById(tab2.getId()));
Assert.assertNull(manager.getTabById(tab2.getId() + 1));
AsyncTabParamsManager asyncTabParamsManager = AsyncTabParamsManagerSingleton.getInstance();
asyncTabParamsManager.getAsyncTabParams().clear();
final int asyncTabId = 123;
final TabReparentingParams dummyParams =
new TabReparentingParams(new MockTab(0, false), null);
Assert.assertFalse(manager.tabExistsInAnySelector(asyncTabId));
Assert.assertNull(manager.getTabById(asyncTabId));
asyncTabParamsManager.add(asyncTabId, dummyParams);
try {
Assert.assertTrue(manager.tabExistsInAnySelector(asyncTabId));
Assert.assertNotNull(manager.getTabById(asyncTabId));
} finally {
asyncTabParamsManager.getAsyncTabParams().clear();
}
......
......@@ -35,6 +35,7 @@ android_library("java") {
"android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorTabObserver.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/TabModelUtils.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/TabReparentingParams.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/TabWindowManager.java",
]
deps = [
"//base:base_java",
......@@ -49,5 +50,8 @@ android_library("java") {
android_library_factory("factory_java") {
deps = [ ":java" ]
sources = [ "internal/android/java/src/org/chromium/chrome/browser/tabmodel/AsyncTabParamsManagerFactory.java" ]
sources = [
"internal/android/java/src/org/chromium/chrome/browser/tabmodel/AsyncTabParamsManagerFactory.java",
"internal/android/java/src/org/chromium/chrome/browser/tabmodel/TabWindowManagerFactory.java",
]
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.tabmodel;
import android.app.Activity;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
import org.chromium.ui.base.WindowAndroid;
/**
* Manages multiple {@link TabModelSelector} instances, each owned by different {@link Activity}s.
*
* Each of the 0-3 {@link WindowAndroid} contains 1 {@link Activity},
* which contains 1 {@link TabModelSelector}, which contains 2 {@link TabModel}s,
* each of which contains n {@link Tab}s.
*
* Also manages tabs being reparented in AsyncTabParamsManager.
*
* This is the highest level of the hierarchy of Tab containers.
*/
public interface TabWindowManager {
/** The maximum number of simultaneous TabModelSelector instances in this Application. */
int MAX_SIMULTANEOUS_SELECTORS = 3;
/**
* Called to request a {@link TabModelSelector} based on {@code index}. Note that the
* {@link TabModelSelector} returned might not actually be the one related to {@code index} and
* {@link #getIndexForWindow(Activity)} should be called to grab the actual index if required.
*
* @param tabCreatorManager An instance of {@link TabCreatorManager}.
* @param nextTabPolicySupplier An instance of {@link NextTabPolicySupplier}.
* @param index The index of the requested {@link TabModelSelector}. Not guaranteed to be the
* index of the {@link TabModelSelector} returned.
* @return A {@link TabModelSelector} index, or {@code null} if there are too many
* {@link TabModelSelector}s already built.
*/
TabModelSelector requestSelector(Activity activity, TabCreatorManager tabCreatorManager,
NextTabPolicySupplier nextTabPolicySupplier, int index);
/**
* An index that represents the invalid state (i.e. when the window wasn't found in the list).
*/
int INVALID_WINDOW_INDEX = -1;
/**
* Finds the current index of the {@link TabModelSelector} bound to {@code window}.
* @param activity The {@link Activity} to find the index of the {@link TabModelSelector}
* for. This uses the underlying {@link Activity} stored in the
* {@link WindowAndroid}.
* @return The index of the {@link TabModelSelector} or {@link #INVALID_WINDOW_INDEX} if
* not found.
*/
int getIndexForWindow(Activity activity);
/**
* @return The number of {@link TabModelSelector}s currently assigned to {@link Activity}s.
*/
int getNumberOfAssignedTabModelSelectors();
/**
* @return The total number of incognito tabs across all tab model selectors.
*/
int getIncognitoTabCount();
/**
* @param tab The tab to look for in each model.
* @return The TabModel containing the given Tab or null if one doesn't exist.
**/
TabModel getTabModelForTab(Tab tab);
/**
* @param tabId The ID of the tab in question.
* @return Specified {@link Tab} or {@code null} if the {@link Tab} is not found.
*/
Tab getTabById(int tabId);
}
......@@ -8,10 +8,13 @@ android_library("java") {
visibility = [
"//chrome/android:chrome_all_java",
"//chrome/android:chrome_junit_tests__java_binary",
"//chrome/android:chrome_test_java",
]
sources = [
"android/java/src/org/chromium/chrome/browser/tabmodel/AsyncTabParamsManagerFactory.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/AsyncTabParamsManagerImpl.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/TabWindowManagerFactory.java",
"android/java/src/org/chromium/chrome/browser/tabmodel/TabWindowManagerImpl.java",
]
deps = [
"//base:base_java",
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.tabmodel;
/**
* Factory for creating {@link TabWindowManager}.
*/
public class TabWindowManagerFactory {
/**
* @return New instance of {@link TabWindowManagerImpl}.
*/
public static TabWindowManager createInstance(
TabModelSelectorFactory selectorFactory, AsyncTabParamsManager asyncTabParamsManager) {
return new TabWindowManagerImpl(selectorFactory, asyncTabParamsManager);
}
}
......@@ -7,14 +7,11 @@ package org.chromium.chrome.browser.tabmodel;
import android.app.Activity;
import android.util.SparseArray;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ApplicationStatus.ActivityStateListener;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
import org.chromium.ui.base.WindowAndroid;
import java.util.ArrayList;
import java.util.HashMap;
......@@ -23,16 +20,10 @@ import java.util.Map;
/**
* Manages multiple {@link TabModelSelector} instances, each owned by different {@link Activity}s.
*
* Also manages tabs being reparented in AsyncTabParamsManager.
*/
public class TabWindowManager implements ActivityStateListener {
/**
* An index that represents the invalid state (i.e. when the window wasn't found in the list).
*/
public static final int INVALID_WINDOW_INDEX = -1;
/** The maximum number of simultaneous TabModelSelector instances in this Application. */
public static final int MAX_SIMULTANEOUS_SELECTORS = 3;
public class TabWindowManagerImpl implements ActivityStateListener, TabWindowManager {
private TabModelSelectorFactory mSelectorFactory;
private final AsyncTabParamsManager mAsyncTabParamsManager;
......@@ -40,18 +31,18 @@ public class TabWindowManager implements ActivityStateListener {
private Map<Activity, TabModelSelector> mAssignments = new HashMap<>();
/**
* Called to request a {@link TabModelSelector} based on {@code index}. Note that the
* {@link TabModelSelector} returned might not actually be the one related to {@code index} and
* {@link #getIndexForWindow(Activity)} should be called to grab the actual index if required.
*
* @param tabCreatorManager An instance of {@link TabCreatorManager}.
* @param nextTabPolicySupplier An instance of {@link NextTabPolicySupplier}.
* @param index The index of the requested {@link TabModelSelector}. Not guaranteed to be the
* index of the {@link TabModelSelector} returned.
* @return A {@link TabModelSelector} index, or {@code null} if there are too many
* {@link TabModelSelector}s already built.
*/
TabWindowManagerImpl(
TabModelSelectorFactory selectorFactory, AsyncTabParamsManager asyncTabParamsManager) {
mSelectorFactory = selectorFactory;
mAsyncTabParamsManager = asyncTabParamsManager;
ApplicationStatus.registerStateListenerForAllActivities(this);
for (int i = 0; i < TabWindowManager.MAX_SIMULTANEOUS_SELECTORS; i++) {
mSelectors.add(null);
}
}
@Override
public TabModelSelector requestSelector(Activity activity, TabCreatorManager tabCreatorManager,
NextTabPolicySupplier nextTabPolicySupplier, int index) {
if (mAssignments.get(activity) != null) {
......@@ -80,32 +71,21 @@ public class TabWindowManager implements ActivityStateListener {
return selector;
}
/**
* Finds the current index of the {@link TabModelSelector} bound to {@code window}.
* @param activity The {@link Activity} to find the index of the {@link TabModelSelector}
* for. This uses the underlying {@link Activity} stored in the
* {@link WindowAndroid}.
* @return The index of the {@link TabModelSelector} or {@link #INVALID_WINDOW_INDEX} if
* not found.
*/
@Override
public int getIndexForWindow(Activity activity) {
if (activity == null) return INVALID_WINDOW_INDEX;
if (activity == null) return TabWindowManager.INVALID_WINDOW_INDEX;
TabModelSelector selector = mAssignments.get(activity);
if (selector == null) return INVALID_WINDOW_INDEX;
if (selector == null) return TabWindowManager.INVALID_WINDOW_INDEX;
int index = mSelectors.indexOf(selector);
return index == -1 ? INVALID_WINDOW_INDEX : index;
return index == -1 ? TabWindowManager.INVALID_WINDOW_INDEX : index;
}
/**
* @return The number of {@link TabModelSelector}s currently assigned to {@link Activity}s.
*/
@Override
public int getNumberOfAssignedTabModelSelectors() {
return mAssignments.size();
}
/**
* @return The total number of incognito tabs across all tab model selectors.
*/
@Override
public int getIncognitoTabCount() {
int count = 0;
for (int i = 0; i < mSelectors.size(); i++) {
......@@ -124,18 +104,7 @@ public class TabWindowManager implements ActivityStateListener {
return count;
}
/**
* @param tabId The ID of the tab in question.
* @return Whether the given tab exists in any currently loaded selector.
*/
public boolean tabExistsInAnySelector(int tabId) {
return getTabById(tabId) != null;
}
/**
* @param tab The tab to look for in each model.
* @return The TabModel containing the given Tab or null if one doesn't exist.
**/
@Override
public TabModel getTabModelForTab(Tab tab) {
if (tab == null) return null;
......@@ -150,10 +119,7 @@ public class TabWindowManager implements ActivityStateListener {
return null;
}
/**
* @param tabId The ID of the tab in question.
* @return Specified {@link Tab} or {@code null} if the {@link Tab} is not found.
*/
@Override
public Tab getTabById(int tabId) {
for (int i = 0; i < mSelectors.size(); i++) {
TabModelSelector selector = mSelectors.get(i);
......@@ -170,31 +136,15 @@ public class TabWindowManager implements ActivityStateListener {
return null;
}
// ActivityStateListener
@Override
public void onActivityStateChange(Activity activity, int newState) {
if (newState == ActivityState.DESTROYED && mAssignments.containsKey(activity)) {
int index = mSelectors.indexOf(mAssignments.remove(activity));
if (index >= 0) mSelectors.set(index, null);
if (index >= 0) {
mSelectors.set(index, null);
}
// TODO(dtrainor): Move TabModelSelector#destroy() calls here.
}
}
/**
* Allows overriding the default {@link TabModelSelectorFactory} with another one. Typically
* for testing.
* @param factory A {@link TabModelSelectorFactory} instance.
*/
@VisibleForTesting
public void setTabModelSelectorFactory(TabModelSelectorFactory factory) {
mSelectorFactory = factory;
}
public TabWindowManager(
TabModelSelectorFactory selectorFactory, AsyncTabParamsManager asyncTabParamsManager) {
mSelectorFactory = selectorFactory;
mAsyncTabParamsManager = asyncTabParamsManager;
ApplicationStatus.registerStateListenerForAllActivities(this);
for (int i = 0; i < MAX_SIMULTANEOUS_SELECTORS; i++) mSelectors.add(null);
}
}
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