Remove dependencies from DownloadsHome on Chrome

- Remove dependencies on the offlinepages package by passing in an
ObservableSupplier for whether or not prefetch is enabled.

- Remove dependencies on DownloadSettings (which has dependencies that
cannot be moved yet) by passing in a Callback<Context> to launch the
settings screen.

- Remove a dependency on the OfflineContentAggregatorFactory (which
should stay as glue code) by passing in the OfflineContentProvider
interface.

Bug: 1046032
Change-Id: I5d8935661be63e452f08578f18cc33ed735b0536
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023505
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735767}

chrome/android/chrome_java_sources.gni

@@ -546,6 +546,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/download/home/LegacyDownloadProvider.java",
   "java/src/org/chromium/chrome/browser/download/home/LegacyDownloadProviderImpl.java",
   "java/src/org/chromium/chrome/browser/download/home/OfflineItemSource.java",
+  "java/src/org/chromium/chrome/browser/download/home/PrefetchEnabledSupplier.java",
   "java/src/org/chromium/chrome/browser/download/home/StableIds.java",
   "java/src/org/chromium/chrome/browser/download/home/empty/EmptyCoordinator.java",
   "java/src/org/chromium/chrome/browser/download/home/empty/EmptyProperties.java",

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadActivity.java

@@ -70,7 +70,7 @@ public class DownloadActivity extends SnackbarActivity implements ModalDialogMan
         mModalDialogManager = new ModalDialogManager(
                 new AppModalPresenter(this), ModalDialogManager.ModalDialogType.APP);
         mDownloadCoordinator = DownloadManagerCoordinatorFactory.create(
-                this, config, getSnackbarManager(), parentComponent, mModalDialogManager);
+                this, config, getSnackbarManager(), mModalDialogManager);
         setContentView(mDownloadCoordinator.getView());
         mIsOffTheRecord = isOffTheRecord;
         mDownloadCoordinator.addObserver(mUiObserver);

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadPage.java

@@ -43,9 +43,8 @@ public class DownloadPage extends BasicNativePage implements DownloadManagerCoor
                         .setShowPaginationHeaders(DownloadUtils.shouldShowPaginationHeaders())
                         .build();
 
-        mDownloadCoordinator = DownloadManagerCoordinatorFactory.create(activity, config,
-                activity.getSnackbarManager(), activity.getComponentName(),
-                activity.getModalDialogManager());
+        mDownloadCoordinator = DownloadManagerCoordinatorFactory.create(
+                activity, config, activity.getSnackbarManager(), activity.getModalDialogManager());
 
         mDownloadCoordinator.addObserver(this);
         mTitle = activity.getString(R.string.menu_downloads);

chrome/android/java/src/org/chromium/chrome/browser/download/home/DownloadManagerCoordinatorFactory.java

@@ -5,10 +5,13 @@
 package org.chromium.chrome.browser.download.home;
 
 import android.app.Activity;
-import android.content.ComponentName;
+import android.content.Context;
 
+import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
 import org.chromium.chrome.browser.feature_engagement.TrackerFactory;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.settings.SettingsLauncher;
+import org.chromium.chrome.browser.settings.download.DownloadSettings;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.ui.modaldialog.ModalDialogManager;
 
@@ -18,19 +21,24 @@ public class DownloadManagerCoordinatorFactory {
      * Returns an instance of a {@link DownloadManagerCoordinator} to be used in the UI.
      * @param activity           The parent {@link Activity}.
      * @param config             A {@link DownloadManagerUiConfig} to provide configuration params.
-     * @param parentComponent    The parent component.
      * @param snackbarManager    The {@link SnackbarManager} that should be used to show snackbars.
      * @param modalDialogManager The {@link ModalDialogManager} that should be used to show dialog.
      * @return                   A new {@link DownloadManagerCoordinator} instance.
      */
     public static DownloadManagerCoordinator create(Activity activity,
             DownloadManagerUiConfig config, SnackbarManager snackbarManager,
-            ComponentName parentComponent, ModalDialogManager modalDialogManager) {
+            ModalDialogManager modalDialogManager) {
         Profile profile = Profile.getMainProfile();
         LegacyDownloadProvider legacyProvider =
                 config.useNewDownloadPath ? new LegacyDownloadProviderImpl() : null;
-        return new DownloadManagerCoordinatorImpl(activity, config, snackbarManager,
+        return new DownloadManagerCoordinatorImpl(activity, config, new PrefetchEnabledSupplier(),
+                DownloadManagerCoordinatorFactory::settingsLaunchHelper, snackbarManager,
                 modalDialogManager, TrackerFactory.getTrackerForProfile(profile),
-                new FaviconProviderImpl(profile), legacyProvider);
+                new FaviconProviderImpl(profile), OfflineContentAggregatorFactory.get(),
+                legacyProvider);
+    }
+
+    private static void settingsLaunchHelper(Context context) {
+        SettingsLauncher.getInstance().launchSettingsPage(context, DownloadSettings.class);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/download/home/DownloadManagerCoordinatorImpl.java

@@ -5,14 +5,17 @@
 package org.chromium.chrome.browser.download.home;
 
 import android.app.Activity;
+import android.content.Context;
 import android.view.Gravity;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.FrameLayout;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.Callback;
 import org.chromium.base.ObserverList;
 import org.chromium.base.metrics.RecordUserAction;
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.chrome.browser.download.home.filter.Filters;
 import org.chromium.chrome.browser.download.home.filter.Filters.FilterType;
 import org.chromium.chrome.browser.download.home.list.DateOrderedListCoordinator;
@@ -20,13 +23,11 @@ import org.chromium.chrome.browser.download.home.list.DateOrderedListCoordinator
 import org.chromium.chrome.browser.download.home.list.ListItem;
 import org.chromium.chrome.browser.download.home.snackbars.DeleteUndoCoordinator;
 import org.chromium.chrome.browser.download.home.toolbar.ToolbarCoordinator;
-import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
-import org.chromium.chrome.browser.settings.SettingsLauncher;
-import org.chromium.chrome.browser.settings.download.DownloadSettings;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.widget.selection.SelectionDelegate;
 import org.chromium.chrome.download.R;
 import org.chromium.components.feature_engagement.Tracker;
+import org.chromium.components.offline_items_collection.OfflineContentProvider;
 import org.chromium.ui.modaldialog.ModalDialogManager;
 
 import java.io.Closeable;
@@ -45,6 +46,7 @@ class DownloadManagerCoordinatorImpl
     private final SelectionDelegate<ListItem> mSelectionDelegate;
 
     private final Activity mActivity;
+    private final Callback<Context> mSettingsLauncher;
 
     private ViewGroup mMainView;
 
@@ -52,13 +54,16 @@ class DownloadManagerCoordinatorImpl
 
     /** Builds a {@link DownloadManagerCoordinatorImpl} instance. */
     public DownloadManagerCoordinatorImpl(Activity activity, DownloadManagerUiConfig config,
-            SnackbarManager snackbarManager, ModalDialogManager modalDialogManager, Tracker tracker,
-            FaviconProvider faviconProvider, LegacyDownloadProvider legacyProvider) {
+            ObservableSupplier<Boolean> isPrefetchEnabledSupplier,
+            Callback<Context> settingsLauncher, SnackbarManager snackbarManager,
+            ModalDialogManager modalDialogManager, Tracker tracker, FaviconProvider faviconProvider,
+            OfflineContentProvider provider, LegacyDownloadProvider legacyProvider) {
         mActivity = activity;
+        mSettingsLauncher = settingsLauncher;
         mDeleteCoordinator = new DeleteUndoCoordinator(snackbarManager);
         mSelectionDelegate = new SelectionDelegate<ListItem>();
         mListCoordinator = new DateOrderedListCoordinator(mActivity, config,
-                OfflineContentAggregatorFactory.get(), legacyProvider,
+                isPrefetchEnabledSupplier, provider, legacyProvider,
                 mDeleteCoordinator::showSnackbar, mSelectionDelegate, this::notifyFilterChanged,
                 createDateOrderedListObserver(), modalDialogManager, faviconProvider);
         mToolbarCoordinator = new ToolbarCoordinator(mActivity, this, mListCoordinator,
@@ -152,7 +157,7 @@ class DownloadManagerCoordinatorImpl
     @Override
     public void openSettings() {
         RecordUserAction.record("Android.DownloadManager.Settings");
-        SettingsLauncher.getInstance().launchSettingsPage(mActivity, DownloadSettings.class);
+        mSettingsLauncher.onResult(mActivity);
     }
 
     private void notifyFilterChanged(@FilterType int filter) {

chrome/android/java/src/org/chromium/chrome/browser/download/home/PrefetchEnabledSupplier.java

+// 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.download.home;
+
+import org.chromium.base.Callback;
+import org.chromium.base.ObserverList;
+import org.chromium.base.supplier.ObservableSupplier;
+import org.chromium.chrome.browser.offlinepages.prefetch.PrefetchConfiguration;
+import org.chromium.chrome.browser.preferences.Pref;
+import org.chromium.chrome.browser.preferences.PrefChangeRegistrar;
+
+/**
+ * Helper class to determine whether or not the prefetch setting is enabled for Chrome.
+ * This class does not require an explicit destroy call, but needs all observers to be
+ * unregistered for full clean up.
+ */
+class PrefetchEnabledSupplier implements ObservableSupplier<Boolean> {
+    private final ObserverList<Callback<Boolean>> mObservers = new ObserverList<>();
+    private PrefChangeRegistrar mPrefChangeRegistrar;
+
+    // ObservableSupplier implementation.
+    @Override
+    public Boolean get() {
+        return isPrefetchEnabled();
+    }
+
+    @Override
+    public Boolean addObserver(Callback<Boolean> obs) {
+        if (mObservers.isEmpty()) startTrackingPref();
+
+        mObservers.addObserver(obs);
+
+        return isPrefetchEnabled();
+    }
+
+    @Override
+    public void removeObserver(Callback<Boolean> obs) {
+        mObservers.removeObserver(obs);
+        if (mObservers.isEmpty()) stopTrackingPref();
+    }
+
+    private void startTrackingPref() {
+        if (mPrefChangeRegistrar != null) return;
+        mPrefChangeRegistrar = new PrefChangeRegistrar();
+        mPrefChangeRegistrar.addObserver(
+                Pref.OFFLINE_PREFETCH_USER_SETTING_ENABLED, this::notifyObservers);
+    }
+
+    private void stopTrackingPref() {
+        if (mPrefChangeRegistrar == null) return;
+        mPrefChangeRegistrar.destroy();
+        mPrefChangeRegistrar = null;
+    }
+
+    private void notifyObservers() {
+        boolean enabled = isPrefetchEnabled();
+        for (Callback<Boolean> obs : mObservers) obs.onResult(enabled);
+    }
+
+    private static boolean isPrefetchEnabled() {
+        return PrefetchConfiguration.isPrefetchingFlagEnabled()
+                && PrefetchConfiguration.isPrefetchingEnabledInSettings();
+    }
+}
\ No newline at end of file

chrome/android/java/src/org/chromium/chrome/browser/download/home/filter/FilterCoordinator.java

@@ -9,12 +9,11 @@ import android.view.View;
 
 import androidx.annotation.IntDef;
 
+import org.chromium.base.Callback;
 import org.chromium.base.ObserverList;
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.chrome.browser.download.home.filter.Filters.FilterType;
 import org.chromium.chrome.browser.download.home.filter.chips.ChipsCoordinator;
-import org.chromium.chrome.browser.offlinepages.prefetch.PrefetchConfiguration;
-import org.chromium.chrome.browser.preferences.Pref;
-import org.chromium.chrome.browser.preferences.PrefChangeRegistrar;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 
@@ -36,25 +35,29 @@ public class FilterCoordinator {
         void onFilterChanged(@FilterType int selectedTab);
     }
 
-    private static Boolean sPrefetchUserSettingValueForTesting;
-
     private final ObserverList<Observer> mObserverList = new ObserverList<>();
     private final PropertyModel mModel = new PropertyModel(FilterProperties.ALL_KEYS);
     private final FilterView mView;
 
     private final ChipsCoordinator mChipsCoordinator;
     private final FilterChipsProvider mChipsProvider;
+    private final ObservableSupplier<Boolean> mIsPrefetchEnabledSupplier;
 
-    private PrefChangeRegistrar mPrefChangeRegistrar;
-
+    private final Callback<Boolean> mIsPrefetchEnabledObserver = this::onPrefetchEnabledChanged;
     /**
      * Builds a new FilterCoordinator.
      * @param context The context to build the views and pull parameters from.
+     * @param chipFilterSource The list of OfflineItems to use to generate the set of available
+     *         filters.
+     * @param isPrefetchEnabledSupplier A supplier that indicates whether or not prefetch is
+     *         enabled.
      */
-    public FilterCoordinator(Context context, OfflineItemFilterSource chipFilterSource) {
+    public FilterCoordinator(Context context, OfflineItemFilterSource chipFilterSource,
+            ObservableSupplier<Boolean> isPrefetchEnabledSupplier) {
         mChipsProvider =
                 new FilterChipsProvider(context, type -> handleChipSelected(), chipFilterSource);
         mChipsCoordinator = new ChipsCoordinator(context, mChipsProvider);
+        mIsPrefetchEnabledSupplier = isPrefetchEnabledSupplier;
 
         mView = new FilterView(context);
         PropertyModelChangeProcessor.create(mModel, mView, new FilterViewBinder());
@@ -62,30 +65,14 @@ public class FilterCoordinator {
         mModel.set(FilterProperties.CHANGE_LISTENER, this::handleTabSelected);
         selectTab(TabType.FILES);
 
-        mModel.set(FilterProperties.SHOW_TABS, isPrefetchTabEnabled());
-
-        addPrefetchUserSettingsObserver();
-    }
+        mModel.set(FilterProperties.SHOW_TABS, mIsPrefetchEnabledSupplier.get());
 
-    private void addPrefetchUserSettingsObserver() {
-        if (sPrefetchUserSettingValueForTesting != null) return;
-
-        mPrefChangeRegistrar = new PrefChangeRegistrar();
-        mPrefChangeRegistrar.addObserver(
-                Pref.OFFLINE_PREFETCH_USER_SETTING_ENABLED, new PrefChangeRegistrar.PrefObserver() {
-                    @Override
-                    public void onPreferenceChange() {
-                        mModel.set(FilterProperties.SHOW_TABS, isPrefetchTabEnabled());
-                        int selectedTab = mModel.get(FilterProperties.SELECTED_TAB);
-                        if (!isPrefetchTabEnabled()) selectedTab = TabType.FILES;
-                        handleTabSelected(selectedTab);
-                    }
-                });
+        mIsPrefetchEnabledSupplier.addObserver(mIsPrefetchEnabledObserver);
     }
 
     /** Tears down this coordinator. */
     public void destroy() {
-        if (mPrefChangeRegistrar != null) mPrefChangeRegistrar.destroy();
+        mIsPrefetchEnabledSupplier.removeObserver(mIsPrefetchEnabledObserver);
     }
 
     /** @return The {@link View} representing this widget. */
@@ -103,11 +90,6 @@ public class FilterCoordinator {
         mObserverList.removeObserver(observer);
     }
 
-    /** For testing only. */
-    public static void setPrefetchUserSettingValueForTesting(boolean enabled) {
-        sPrefetchUserSettingValueForTesting = enabled;
-    }
-
     /**
      * Pushes a selected filter onto this {@link FilterCoordinator}.  This is used when external
      * components might need to update the UI state.
@@ -115,7 +97,7 @@ public class FilterCoordinator {
     public void setSelectedFilter(@FilterType int filter) {
         @TabType
         int tabSelected;
-        if (filter == Filters.FilterType.PREFETCHED && isPrefetchTabEnabled()) {
+        if (filter == Filters.FilterType.PREFETCHED && mIsPrefetchEnabledSupplier.get()) {
             tabSelected = TabType.PREFETCH;
         } else {
             mChipsProvider.setFilterSelected(filter);
@@ -157,10 +139,10 @@ public class FilterCoordinator {
         handleTabSelected(mModel.get(FilterProperties.SELECTED_TAB));
     }
 
-    private static boolean isPrefetchTabEnabled() {
-        return sPrefetchUserSettingValueForTesting == null
-                ? PrefetchConfiguration.isPrefetchingFlagEnabled()
-                        && PrefetchConfiguration.isPrefetchingEnabledInSettings()
-                : sPrefetchUserSettingValueForTesting;
+    private void onPrefetchEnabledChanged(Boolean enabled) {
+        mModel.set(FilterProperties.SHOW_TABS, enabled);
+        int selectedTab = mModel.get(FilterProperties.SELECTED_TAB);
+        if (!enabled) selectedTab = TabType.FILES;
+        handleTabSelected(selectedTab);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/DateOrderedListCoordinator.java

@@ -14,6 +14,7 @@ import android.widget.FrameLayout;
 
 import org.chromium.base.Callback;
 import org.chromium.base.Log;
+import org.chromium.base.supplier.ObservableSupplier;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.download.home.DownloadManagerUiConfig;
 import org.chromium.chrome.browser.download.home.FaviconProvider;
@@ -92,6 +93,8 @@ public class DateOrderedListCoordinator implements ToolbarCoordinator.ToolbarLis
      * {@code provider} as a list of items.
      * @param context The {@link Context} to use to build the views.
      * @param config The {@link DownloadManagerUiConfig} to provide UI configuration params.
+     * @param isPrefetchEnabledSupplier A supplier that indicates whether or not prefetch is
+     *         enabled.
      * @param provider The {@link OfflineContentProvider} to visually represent.
      * @param legacyProvider A legacy version of a provider for downloads.
      * @param deleteController A class to manage whether or not items can be deleted.
@@ -101,8 +104,9 @@ public class DateOrderedListCoordinator implements ToolbarCoordinator.ToolbarLis
      * @param dateOrderedListObserver A {@link DateOrderedListObserver}.
      */
     public DateOrderedListCoordinator(Context context, DownloadManagerUiConfig config,
-            OfflineContentProvider provider, LegacyDownloadProvider legacyProvider,
-            DeleteController deleteController, SelectionDelegate<ListItem> selectionDelegate,
+            ObservableSupplier<Boolean> isPrefetchEnabledSupplier, OfflineContentProvider provider,
+            LegacyDownloadProvider legacyProvider, DeleteController deleteController,
+            SelectionDelegate<ListItem> selectionDelegate,
             FilterCoordinator.Observer filterObserver,
             DateOrderedListObserver dateOrderedListObserver, ModalDialogManager modalDialogManager,
             FaviconProvider faviconProvider) {
@@ -122,7 +126,8 @@ public class DateOrderedListCoordinator implements ToolbarCoordinator.ToolbarLis
 
         mStorageCoordinator = new StorageCoordinator(context, mMediator.getFilterSource());
 
-        mFilterCoordinator = new FilterCoordinator(context, mMediator.getFilterSource());
+        mFilterCoordinator = new FilterCoordinator(
+                context, mMediator.getFilterSource(), isPrefetchEnabledSupplier);
         mFilterCoordinator.addObserver(mMediator::onFilterTypeSelected);
         mFilterCoordinator.addObserver(filterObserver);
         mFilterCoordinator.addObserver(mEmptyCoordinator);

chrome/android/javatests/src/org/chromium/chrome/browser/download/home/DownloadActivityV2Test.java

@@ -24,6 +24,7 @@ import static org.hamcrest.core.AllOf.allOf;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
 
+import android.content.Context;
 import android.os.Handler;
 import android.os.Looper;
 import android.support.test.espresso.action.ViewActions;
@@ -37,11 +38,11 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
 import org.chromium.base.Callback;
+import org.chromium.base.supplier.ObservableSupplierImpl;
 import org.chromium.base.task.PostTask;
 import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.base.test.util.Restriction;
-import org.chromium.chrome.browser.download.home.filter.FilterCoordinator;
 import org.chromium.chrome.browser.download.home.rename.RenameUtils;
 import org.chromium.chrome.browser.download.home.toolbar.DownloadHomeToolbar;
 import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
@@ -133,7 +134,6 @@ public class DownloadActivityV2Test extends DummyUiActivityTestCase {
     }
 
     private void setUpUi() {
-        FilterCoordinator.setPrefetchUserSettingValueForTesting(true);
         DownloadManagerUiConfig config = DownloadManagerUiConfigHelper.fromFlags()
                                                  .setIsOffTheRecord(false)
                                                  .setIsSeparateActivity(true)
@@ -147,8 +147,13 @@ public class DownloadActivityV2Test extends DummyUiActivityTestCase {
                 new ModalDialogManager(mAppModalPresenter, ModalDialogManager.ModalDialogType.APP);
 
         FaviconProvider faviconProvider = (url, faviconSizePx, callback) -> {};
+        Callback<Context> settingsLauncher = context -> {};
+        ObservableSupplierImpl<Boolean> isPrefetchEnabledSupplier = new ObservableSupplierImpl<>();
+        isPrefetchEnabledSupplier.set(true);
+
         mDownloadCoordinator = new DownloadManagerCoordinatorImpl(getActivity(), config,
-                mSnackbarManager, mModalDialogManager, mTracker, faviconProvider,
+                isPrefetchEnabledSupplier, settingsLauncher, mSnackbarManager, mModalDialogManager,
+                mTracker, faviconProvider, OfflineContentAggregatorFactory.get(),
                 /* LegacyDownloadProvider */ null);
         getActivity().setContentView(mDownloadCoordinator.getView());