Skip to content

GitLab

T
tangled
Commit 0ecee86d authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot
Browse files
Options

Revert "Reparent the current Tab when the theme is changed" 

This reverts commit 24eca4b9.

Reason for revert: Crashing Tot

Original change's description:
> Reparent the current Tab when the theme is changed
>
> * Creating a class which handles reparenting the current tab when the
>   theme is swapped.
> * Hooking that class into ChromeActivity.
> * Adding plumbing to a few ReparentingTask/TabReparentingParams to
>   facilitate the process.
>
> Bug: 944302
> Change-Id: I2801eba979b3acafa02856a9053bc474bf2f581c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1931763
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Commit-Queue: Brandon Wylie <wylieb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#726136}

TBR=fgorski@chromium.org,yusufo@chromium.org,wylieb@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 944302
Change-Id: If032ff5e4cf61f4047e4cf146c8ae6a03a78aa77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1979398Reviewed-by: default avatarBrandon Wylie <wylieb@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726996}
parent 2e8a62b7
Hide whitespace changes
Inline Side-by-side
Showing with 16 additions and 258 deletions
chrome/android/chrome_java_sources.gni
View file @ 0ecee86d
......@@ -938,7 +938,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/night_mode/GlobalNightModeStateController.java",
"java/src/org/chromium/chrome/browser/night_mode/GlobalNightModeStateProviderHolder.java",
"java/src/org/chromium/chrome/browser/night_mode/NightModeMetrics.java",
"java/src/org/chromium/chrome/browser/night_mode/NightModeReparentingController.java",
"java/src/org/chromium/chrome/browser/night_mode/NightModeStateProvider.java",
"java/src/org/chromium/chrome/browser/night_mode/NightModeUtils.java",
"java/src/org/chromium/chrome/browser/night_mode/PowerSavingModeMonitor.java",
......@@ -1621,7 +1620,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/tab/TabWebContentsUserData.java",
"java/src/org/chromium/chrome/browser/tab/TrustedCdn.java",
"java/src/org/chromium/chrome/browser/tab_activity_glue/ActivityTabWebContentsDelegateAndroid.java",
"java/src/org/chromium/chrome/browser/tab_activity_glue/ReparentingDelegateFactory.java",
"java/src/org/chromium/chrome/browser/tab_activity_glue/ReparentingTask.java",
"java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java",
"java/src/org/chromium/chrome/browser/tabbed_mode/TabbedNavigationBarColorController.java",
......
chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
View file @ 0ecee86d
......@@ -87,7 +87,6 @@ import org.chromium.chrome.browser.firstrun.ForcedSigninProcessor;
import org.chromium.chrome.browser.flags.ActivityType;
import org.chromium.chrome.browser.flags.FeatureUtilities;
import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
import org.chromium.chrome.browser.fullscreen.ComposedBrowserControlsVisibilityDelegate;
import org.chromium.chrome.browser.gsa.ContextReporter;
import org.chromium.chrome.browser.gsa.GSAAccountChangeListener;
import org.chromium.chrome.browser.gsa.GSAState;
......@@ -106,7 +105,6 @@ import org.chromium.chrome.browser.metrics.LaunchMetrics;
import org.chromium.chrome.browser.metrics.UmaSessionStats;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.nfc.BeamController;
import org.chromium.chrome.browser.night_mode.NightModeReparentingController;
import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.ntp.NewTabPageUma;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
......@@ -131,13 +129,11 @@ import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarManageable;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.sync.SyncController;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabDelegateFactory;
import org.chromium.chrome.browser.tab.TabHidingType;
import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.tab.TabState;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingDelegateFactory;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
import org.chromium.chrome.browser.tabmodel.EmptyTabModel;
import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
......@@ -324,9 +320,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
private List<MenuOrKeyboardActionController.MenuOrKeyboardActionHandler> mMenuActionHandlers =
new ArrayList<>();
/** Controls tab reparenting for night mode. */
NightModeReparentingController mNightModeReparentingController;
@Override
protected ActivityWindowAndroid createWindowAndroid() {
return new ChromeWindow(this);
......@@ -363,11 +356,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
}
getWindow().setBackgroundDrawable(getBackgroundDrawable());
mNightModeReparentingController = new NightModeReparentingController(
ReparentingDelegateFactory.createNightModeReparentingControllerDelegate(this),
ReparentingDelegateFactory.createReparentingTaskDelegate(this));
getLifecycleDispatcher().register(mNightModeReparentingController);
}
protected RootUiCoordinator createRootUiCoordinator() {
......@@ -1557,11 +1545,6 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
return mActivityTabProvider;
}
public TabDelegateFactory getTabDelegateFactory() {
return new TabbedModeTabDelegateFactory(
this, new ComposedBrowserControlsVisibilityDelegate(), getShareDelegateSupplier());
}
/**
* Returns the {@link InsetObserverView} that has the current system window
* insets information.
......@@ -2421,16 +2404,4 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
public RootUiCoordinator getRootUiCoordinatorForTesting() {
return mRootUiCoordinator;
}
// NightModeStateProvider.Observer implementation.
@Override
public void onNightModeStateChanged() {
// Note: order matters here because the call to super will recreate the activity.
// Note: it's possible for this method to be called before mNightModeReparentingController
// is constructed.
if (mNightModeReparentingController != null) {
mNightModeReparentingController.onNightModeStateChanged();
}
super.onNightModeStateChanged();
}
}
chrome/android/java/src/org/chromium/chrome/browser/customtabs/HiddenTabHolder.java
View file @ 0ecee86d
......@@ -127,7 +127,7 @@ public class HiddenTabHolder {
int width = bounds.right - bounds.left;
int height = bounds.bottom - bounds.top;
tab.getWebContents().setSize(width, height);
ReparentingTask.from(tab).detach();
ReparentingTask.detach(tab);
return tab;
}
......
chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabController.java
View file @ 0ecee86d
......@@ -46,7 +46,6 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabAssociatedApp;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab.TabRedirectHandler;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingDelegateFactory;
import org.chromium.chrome.browser.tabmodel.AsyncTabParams;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
import org.chromium.chrome.browser.tabmodel.TabModel;
......@@ -317,8 +316,7 @@ public class CustomTabActivityTabController
if (mode == TabCreationMode.HIDDEN) {
TabReparentingParams params =
(TabReparentingParams) AsyncTabParamsManager.remove(tab.getId());
mReparentingTaskProvider.get(tab).finish(
ReparentingDelegateFactory.createReparentingTaskDelegate(mActivity),
mReparentingTaskProvider.get(tab).finish(mActivity, mCustomTabDelegateFactory.get(),
(params == null ? null : params.getFinalizeCallback()));
}
......
chrome/android/java/src/org/chromium/chrome/browser/night_mode/NightModeReparentingController.java deleted 100644 → 0
View file @ 2e8a62b7
// Copyright 2019 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.night_mode;
import android.util.SparseArray;
import androidx.annotation.NonNull;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingTask;
import org.chromium.chrome.browser.tabmodel.AsyncTabParams;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
import org.chromium.chrome.browser.tabmodel.TabModel;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabReparentingParams;
// TODO(wylieb): Write unittests for this class.
/** Controls the reparenting of tabs when the theme is swapped. */
public class NightModeReparentingController
implements StartStopWithNativeObserver, NightModeStateProvider.Observer {
/** Provides data to {@link NightModeReparentingController} facilitate reparenting tabs. */
public interface Delegate {
/** The current ActivityTabProvider which is used to get the current Tab. */
ActivityTabProvider getActivityTabProvider();
/** Gets a {@link TabModelSelector} which is used to add the tab. */
TabModelSelector getTabModelSelector();
}
private Delegate mDelegate;
private ReparentingTask.Delegate mReparentingDelegate;
/** Constructs a {@link NightModeReparentingController} with the given delegate. */
public NightModeReparentingController(
@NonNull Delegate delegate, @NonNull ReparentingTask.Delegate reparentingDelegate) {
mDelegate = delegate;
mReparentingDelegate = reparentingDelegate;
}
@Override
public void onStartWithNative() {
// Note: for now only the current tab is added to the AsyncTabParamsManager when the theme
// is changed. In the future these will be added in tab index order and read at reverse
// tab index order.
final SparseArray<AsyncTabParams> paramsArray = AsyncTabParamsManager.getAsyncTabParams();
for (int i = 0; i < paramsArray.size(); i++) {
final int tabId = paramsArray.keyAt(i);
AsyncTabParams params = paramsArray.get(tabId);
if (!(params instanceof TabReparentingParams)) continue;
if (params == null) continue;
final TabReparentingParams reparentingParams = (TabReparentingParams) params;
if (!reparentingParams.isFromNightModeReparenting()) continue;
if (!reparentingParams.hasTabToReparent()) continue;
if (!reparentingParams.hasTabIndex()) continue;
final ReparentingTask reparentingTask =
ReparentingTask.get(reparentingParams.getTabToReparent());
if (reparentingTask == null) continue;
reparentingTask.finish(mReparentingDelegate, () -> {
mDelegate.getTabModelSelector().getCurrentModel().addTab(
reparentingParams.getTabToReparent(), reparentingParams.getTabIndex(),
TabLaunchType.FROM_REPARENTING);
AsyncTabParamsManager.remove(tabId);
});
}
}
@Override
public void onStopWithNative() {}
@Override
public void onNightModeStateChanged() {
// TODO(crbug.com/1031332): Reparent all tabs in the current tab model.
Tab tabToDetach = mDelegate.getActivityTabProvider().get();
TabModel currentTabModel = mDelegate.getTabModelSelector().getCurrentModel();
TabReparentingParams params = new TabReparentingParams(tabToDetach, null, null);
params.setTabIndex(currentTabModel.indexOf(tabToDetach));
params.setFromNightModeReparenting(true);
AsyncTabParamsManager.add(tabToDetach.getId(), params);
currentTabModel.removeTab(tabToDetach);
ReparentingTask.from(tabToDetach).detach();
}
}
chrome/android/java/src/org/chromium/chrome/browser/tab_activity_glue/ReparentingDelegateFactory.java deleted 100644 → 0
View file @ 2e8a62b7
// Copyright 2019 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.tab_activity_glue;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.night_mode.NightModeReparentingController;
import org.chromium.chrome.browser.tab.TabDelegateFactory;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.ui.base.WindowAndroid;
/** Constructs delegates needed for reparenting tabs. */
public class ReparentingDelegateFactory {
/**
* @return Creates an implementation of {@link ReparentingTask.Delegate} that supplies
* dependencies for {@link ReparentingTask} to reparent a Tab.
*/
public static ReparentingTask.Delegate createReparentingTaskDelegate(
final ChromeActivity chromeActivity) {
return new ReparentingTask.Delegate() {
@Override
public CompositorViewHolder getCompositorViewHolder() {
return chromeActivity.getCompositorViewHolder();
}
@Override
public WindowAndroid getWindowAndroid() {
return chromeActivity.getWindowAndroid();
}
@Override
public TabDelegateFactory getTabDelegateFactory() {
return chromeActivity.getTabDelegateFactory();
}
};
}
/**
* @return Creates an implementation of {@link NightModeReparentingController.Delegate} that
* supplies dependencies to {@link NightModeReparentingController}.
*/
public static NightModeReparentingController.Delegate
createNightModeReparentingControllerDelegate(final ChromeActivity chromeActivity) {
return new NightModeReparentingController.Delegate() {
@Override
public ActivityTabProvider getActivityTabProvider() {
return chromeActivity.getActivityTabProvider();
}
@Override
public TabModelSelector getTabModelSelector() {
return chromeActivity.getTabModelSelector();
}
};
}
}
chrome/android/java/src/org/chromium/chrome/browser/tab_activity_glue/ReparentingTask.java
View file @ 0ecee86d
......@@ -12,21 +12,19 @@ import android.os.Bundle;
import android.provider.Browser;
import android.text.TextUtils;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.chromium.base.ContextUtils;
import org.chromium.base.UserData;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabDelegateFactory;
import org.chromium.chrome.browser.tab.TabImpl;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabReparentingParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;
......@@ -35,27 +33,6 @@ import org.chromium.ui.base.WindowAndroid;
* Takes care of reparenting a Tab object from one Activity to another.
*/
public class ReparentingTask implements UserData {
/** Provides data to {@link ReparentingTask} facilitate reparenting tabs. */
public interface Delegate {
/**
* Gets a {@link CompositorViewHolder} which is passed on to {@link ReparentingTask}, used
* in the reparenting process.
*/
CompositorViewHolder getCompositorViewHolder();
/**
* Gets a {@link WindowAndroid} which is passed on to {@link ReparentingTask}, used in the
* reparenting process.
*/
WindowAndroid getWindowAndroid();
/**
* Gets a {@link TabDelegateFactory} which is passed on to {@link ReparentingTask}, used in
* the reparenting process.
*/
TabDelegateFactory getTabDelegateFactory();
}
private static final Class<ReparentingTask> USER_DATA_KEY = ReparentingTask.class;
private final Tab mTab;
......@@ -75,7 +52,7 @@ public class ReparentingTask implements UserData {
}
@Nullable
public static ReparentingTask get(Tab tab) {
private static ReparentingTask get(Tab tab) {
return tab.getUserDataHost().getUserData(USER_DATA_KEY);
}
......@@ -119,7 +96,7 @@ public class ReparentingTask implements UserData {
AsyncTabParamsManager.add(
mTab.getId(), new TabReparentingParams(mTab, intent, finalizeCallback));
detach();
detach(mTab);
}
context.startActivity(intent, startActivityOptions);
......@@ -133,16 +110,16 @@ public class ReparentingTask implements UserData {
* - Removes the tab from its current {@link TabModelSelector}, effectively severing
* the {@link Activity} to {@link Tab} link.
*/
public void detach() {
public static void detach(Tab tab) {
// TODO(yusufo): We can't call tab.updateWindowAndroid that sets |mWindowAndroid| to null
// because many code paths (including navigation) expect the tab to always be associated
// with an activity, and will crash. crbug.com/657007
WebContents webContents = mTab.getWebContents();
WebContents webContents = tab.getWebContents();
if (webContents != null) webContents.setTopLevelNativeWindow(null);
// TabModelSelector of this Tab, if present, gets notified to remove the tab from
// the TabModel it belonged to.
mTab.updateAttachment(null, null);
tab.updateAttachment(null, null);
}
/**
......@@ -150,12 +127,14 @@ public class ReparentingTask implements UserData {
* the tab and related objects to reference it. This updates many delegates inside the tab
* and {@link WebContents} both on java and native sides.
*
* @param delegate A delegate that provides dependencies.
* @param activity A new {@link ChromeActivity} to attach this Tab instance to.
* @param tabDelegateFactory The new delegate factory this tab should be using.
* @param finalizeCallback A Callback to be called after the Tab has been reparented.
*/
public void finish(@NonNull Delegate delegate, @Nullable Runnable finalizeCallback) {
delegate.getCompositorViewHolder().prepareForTabReparenting();
attach(delegate.getWindowAndroid(), delegate.getTabDelegateFactory());
public void finish(ChromeActivity activity, TabDelegateFactory tabDelegateFactory,
@Nullable Runnable finalizeCallback) {
activity.getCompositorViewHolder().prepareForTabReparenting();
attach(activity.getWindowAndroid(), tabDelegateFactory);
((TabImpl) mTab).setIsTabStateDirty(true);
if (finalizeCallback != null) finalizeCallback.run();
}
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/ChromeTabCreator.java
View file @ 0ecee86d
......@@ -25,7 +25,6 @@ import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab.TabParentIntent;
import org.chromium.chrome.browser.tab.TabRedirectHandler;
import org.chromium.chrome.browser.tab.TabState;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingDelegateFactory;
import org.chromium.chrome.browser.tab_activity_glue.ReparentingTask;
import org.chromium.chrome.browser.util.IntentUtils;
import org.chromium.components.url_formatter.UrlFormatter;
......@@ -129,8 +128,7 @@ public class ChromeTabCreator extends TabCreatorManager.TabCreator {
TabReparentingParams params = (TabReparentingParams) asyncParams;
tab = params.getTabToReparent();
ReparentingTask.from(tab).finish(
ReparentingDelegateFactory.createReparentingTaskDelegate(mActivity),
params.getFinalizeCallback());
mActivity, createDefaultTabDelegateFactory(), params.getFinalizeCallback());
} else if (asyncParams != null && asyncParams.getWebContents() != null) {
openInForeground = true;
WebContents webContents = asyncParams.getWebContents();
......
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabReparentingParams.java
View file @ 0ecee86d
......@@ -17,15 +17,10 @@ import org.chromium.content_public.browser.WebContents;
* Class for handling tab reparenting operations across multiple activities.
*/
public class TabReparentingParams implements AsyncTabParams {
private static final int TAB_INDEX_NOT_SET = -1;
private final Tab mTabToReparent;
private final Intent mOriginalIntent;
private final Runnable mFinalizeCallback;
private int mTabIndex = TAB_INDEX_NOT_SET;
private boolean mIsFromNightModeReparenting;
/**
* Basic constructor for {@link TabReparentingParams}.
*/
......@@ -66,10 +61,6 @@ public class TabReparentingParams implements AsyncTabParams {
return mTabToReparent;
}
public boolean hasTabToReparent() {
return mTabToReparent != null;
}
/**
* Returns the callback to be used once Tab reparenting has finished, if any.
*/
......@@ -81,31 +72,4 @@ public class TabReparentingParams implements AsyncTabParams {
public void destroy() {
if (mTabToReparent != null) mTabToReparent.destroy();
}
// Night mode reparenting implementation.
/** Set the tab index for later retrieval. */
public void setTabIndex(int tabIndex) {
mTabIndex = tabIndex;
}
/** @return Index of the stored tab. */
public int getTabIndex() {
return mTabIndex;
}
/** @return Whether this holds a tab index. */
public boolean hasTabIndex() {
return mTabIndex != TAB_INDEX_NOT_SET;
}
/** Set whether these params are from night mode reparenting. */
public void setFromNightModeReparenting(boolean fromNightModeReparenting) {
mIsFromNightModeReparenting = fromNightModeReparenting;
}
/** @return Whether these params are from night mode reparenting. */
public boolean isFromNightModeReparenting() {
return mIsFromNightModeReparenting;
}
}
chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabControllerTest.java
View file @ 0ecee86d
......@@ -126,7 +126,7 @@ public class CustomTabActivityTabControllerTest {
public void finishesReparentingHiddenTab() {
Tab hiddenTab = env.prepareHiddenTab();
env.reachNativeInit(mTabController);
verify(env.reparentingTaskProvider.get(hiddenTab)).finish(any(), any());
verify(env.reparentingTaskProvider.get(hiddenTab)).finish(any(), any(), any());
}
@Test
......
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