PreviewTab: Ensure to instantiate the coordinator per activity in CCT

https://crrev.com/c/2214802 was still instantiating
EphemeralTabCoordinator for every Tab object. This CL ensures that
there will be a single instance for CustomTabActivity.

Bug: 1084731, 1086860
Change-Id: I04697f82397376befa63b05e47575d19d636aadf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216148
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773967}

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/ephemeraltab/EphemeralTabCoordinator.java

@@ -4,6 +4,10 @@
 
 package org.chromium.chrome.browser.compositor.bottombar.ephemeraltab;
 
+import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.ACTIVITY_CONTEXT;
+import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.DECOR_VIEW;
+import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.IS_PROMOTABLE_TO_TAB_BOOLEAN;
+
 import android.content.Context;
 import android.graphics.drawable.Drawable;
 import android.os.Handler;
@@ -13,9 +17,11 @@ import org.chromium.base.Callback;
 import org.chromium.base.SysUtils;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ChromeVersionInfo;
 import org.chromium.chrome.browser.WebContentsFactory;
 import org.chromium.chrome.browser.content.ContentUtils;
+import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -35,24 +41,28 @@ import org.chromium.components.feature_engagement.Tracker;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.UiUtils;
+import org.chromium.ui.base.ActivityWindowAndroid;
 import org.chromium.ui.base.PageTransition;
 import org.chromium.ui.base.ViewAndroidDelegate;
-import org.chromium.ui.base.WindowAndroid;
+
+import javax.inject.Inject;
+import javax.inject.Named;
 
 /**
  * Central class for ephemeral tab, responsible for spinning off other classes necessary to display
  * the preview tab UI.
  */
+@ActivityScope
 public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
     private final Context mContext;
-    private final WindowAndroid mWindow;
+    private final ActivityWindowAndroid mWindow;
     private final View mLayoutView;
     private final Handler mHandler = new Handler();
-    private final Supplier<Tab> mTabProvider;
+    private final ActivityTabProvider mTabProvider;
     private final Supplier<TabCreator> mTabCreator;
-    private final Supplier<BottomSheetController> mBottomSheetController;
+    private final BottomSheetController mBottomSheetController;
     private final EphemeralTabMetrics mMetrics = new EphemeralTabMetrics();
-    private final Supplier<Boolean> mCanPromoteToNewTab;
+    private final boolean mCanPromoteToNewTab;
 
     private EphemeralTabMediator mMediator;
 
@@ -72,15 +82,17 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
      * @param context The associated {@link Context}.
      * @param window The associated {@link WindowAndroid}.
      * @param layoutView The {@link View} to listen layout change on.
-     * @param tabProvider Supplier for the current activity tab.
+     * @param tabProvider Provider of the current activity tab.
      * @param tabCreator Supplier for {@link TabCreator} handling a new tab creation.
-     * @param bottomSheetController Supplier for The associated {@link BottomSheetController}.
-     * @param canPromoteToNewTab A predicate tells if the tab can be promoted to a normal tab.
+     * @param bottomSheetController {@link BottomSheetController} as the container of the tab.
+     * @param canPromoteToNewTab Whether the tab can be promoted to a normal tab.
      */
-    public EphemeralTabCoordinator(Context context, WindowAndroid window, View layoutView,
-            Supplier<Tab> tabProvider, Supplier<TabCreator> tabCreator,
-            Supplier<BottomSheetController> bottomSheetController,
-            Supplier<Boolean> canPromoteToNewTab) {
+    @Inject
+    public EphemeralTabCoordinator(@Named(ACTIVITY_CONTEXT) Context context,
+            ActivityWindowAndroid window, @Named(DECOR_VIEW) View layoutView,
+            ActivityTabProvider tabProvider, Supplier<TabCreator> tabCreator,
+            BottomSheetController bottomSheetController,
+            @Named(IS_PROMOTABLE_TO_TAB_BOOLEAN) boolean canPromoteToNewTab) {
         mContext = context;
         mWindow = window;
         mLayoutView = layoutView;
@@ -122,7 +134,7 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
             float topControlsHeight =
                     mContext.getResources().getDimensionPixelSize(R.dimen.toolbar_height_no_shadow)
                     / mWindow.getDisplay().getDipScale();
-            mMediator = new EphemeralTabMediator(mBottomSheetController.get(),
+            mMediator = new EphemeralTabMediator(mBottomSheetController,
                     new FaviconLoader(mContext), mMetrics, (int) topControlsHeight);
         }
         if (mWebContents == null) {
@@ -168,12 +180,10 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
                 @Override
                 public void onSheetOffsetChanged(float heightFraction, float offsetPx) {
                     if (mSheetContent == null) return;
-                    if (mCanPromoteToNewTab.get()) {
-                        mSheetContent.showOpenInNewTabButton(heightFraction);
-                    }
+                    if (mCanPromoteToNewTab) mSheetContent.showOpenInNewTabButton(heightFraction);
                 }
             };
-            mBottomSheetController.get().addObserver(mSheetObserver);
+            mBottomSheetController.addObserver(mSheetObserver);
             mSheetContent = new EphemeralTabSheetContent(mContext, this::openInNewTab,
                     this::onToolbarClick, this::close, this::destroyContent, getMaxSheetHeight());
             mMediator.init(mWebContents, mContentView, mSheetContent, profile);
@@ -216,14 +226,14 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
 
         // Delay the clean-up till the state transitions are completed.
         mHandler.post(() -> {
-            if (mSheetObserver != null) mBottomSheetController.get().removeObserver(mSheetObserver);
+            if (mSheetObserver != null) mBottomSheetController.removeObserver(mSheetObserver);
             mSheetContent = null;
         });
     }
 
     private void openInNewTab() {
-        if (mCanPromoteToNewTab.get() && mUrl != null) {
-            mBottomSheetController.get().hideContent(
+        if (mCanPromoteToNewTab && mUrl != null) {
+            mBottomSheetController.hideContent(
                     mSheetContent, /* animate= */ true, StateChangeReason.PROMOTE_TAB);
             mTabCreator.get().createNewTab(new LoadUrlParams(mUrl, PageTransition.LINK),
                     TabLaunchType.FROM_LINK, mTabProvider.get());
@@ -232,11 +242,11 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
     }
 
     private void onToolbarClick() {
-        int state = mBottomSheetController.get().getSheetState();
+        int state = mBottomSheetController.getSheetState();
         if (state == SheetState.PEEK) {
-            mBottomSheetController.get().expandSheet();
+            mBottomSheetController.expandSheet();
         } else if (state == SheetState.FULL) {
-            mBottomSheetController.get().collapseSheet(true);
+            mBottomSheetController.collapseSheet(true);
         }
     }
 
@@ -244,7 +254,7 @@ public class EphemeralTabCoordinator implements View.OnLayoutChangeListener {
      * Close the ephemeral tab.
      */
     public void close() {
-        mBottomSheetController.get().hideContent(mSheetContent, /* animate= */ true);
+        mBottomSheetController.hideContent(mSheetContent, /* animate= */ true);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabDelegateFactory.java

@@ -66,6 +66,8 @@ import org.chromium.ui.mojom.WindowOpenDisposition;
 
 import javax.inject.Inject;
 
+import dagger.Lazy;
+
 /**
  * A {@link TabDelegateFactory} class to be used in all {@link Tab} owned
  * by a {@link CustomTabActivity}.
@@ -343,7 +345,7 @@ public class CustomTabDelegateFactory implements TabDelegateFactory {
 
     private TabWebContentsDelegateAndroid mWebContentsDelegateAndroid;
     private ExternalNavigationDelegateImpl mNavigationDelegate;
-    private Supplier<EphemeralTabCoordinator> mEphemeralTabCoordinatorSupplier;
+    private Lazy<EphemeralTabCoordinator> mEphemeralTabCoordinator;
 
     /**
      * @param activity {@link ChromeActivity} instance.
@@ -367,7 +369,7 @@ public class CustomTabDelegateFactory implements TabDelegateFactory {
             @WebDisplayMode int displayMode, boolean shouldEnableEmbeddedMediaExperience,
             BrowserControlsVisibilityDelegate visibilityDelegate, ExternalAuthUtils authUtils,
             MultiWindowUtils multiWindowUtils, @Nullable PendingIntent focusIntent,
-            Verifier verifier, Supplier<EphemeralTabCoordinator> ephemeralTabCoordinatorSupplier) {
+            Verifier verifier, Lazy<EphemeralTabCoordinator> ephemeralTabCoordinator) {
         mActivity = activity;
         mShouldHideBrowserControls = shouldHideBrowserControls;
         mIsOpenedByChrome = isOpenedByChrome;
@@ -380,7 +382,7 @@ public class CustomTabDelegateFactory implements TabDelegateFactory {
         mMultiWindowUtils = multiWindowUtils;
         mFocusIntent = focusIntent;
         mVerifier = verifier;
-        mEphemeralTabCoordinatorSupplier = ephemeralTabCoordinatorSupplier;
+        mEphemeralTabCoordinator = ephemeralTabCoordinator;
     }
 
     @Inject
@@ -388,13 +390,13 @@ public class CustomTabDelegateFactory implements TabDelegateFactory {
             BrowserServicesIntentDataProvider intentDataProvider,
             CustomTabBrowserControlsVisibilityDelegate visibilityDelegate,
             ExternalAuthUtils authUtils, MultiWindowUtils multiWindowUtils, Verifier verifier,
-            Supplier<EphemeralTabCoordinator> ephemeralTabCoordinatorSupplier) {
+            Lazy<EphemeralTabCoordinator> ephemeralTabCoordinator) {
         this(activity, intentDataProvider.shouldEnableUrlBarHiding(),
                 intentDataProvider.isOpenedByChrome(), getWebApkScopeUrl(intentDataProvider),
                 getDisplayMode(intentDataProvider),
                 intentDataProvider.shouldEnableEmbeddedMediaExperience(), visibilityDelegate,
                 authUtils, multiWindowUtils, intentDataProvider.getFocusIntent(), verifier,
-                ephemeralTabCoordinatorSupplier);
+                ephemeralTabCoordinator);
     }
 
     /**
@@ -457,8 +459,10 @@ public class CustomTabDelegateFactory implements TabDelegateFactory {
                 mActivity == null ? null : mActivity.getShareDelegateSupplier();
         TabModelSelector tabModelSelector =
                 mActivity != null ? mActivity.getTabModelSelector() : null;
-        return new ChromeContextMenuPopulator(new TabContextMenuItemDelegate(tab, tabModelSelector,
-                                                      mEphemeralTabCoordinatorSupplier),
+        return new ChromeContextMenuPopulator(
+                new TabContextMenuItemDelegate(tab, tabModelSelector,
+                        EphemeralTabCoordinator.isSupported() ? mEphemeralTabCoordinator::get
+                                                              : () -> null),
                 shareDelegateSupplier, contextMenuMode, ExternalAuthUtils.getInstance());
     }
 

chrome/android/java/src/org/chromium/chrome/browser/dependency_injection/ChromeActivityCommonsModule.java

@@ -5,21 +5,24 @@
 package org.chromium.chrome.browser.dependency_injection;
 
 import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.ACTIVITY_CONTEXT;
+import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.DECOR_VIEW;
+import static org.chromium.chrome.browser.dependency_injection.ChromeCommonQualifiers.IS_PROMOTABLE_TO_TAB_BOOLEAN;
 
 import android.app.Activity;
 import android.content.Context;
 import android.content.res.Resources;
+import android.view.View;
 
 import org.chromium.base.supplier.Supplier;
 import org.chromium.chrome.browser.ActivityTabProvider;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.compositor.CompositorViewHolder;
-import org.chromium.chrome.browser.compositor.bottombar.ephemeraltab.EphemeralTabCoordinator;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
 import org.chromium.chrome.browser.compositor.layouts.content.TabContentManager;
 import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
+import org.chromium.chrome.browser.tabmodel.TabCreatorManager.TabCreator;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.ui.system.StatusBarColorController;
@@ -95,6 +98,12 @@ public class ChromeActivityCommonsModule {
         return mActivity;
     }
 
+    @Provides
+    @Named(DECOR_VIEW)
+    public View provideDecorView() {
+        return mActivity.getWindow().getDecorView();
+    }
+
     @Provides
     public Resources provideResources() {
         return mActivity.getResources();
@@ -135,6 +144,17 @@ public class ChromeActivityCommonsModule {
         return mActivity;
     }
 
+    @Provides
+    public Supplier<TabCreator> provideTabCreator() {
+        return mActivity::getCurrentTabCreator;
+    }
+
+    @Provides
+    @Named(IS_PROMOTABLE_TO_TAB_BOOLEAN)
+    public boolean provideIsPromotableToTab() {
+        return !mActivity.isCustomTab();
+    }
+
     @Provides
     public StatusBarColorController provideStatusBarColorController() {
         return mActivity.getStatusBarColorController();
@@ -149,17 +169,4 @@ public class ChromeActivityCommonsModule {
     public NotificationManagerProxy provideNotificationManagerProxy() {
         return new NotificationManagerProxyImpl(mActivity.getApplicationContext());
     }
-
-    @Provides
-    public Supplier<EphemeralTabCoordinator> provideEphemeralTabCoordinatorSupplier() {
-        return () -> {
-            // |isSupported| uses feature flag facility which is not ready at the point
-            // of injection. Delay the decision till the coordinator is actually used.
-            if (!EphemeralTabCoordinator.isSupported()) return null;
-            return new EphemeralTabCoordinator(mActivity, mActivity.getWindowAndroid(),
-                    mActivity.getWindow().getDecorView(), mActivity.getActivityTabProvider(),
-                    mActivity::getCurrentTabCreator, mActivity::getBottomSheetController,
-                    () -> !mActivity.isCustomTab());
-        };
-    }
 }

chrome/android/java/src/org/chromium/chrome/browser/dependency_injection/ChromeCommonQualifiers.java

@@ -13,4 +13,7 @@ public interface ChromeCommonQualifiers {
 
     String ACTIVITY_CONTEXT = "ACTIVITY_CONTEXT";
     String APP_CONTEXT = "APP_CONTEXT";
+
+    String DECOR_VIEW = "DECOR_VIEW";
+    String IS_PROMOTABLE_TO_TAB_BOOLEAN = "IS_PROMOTABLE_TO_TAB_BOOLEAN";
 }

chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java

@@ -172,7 +172,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
             mEphemeralTabCoordinatorSupplier.set(new EphemeralTabCoordinator(mActivity,
                     mActivity.getWindowAndroid(), mActivity.getWindow().getDecorView(),
                     mActivity.getActivityTabProvider(), mActivity::getCurrentTabCreator,
-                    mActivity::getBottomSheetController, () -> true));
+                    mActivity.getBottomSheetController(), true));
         }
 
         PostTask.postTask(UiThreadTaskTraits.DEFAULT, this::initializeIPH);