Revert CCT header customization when dynamic module fails to load.
For the benefit of a consistent UI, CCT header customization is
immediately allowed when module loading is started. When module loading
fails, we should revert any customization incurred during module
loading. However, this is not happening prior to this CL because
maybeCustomizeCctHeader() becomes no-op when module fails to load.
We are also using the same criteria as used by CloseButtonNavigator to
determine when to customize the header.
Bug: 882404
Change-Id: Id58d80abf7cea8ecea0fccdf7f85ebe28970ec6d
Reviewed-on: https://chromium-review.googlesource.com/c/1437086
Commit-Queue: John Lin <chuanl@google.com>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633949}diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java
index 3d71630..cef14d1 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/dynamicmodule/DynamicModuleCoordinator.java
@@ -27,6 +27,7 @@
import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.browserservices.PostMessageHandler;
import org.chromium.chrome.browser.customtabs.CloseButtonNavigator;
+import org.chromium.chrome.browser.customtabs.CloseButtonNavigator.PageCriteria;
import org.chromium.chrome.browser.customtabs.CustomTabBottomBarDelegate;
import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
import org.chromium.chrome.browser.customtabs.CustomTabTopBarDelegate;
@@ -154,6 +155,8 @@
new DynamicModuleNavigationEventObserver();
private final DynamicModulePageLoadObserver mPageLoadObserver;
+ private final PageCriteria mPageCriteria;
+
@Inject
public DynamicModuleCoordinator(CustomTabIntentDataProvider intentDataProvider,
CloseButtonNavigator closeButtonNavigator,
@@ -184,8 +187,8 @@
mBottomBarDelegate = bottomBarDelegate;
mFullscreenManager = fullscreenManager;
- closeButtonNavigator.setLandingPageCriteria(url ->
- (isModuleLoading() || isModuleLoaded()) && isModuleManagedUrl(url));
+ mPageCriteria = url -> (isModuleLoading() || isModuleLoaded()) && isModuleManagedUrl(url);
+ closeButtonNavigator.setLandingPageCriteria(mPageCriteria);
activityLifecycleDispatcher.register(this);
}
@@ -385,6 +388,10 @@
return mModuleCallback != null;
}
+ /* package */ boolean hasModuleFailedToLoad() {
+ return mActivityDelegate == null;
+ }
+
private boolean isModuleManagedUrl(String url) {
if (TextUtils.isEmpty(url)) {
return false;
@@ -445,9 +452,9 @@
mIntentDataProvider.getSession());
}
- private View getProgressBarAnchorView(boolean isModuleManagedUrl) {
+ private View getProgressBarAnchorView(boolean showTopBar) {
View anchorView = null;
- if (isModuleManagedUrl) {
+ if (showTopBar) {
View topBarContentView = mTopBarDelegate.get().getTopBarContentView();
if (topBarContentView != null && topBarContentView.getVisibility() == View.VISIBLE) {
anchorView = topBarContentView;
@@ -459,19 +466,19 @@
}
private void maybeCustomizeCctHeader(String url) {
- if (!isModuleLoaded() && !isModuleLoading()) return;
+ if (!isModuleLoaded() && !isModuleLoading() && !hasModuleFailedToLoad()) return;
- boolean isModuleManagedUrl = isModuleManagedUrl(url);
- mTopBarDelegate.get().showTopBarIfNecessary(isModuleManagedUrl);
+ boolean showTopBar = mPageCriteria.matches(url);
+ mTopBarDelegate.get().showTopBarIfNecessary(showTopBar);
if (shouldHideCctHeaderOnModuleManagedUrls()) {
mActivity.getToolbarManager().setToolbarVisibility(
- isModuleManagedUrl ? View.GONE : mDefaultToolbarVisibility);
+ showTopBar ? View.GONE : mDefaultToolbarVisibility);
mActivity.getToolbarManager().setToolbarShadowVisibility(
- isModuleManagedUrl ? View.GONE : mDefaultToolbarShadowVisibility);
+ showTopBar ? View.GONE : mDefaultToolbarShadowVisibility);
mFullscreenManager.get().setTopControlsHeight(
- isModuleManagedUrl ? getTopBarHeight() : mDefaultTopControlContainerHeight);
+ showTopBar ? getTopBarHeight() : mDefaultTopControlContainerHeight);
mActivity.getToolbarManager().setProgressBarAnchorView(
- getProgressBarAnchorView(isModuleManagedUrl));
+ getProgressBarAnchorView(showTopBar));
}
}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModulePostMessageTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModulePostMessageTest.java
index 3e75871..9ed6b4c 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModulePostMessageTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModulePostMessageTest.java
@@ -193,7 +193,8 @@
// onFinishNativeInitialization() call happens before therefore if we instantiate
// DynamicModuleCoordinator now, the module will not be loaded.
DynamicModuleCoordinator coordinator = getModuleCoordinator();
- assertFalse(coordinator.isModuleLoading() || coordinator.isModuleLoaded());
+ assertFalse(coordinator.isModuleLoading() || coordinator.isModuleLoaded()
+ || coordinator.hasModuleFailedToLoad());
// We shouldn't be able to open a channel or post messages yet.
assertFalse(coordinator.requestPostMessageChannel(FAKE_ORIGIN_URI));
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleUITest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleUITest.java
index 015b870..ba93c53 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleUITest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/dynamicmodule/CustomTabsDynamicModuleUITest.java
@@ -318,7 +318,7 @@
@Test
@SmallTest
@Features.EnableFeatures(ChromeFeatureList.CCT_MODULE)
- public void testSetTopBarContentView_moduleLoadingFailed_noTopBar() throws Exception {
+ public void testSetTopBarContentView_moduleLoadingFailed_cctHeaderVisible() throws Exception {
Intent intent = new IntentBuilder(mTestPage).setModuleFailToLoadComponentName().build();
mActivityRule.startCustomTabActivityWithIntent(intent);
@@ -327,9 +327,13 @@
runOnUiThreadBlocking(() -> {
View anyView = new View(getActivity());
getModuleCoordinator().setTopBarContentView(anyView);
+ ViewGroup topBar = getActivity().findViewById(R.id.topbar);
+ Assert.assertNotNull(topBar);
+ Assert.assertThat(anyView.getParent(), equalTo(topBar));
+ assertEquals(View.GONE, anyView.getVisibility());
});
- assertNoTopBar();
+ assertCCTHeaderIsVisible();
}
@Test
