[EoC] Create suggestions views after sheet is opened

Wait to create views for the suggested content until after the sheet is
opened. This will reduce memory cost if the sheet is never opened.

BUG=822955

Change-Id: I657f601cc1862227514eb599df205da2c5dee1ad
Reviewed-on: https://chromium-review.googlesource.com/990773
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548084}

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContentCoordinator.java

@@ -27,11 +27,11 @@ import org.chromium.ui.base.WindowAndroid;
  * {@link ContextualSuggestionsCoordinator} and lifecycle of sub-component objects.
  */
 class ContentCoordinator {
-    private final ContextualSuggestionsModel mModel;
-    private final WindowAndroid mWindowAndroid;
-    private final ContextMenuManager mContextMenuManager;
+    private final SuggestionsRecyclerView mRecyclerView;
 
-    private SuggestionsRecyclerView mRecyclerView;
+    private ContextualSuggestionsModel mModel;
+    private WindowAndroid mWindowAndroid;
+    private ContextMenuManager mContextMenuManager;
     private RecyclerViewModelChangeProcessor<ClusterListObservable, NewTabPageViewHolder>
             mModelChangeProcessor;
 
@@ -39,6 +39,26 @@ class ContentCoordinator {
      * Construct a new {@link ContentCoordinator}.
      * @param context The {@link Context} used to retrieve resources.
      * @param parentView The parent {@link View} to which the content will eventually be attached.
+     */
+    ContentCoordinator(Context context, ViewGroup parentView) {
+        mRecyclerView = (SuggestionsRecyclerView) LayoutInflater.from(context).inflate(
+                R.layout.contextual_suggestions_layout, parentView, false);
+    }
+
+    /** @return The content {@link View}. */
+    View getView() {
+        return mRecyclerView;
+    }
+
+    /** @return The vertical scroll offset of the content view. */
+    int getVerticalScrollOffset() {
+        return mRecyclerView.computeVerticalScrollOffset();
+    }
+
+    /**
+     * Show suggestions, retrieved from the model, in the content view.
+     *
+     * @param context The {@link Context} used to retrieve resources.
      * @param profile The regular {@link Profile}.
      * @param uiDelegate The {@link SuggestionsUiDelegate} used to help construct items in the
      *                   content view.
@@ -46,15 +66,12 @@ class ContentCoordinator {
      * @param windowAndroid The {@link WindowAndroid} for attaching a context menu listener.
      * @param closeContextMenuCallback The callback when a context menu is closed.
      */
-    ContentCoordinator(Context context, ViewGroup parentView, Profile profile,
-            SuggestionsUiDelegate uiDelegate, ContextualSuggestionsModel model,
-            WindowAndroid windowAndroid, Runnable closeContextMenuCallback) {
+    void showSuggestions(Context context, Profile profile, SuggestionsUiDelegate uiDelegate,
+            ContextualSuggestionsModel model, WindowAndroid windowAndroid,
+            Runnable closeContextMenuCallback) {
         mModel = model;
         mWindowAndroid = windowAndroid;
 
-        mRecyclerView = (SuggestionsRecyclerView) LayoutInflater.from(context).inflate(
-                R.layout.contextual_suggestions_layout, parentView, false);
-
         mContextMenuManager = new ContextMenuManager(uiDelegate.getNavigationDelegate(),
                 mRecyclerView::setTouchEnabled, closeContextMenuCallback);
         mWindowAndroid.addContextMenuCloseListener(mContextMenuManager);
@@ -76,21 +93,15 @@ class ContentCoordinator {
         });
     }
 
-    /** @return The content {@link View}. */
-    View getView() {
-        return mRecyclerView;
-    }
-
-    /** @return The vertical scroll offset of the content view. */
-    int getVerticalScrollOffset() {
-        return mRecyclerView.computeVerticalScrollOffset();
-    }
-
     /** Destroy the content component. */
     void destroy() {
         // The model outlives the content sub-component. Remove the observer so that this object
         // can be garbage collected.
-        mModel.mClusterListObservable.removeObserver(mModelChangeProcessor);
-        mWindowAndroid.removeContextMenuCloseListener(mContextMenuManager);
+        if (mModelChangeProcessor != null) {
+            mModel.mClusterListObservable.removeObserver(mModelChangeProcessor);
+        }
+        if (mWindowAndroid != null) {
+            mWindowAndroid.removeContextMenuCloseListener(mContextMenuManager);
+        }
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsCoordinator.java

@@ -15,6 +15,7 @@ import org.chromium.chrome.browser.suggestions.SuggestionsUiDelegateImpl;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet;
 import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
 
 /**
  * The coordinator for the contextual suggestions UI component. Manages communication with other
@@ -64,13 +65,45 @@ public class ContextualSuggestionsCoordinator {
     }
 
     /**
-     * Preload the contextual suggestions in the {@link BottomSheet}; content won't actually be
-     * shown until {@link #showSuggestions()} is called.
+     * Preload the contextual suggestions content in the {@link BottomSheet}; the sheet won't
+     * actually be shown until {@link #showContentInSheet()} is called.
+     */
+    void preloadContentInSheet() {
+        mToolbarCoordinator =
+                new ToolbarCoordinator(mActivity, mBottomSheetController.getBottomSheet(), mModel);
+        mContentCoordinator =
+                new ContentCoordinator(mActivity, mBottomSheetController.getBottomSheet());
+        mBottomSheetContent = new ContextualSuggestionsBottomSheetContent(
+                mContentCoordinator, mToolbarCoordinator);
+        // TODO(twellington): Handle the case where preload returns false.
+        mBottomSheetController.requestContentPreload(mBottomSheetContent);
+    }
+
+    /**
+     * Show the contextual suggestions content in the {@link BottomSheet}.
+     * {@link #preloadContentInSheet()} must be called prior to calling this method.
+     *
+     * Only the views needed for peeking the bottom sheet will be constructed. Another
+     * call to {@link #displaySuggestions()} is needed to finish inflating views for the
+     * suggestions cards.
+     */
+    void showContentInSheet() {
+        // #preloadContentInSheet will always be called before this method, regardless of
+        // whether content was actually put in the sheet (meaning mBottomSheetContent should never
+        // be null). If content is not successfully preloaded
+        // BottomSheetController#requestContentPreload will return false.
+        assert mBottomSheetContent != null;
+        mBottomSheetController.requestShowContent(mBottomSheetContent, false);
+    }
+
+    /**
+     * Finish showing the contextual suggestions in the {@link BottomSheet}.
+     * {@link #showContentInSheet()} must be called prior to calling this method.
      *
      * @param suggestionsSource The {@link SuggestionsSource} used to retrieve additional things
      *                          needed to display suggestions (e.g. favicons, thumbnails).
      */
-    void preloadSuggestionsInSheet(SuggestionsSource suggestionsSource) {
+    void showSuggestions(SuggestionsSource suggestionsSource) {
         SuggestionsNavigationDelegate navigationDelegate = new SuggestionsNavigationDelegateImpl(
                 mActivity, mProfile, mBottomSheetController.getBottomSheet(), mTabModelSelector);
         SuggestionsUiDelegateImpl uiDelegate =
@@ -79,31 +112,24 @@ public class ContextualSuggestionsCoordinator {
                         mActivity.getChromeApplication().getReferencePool(),
                         mActivity.getSnackbarManager());
 
-        // TODO(twellington): Introduce another method that creates bottom sheet content with only
-        // a toolbar view when suggestions are fist available, and use this method to construct the
-        // content view when the sheet is opened.
-        mToolbarCoordinator =
-                new ToolbarCoordinator(mActivity, mBottomSheetController.getBottomSheet(), mModel);
-        mContentCoordinator = new ContentCoordinator(mActivity,
-                mBottomSheetController.getBottomSheet(), mProfile, uiDelegate, mModel,
+        mContentCoordinator.showSuggestions(mActivity, mProfile, uiDelegate, mModel,
                 mActivity.getWindowAndroid(), mActivity::closeContextMenu);
-        mBottomSheetContent = new ContextualSuggestionsBottomSheetContent(
-                mContentCoordinator, mToolbarCoordinator);
-        // TODO(twellington): Handle the case where preload returns false.
-        mBottomSheetController.requestContentPreload(mBottomSheetContent);
     }
 
     /**
-     * Show the contextual suggestions in the {@link BottomSheet}.
-     * {@link #preloadSuggestionsInSheet()} must be called prior to calling this method.
+     * Add an observer to the {@link BottomSheet}.
+     * @param observer The observer to add.
      */
-    void showSuggestions() {
-        // #preloadSuggestionsInSheet will always be called before this method, regardless of
-        // whether content was actually put in the sheet (meaning mBottomSheetContent should never
-        // be null). If content is not successfully preloaded
-        // BottomSheetController#requestContentPreload will return false.
-        assert mBottomSheetContent != null;
-        mBottomSheetController.requestShowContent(mBottomSheetContent, false);
+    void addBottomSheetObserver(BottomSheetObserver observer) {
+        mBottomSheetController.getBottomSheet().addObserver(observer);
+    }
+
+    /**
+     * Remove an observer to the {@link BottomSheet}.
+     * @param observer The observer to remove.
+     */
+    void removeBottomSheetObserver(BottomSheetObserver observer) {
+        mBottomSheetController.getBottomSheet().removeObserver(observer);
     }
 
     /** Removes contextual suggestions from the {@link BottomSheet}. */

chrome/android/java/src/org/chromium/chrome/browser/contextual_suggestions/ContextualSuggestionsMediator.java

@@ -17,6 +17,9 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tabmodel.TabModelSelector;
 import org.chromium.chrome.browser.util.MathUtils;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheet.StateChangeReason;
+import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
+import org.chromium.chrome.browser.widget.bottomsheet.EmptyBottomSheetObserver;
 import org.chromium.ui.widget.Toast;
 
 import java.util.ArrayList;
@@ -37,12 +40,10 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
     private final EnabledStateMonitor mEnabledStateMonitor;
     private final ChromeFullscreenManager mFullscreenManager;
 
-    @Nullable
-    private SnippetsBridge mBridge;
-    @Nullable
-    private FetchHelper mFetchHelper;
-    @Nullable
-    private String mCurrentRequestUrl;
+    private @Nullable SnippetsBridge mBridge;
+    private @Nullable FetchHelper mFetchHelper;
+    private @Nullable String mCurrentRequestUrl;
+    private @Nullable BottomSheetObserver mSheetObserver;
 
     private boolean mDidSuggestionsShowForTab;
 
@@ -82,8 +83,7 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
                 // the top controls.
                 if (!mDidSuggestionsShowForTab && mModel.hasSuggestions()
                         && areBrowserControlsHidden() && mBridge != null) {
-                    mDidSuggestionsShowForTab = true;
-                    mCoordinator.showSuggestions();
+                    showContentInSheet();
                 }
             }
 
@@ -97,8 +97,17 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
 
     /** Destroys the mediator. */
     void destroy() {
-        if (mFetchHelper != null) mFetchHelper.destroy();
         mEnabledStateMonitor.destroy();
+
+        if (mFetchHelper != null) {
+            mFetchHelper.destroy();
+            mFetchHelper = null;
+        }
+
+        if (mBridge != null) {
+            mBridge.destroy();
+            mBridge = null;
+        }
     }
 
     /**
@@ -117,11 +126,15 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
         } else {
             clearSuggestions();
 
-            mFetchHelper.destroy();
-            mFetchHelper = null;
+            if (mFetchHelper != null) {
+                mFetchHelper.destroy();
+                mFetchHelper = null;
+            }
 
-            mBridge.destroy();
-            mBridge = null;
+            if (mBridge != null) {
+                mBridge.destroy();
+                mBridge = null;
+            }
         }
     }
 
@@ -129,6 +142,8 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
     public void requestSuggestions(String url) {
         mCurrentRequestUrl = url;
         mBridge.fetchContextualSuggestions(url, (suggestions) -> {
+            if (mBridge == null) return;
+
             // Avoiding double fetches causing suggestions for incorrect context.
             if (!TextUtils.equals(url, mCurrentRequestUrl)) return;
 
@@ -137,10 +152,10 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
                     .show();
 
             if (suggestions.size() > 0) {
-                preloadSuggestions(generateClusterList(suggestions), suggestions.get(0).mTitle);
+                preloadContentInSheet(generateClusterList(suggestions), suggestions.get(0).mTitle);
                 // If the controls are already off-screen, show the suggestions immediately so they
                 // are available on reverse scroll.
-                if (areBrowserControlsHidden()) mCoordinator.showSuggestions();
+                if (areBrowserControlsHidden()) showContentInSheet();
             }
         });
     }
@@ -163,15 +178,35 @@ class ContextualSuggestionsMediator implements EnabledStateMonitor.Observer, Fet
         mModel.setTitle(null);
         mCoordinator.removeSuggestions();
         mCurrentRequestUrl = "";
+
+        if (mSheetObserver != null) {
+            mCoordinator.removeBottomSheetObserver(mSheetObserver);
+        }
     }
 
-    private void preloadSuggestions(ClusterList clusters, String title) {
+    private void preloadContentInSheet(ClusterList clusters, String title) {
         if (mBridge == null) return;
 
         mModel.setClusterList(clusters);
         mModel.setCloseButtonOnClickListener(view -> { clearSuggestions(); });
         mModel.setTitle(mContext.getString(R.string.contextual_suggestions_toolbar_title, title));
-        mCoordinator.preloadSuggestionsInSheet(mBridge);
+        mCoordinator.preloadContentInSheet();
+    }
+
+    private void showContentInSheet() {
+        mDidSuggestionsShowForTab = true;
+
+        mSheetObserver = new EmptyBottomSheetObserver() {
+            @Override
+            public void onSheetOpened(@StateChangeReason int reason) {
+                mCoordinator.showSuggestions(mBridge);
+                mCoordinator.removeBottomSheetObserver(this);
+                mSheetObserver = null;
+            }
+        };
+
+        mCoordinator.addBottomSheetObserver(mSheetObserver);
+        mCoordinator.showContentInSheet();
     }
 
     // TODO(twellington): Remove after clusters are returned from the backend.