Commit 97422b2b authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Avoid auto switches to standard mode after closing incognito tab

After crrev.com/c/2314557, |mOverviewModeController| can be null depends
on the stage of Chrome initialization. ChromeNextTabPolicySupplier is
created before the OverviewModeController is instantiated, thus the
OverviewModeController in that class is always null. and it always
returns the HIERARCHICAL policy and messes up the next tab logic in
TabModelImpl.

This CL fixes the issue by using the ObservableSupplier to supply the
OverviewModeController, so by the time Chrome finishes its
initialization the ChromeNextTabPolicySupplier would have a valid
OverviewModeController.

Bug: 1124227
Change-Id: I5123487ba13cf6797e281df1ab834322dd2ef39d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2404458Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Mei Liang <meiliang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806404}
parent 4874496b
......@@ -52,6 +52,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/app/appmenu/DataSaverAppMenuTest.java",
"javatests/src/org/chromium/chrome/browser/app/appmenu/OverviewAppMenuTest.java",
"javatests/src/org/chromium/chrome/browser/app/appmenu/TabbedAppMenuTest.java",
"javatests/src/org/chromium/chrome/browser/app/tabmodel/ChromeNextTabPolicySupplierTest.java",
"javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupTest.java",
"javatests/src/org/chromium/chrome/browser/autofill/AutofillPopupWithKeyboardTest.java",
"javatests/src/org/chromium/chrome/browser/autofill/AutofillTest.java",
......
......@@ -283,6 +283,8 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
*/
private boolean mOverviewShownOnStart;
private NextTabPolicySupplier mNextTabPolicySupplier;
private final ObservableSupplierImpl<OverviewModeBehavior> mOverviewModeBehaviorSupplier =
new ObservableSupplierImpl<>();
private OverviewModeController mOverviewModeController;
......@@ -1535,11 +1537,10 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
&& savedInstanceState.getBoolean("is_incognito_selected", false);
int index = savedInstanceState != null ? savedInstanceState.getInt(WINDOW_INDEX, 0) : 0;
NextTabPolicySupplier nextTabPolicySupplier =
new ChromeNextTabPolicySupplier(mOverviewModeController);
mNextTabPolicySupplier = new ChromeNextTabPolicySupplier(mOverviewModeBehaviorSupplier);
mTabModelSelectorImpl =
(TabModelSelectorImpl) TabWindowManager.getInstance().requestSelector(
this, this, nextTabPolicySupplier, index);
this, this, mNextTabPolicySupplier, index);
if (mTabModelSelectorImpl == null) {
Toast.makeText(
this, getString(R.string.unsupported_number_of_windows), Toast.LENGTH_LONG)
......@@ -2258,4 +2259,9 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
public MultiInstanceManager getMultiInstanceMangerForTesting() {
return mMultiInstanceManager;
}
@VisibleForTesting
public ChromeNextTabPolicySupplier getNextTabPolicySupplier() {
return (ChromeNextTabPolicySupplier) mNextTabPolicySupplier;
}
}
......@@ -4,7 +4,12 @@
package org.chromium.chrome.browser.app.tabmodel;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeController;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.OneShotCallback;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
import org.chromium.chrome.browser.tabmodel.NextTabPolicy;
import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
......@@ -12,18 +17,30 @@ import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
* Decides to show a next tab by location if overview is open, or by hierarchy otherwise.
*/
public class ChromeNextTabPolicySupplier implements NextTabPolicySupplier {
private final OverviewModeController mOverviewModeController;
private OverviewModeBehavior mOverviewModeBehavior;
public ChromeNextTabPolicySupplier(
ObservableSupplier<OverviewModeBehavior> overviewModeControllerObservableSupplier) {
// TODO(crbug.com/1084528): Replace this with OneShotSupplier when it is available.
new OneShotCallback<>(
overviewModeControllerObservableSupplier, this::setOverviewModeBehavior);
}
public ChromeNextTabPolicySupplier(OverviewModeController overviewModeController) {
mOverviewModeController = overviewModeController;
private void setOverviewModeBehavior(@NonNull OverviewModeBehavior overviewModeBehavior) {
mOverviewModeBehavior = overviewModeBehavior;
}
@Override
public @NextTabPolicy Integer get() {
if (mOverviewModeController != null && mOverviewModeController.overviewVisible()) {
if (mOverviewModeBehavior != null && mOverviewModeBehavior.overviewVisible()) {
return NextTabPolicy.LOCATIONAL;
} else {
return NextTabPolicy.HIERARCHICAL;
}
}
@VisibleForTesting
OverviewModeBehavior getOverviewModeBehavior() {
return mOverviewModeBehavior;
}
}
// 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.app.tabmodel;
import androidx.test.filters.SmallTest;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
/**
* Tests for the {@link ChromeNextTabPolicySupplier}.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class ChromeNextTabPolicySupplierTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
@Before
public void setUp() {
mActivityTestRule.startMainActivityFromLauncher();
}
@Test
@SmallTest
public void verifyOverviewModeBehaviorIsNotNull() {
Assert.assertNotNull(mActivityTestRule.getActivity()
.getNextTabPolicySupplier()
.getOverviewModeBehavior());
}
}
\ No newline at end of file
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