Skip to content

Commit

Permalink
[Instant Start] Support pre-native inflation of StartSurface toolbar.
Browse files Browse the repository at this point in the history
In this CL, we fix two issues to make StartSurface toolbar shown
pre-native:
1) Remove the check of ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID
   from NewTabButton, since there is no plan to launch this feature any
   more (crbug.com/1079465). Thus, the NewTabButton can be inflated
   pre-native.
2) Lazily initialize the IncognitoSwitchCoordinator when native is
   initialized.

Bug: 1076904, 1079465
Change-Id: Ib73e4ee87ccca766612e4d8ab03539f7ec4c9a06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2180562
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#767132}
  • Loading branch information
Xi Han authored and Commit Bot committed May 10, 2020
1 parent c3e1531 commit 6b1b7ea
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 8 deletions.
Expand Up @@ -4,9 +4,15 @@

package org.chromium.chrome.features.start_surface;

import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withId;

import static com.google.common.truth.Truth.assertThat;

import static org.hamcrest.CoreMatchers.allOf;

import static org.chromium.chrome.browser.tabmodel.TestTabModelDirectory.M26_GOOGLE_COM;
import static org.chromium.chrome.test.util.ViewUtils.onViewWaiting;

import android.content.Intent;
import android.graphics.Bitmap;
Expand Down Expand Up @@ -50,6 +56,8 @@
import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementDelegate;
import org.chromium.chrome.browser.tasks.tab_management.TabManagementModuleProvider;
import org.chromium.chrome.browser.toolbar.top.StartSurfaceToolbarCoordinator;
import org.chromium.chrome.browser.toolbar.top.TopToolbarCoordinator;
import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
Expand Down Expand Up @@ -340,6 +348,57 @@ public void startSurfaceSinglePanePreNativeAndWithNativeTest() {
Assert.assertTrue(startSurfaceCoordinator.isInitializedWithNativeForTesting());
}

/**
* Tests that the IncognitoSwitchCoordinator isn't create in inflate() if the native library
* isn't ready. It will be lazily created after native initialization.
*/
@Test
@SmallTest
@Restriction({UiRestriction.RESTRICTION_TYPE_PHONE})
// clang-format off
@EnableFeatures({ChromeFeatureList.TAB_SWITCHER_ON_RETURN + "<Study,",
ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
@CommandLineFlags.Add({ChromeSwitches.DISABLE_NATIVE_INITIALIZATION,
"force-fieldtrials=Study/Group",
IMMEDIATE_RETURN_PARAMS + "/start_surface_variation/single"})
public void startSurfaceIncognitoSwitchCoordinatorInflatedWithNativeTest() {
// clang-format on
startMainActivityFromLauncher();
Assert.assertFalse(mActivityTestRule.getActivity().isTablet());
Assert.assertTrue(CachedFeatureFlags.isEnabled(ChromeFeatureList.INSTANT_START));
Assert.assertEquals("single", StartSurfaceConfiguration.START_SURFACE_VARIATION.getValue());
Assert.assertTrue(ReturnToChromeExperimentsUtil.shouldShowTabSwitcher(-1));

CriteriaHelper.pollUiThread(()
-> Assert.assertTrue(mActivityTestRule.getActivity()
.getLayoutManager()
.overviewVisible()));

Assert.assertFalse(LibraryLoader.getInstance().isInitialized());
TopToolbarCoordinator topToolbarCoordinator =
(TopToolbarCoordinator) mActivityTestRule.getActivity()
.getToolbarManager()
.getToolbar();

// TODO(https://crbug.com/1077022): Removes the call of
// {@link TopToolbarCoordinator#setTabSwithcherMode()} once ToolbarManager shows
// the StartSurfaceToolbar in instant start.
TestThreadUtils.runOnUiThreadBlocking(
() -> { topToolbarCoordinator.setTabSwitcherMode(true, true, false); });
onViewWaiting(allOf(withId(R.id.tab_switcher_toolbar), isDisplayed()));

StartSurfaceToolbarCoordinator startSurfaceToolbarCoordinator =
topToolbarCoordinator.getStartSurfaceToolbarForTesting();
// Verifies that the IncognitoSwitchCoordinator hasn't been created when the
// {@link StartSurfaceToolbarCoordinator#inflate()} is called.
Assert.assertNull(startSurfaceToolbarCoordinator.getIncognitoSwitchCoordinatorForTesting());

// Initializes native.
startAndWaitNativeInitialization();
Assert.assertNotNull(
startSurfaceToolbarCoordinator.getIncognitoSwitchCoordinatorForTesting());
}

@Test
@SmallTest
@Restriction({UiRestriction.RESTRICTION_TYPE_TABLET})
Expand Down
Expand Up @@ -16,7 +16,6 @@
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.toolbar.IncognitoStateProvider.IncognitoStateObserver;
import org.chromium.ui.base.DeviceFormFactor;
Expand Down Expand Up @@ -87,8 +86,6 @@ private void updateDrawableTint() {
final boolean shouldUseLightMode =
DeviceFormFactor.isNonMultiDisplayContextOnTablet(getContext())
|| ((DeviceClassManager.enableAccessibilityLayout()
|| ChromeFeatureList.isEnabled(
ChromeFeatureList.HORIZONTAL_TAB_SWITCHER_ANDROID)
|| TabUiFeatureUtilities.isGridTabSwitcherEnabled())
&& mIsIncognito);
ApiCompatibilityUtils.setImageTintList(
Expand Down
Expand Up @@ -9,6 +9,9 @@
import android.view.View.OnLongClickListener;
import android.view.ViewStub;

import androidx.annotation.VisibleForTesting;

import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ThemeColorProvider;
import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior;
Expand All @@ -30,7 +33,7 @@
* StartSurfaceToolbar has with the outside world. Lazily creates the tab toolbar the first time
* it's needed.
*/
class StartSurfaceToolbarCoordinator {
public class StartSurfaceToolbarCoordinator {
private final StartSurfaceToolbarMediator mToolbarMediator;
private final ViewStub mStub;
private final PropertyModel mPropertyModel;
Expand Down Expand Up @@ -210,6 +213,11 @@ void setOnTabSwitcherLongClickHandler(OnLongClickListener listener) {
}

void onNativeLibraryReady() {
// It is possible that the {@link mIncognitoSwitchCoordinator} isn't created because
// inflate() is called when the native library isn't ready. So create it now.
if (isInflated()) {
maybeCreateIncognitoSwitchCoordinator();
}
mToolbarMediator.onNativeLibraryReady();
}

Expand All @@ -218,10 +226,8 @@ private void inflate() {
mView = (StartSurfaceToolbarView) mStub.inflate();
mPropertyModelChangeProcessor = PropertyModelChangeProcessor.create(
mPropertyModel, mView, StartSurfaceToolbarViewBinder::bind);

if (IncognitoUtils.isIncognitoModeEnabled()
&& !StartSurfaceConfiguration.START_SURFACE_SHOW_STACK_TAB_SWITCHER.getValue()) {
mIncognitoSwitchCoordinator = new IncognitoSwitchCoordinator(mView, mTabModelSelector);
if (LibraryLoader.getInstance().isInitialized()) {
maybeCreateIncognitoSwitchCoordinator();
}

if (StartSurfaceConfiguration.START_SURFACE_SHOW_STACK_TAB_SWITCHER.getValue()) {
Expand All @@ -248,7 +254,21 @@ private void inflate() {
}
}

private void maybeCreateIncognitoSwitchCoordinator() {
if (mIncognitoSwitchCoordinator != null) return;

if (IncognitoUtils.isIncognitoModeEnabled()
&& !StartSurfaceConfiguration.START_SURFACE_SHOW_STACK_TAB_SWITCHER.getValue()) {
mIncognitoSwitchCoordinator = new IncognitoSwitchCoordinator(mView, mTabModelSelector);
}
}

private boolean isInflated() {
return mView != null;
}

@VisibleForTesting
public IncognitoSwitchCoordinator getIncognitoSwitchCoordinatorForTesting() {
return mIncognitoSwitchCoordinator;
}
}
Expand Up @@ -628,4 +628,9 @@ public int getHeight() {
public ToolbarLayout getToolbarLayoutForTesting() {
return mToolbarLayout;
}

@VisibleForTesting
public StartSurfaceToolbarCoordinator getStartSurfaceToolbarForTesting() {
return mStartSurfaceToolbarCoordinator;
}
}

0 comments on commit 6b1b7ea

Please sign in to comment.