Download home : Show/hide prefetch tab correctly

This CL fixes the prefetch tab visibility on download home.
The prefetch tab is visible only when prefetching is enabled
which requires the feature and the settings to be enabled.
Also adds an observer to the user setting pref, so that the
tab shows/hides as soon as the setting is toggled.

Bug: 951821
Change-Id: I85bc2d0f346d01b1a0f7f9d8b42430f3614ed373
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1564902
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652425}

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

@@ -12,6 +12,8 @@
 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;
 
@@ -33,13 +35,17 @@ public interface Observer {
         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 PrefChangeRegistrar mPrefChangeRegistrar;
+
     /**
      * Builds a new FilterCoordinator.
      * @param context The context to build the views and pull parameters from.
@@ -55,7 +61,30 @@ public FilterCoordinator(Context context, OfflineItemFilterSource chipFilterSour
         mModel.set(FilterProperties.CHANGE_LISTENER, this::handleTabSelected);
         selectTab(TabType.FILES);
 
-        mModel.set(FilterProperties.SHOW_TABS, PrefetchConfiguration.isPrefetchingFlagEnabled());
+        mModel.set(FilterProperties.SHOW_TABS, isPrefetchTabEnabled());
+
+        addPrefetchUserSettingsObserver();
+    }
+
+    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);
+                    }
+                });
+    }
+
+    /** Tears down this coordinator. */
+    public void destroy() {
+        if (mPrefChangeRegistrar != null) mPrefChangeRegistrar.destroy();
     }
 
     /** @return The {@link View} representing this widget. */
@@ -73,15 +102,19 @@ public void removeObserver(Observer observer) {
         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.
      */
     public void setSelectedFilter(@FilterType int filter) {
         @TabType
         int tabSelected;
-        if (filter == Filters.FilterType.PREFETCHED
-                && PrefetchConfiguration.isPrefetchingFlagEnabled()) {
+        if (filter == Filters.FilterType.PREFETCHED && isPrefetchTabEnabled()) {
             tabSelected = TabType.PREFETCH;
         } else {
             mChipsProvider.setFilterSelected(filter);
@@ -122,4 +155,11 @@ private void notifyFilterChanged(@FilterType int filter) {
     private void handleChipSelected() {
         handleTabSelected(mModel.get(FilterProperties.SELECTED_TAB));
     }
-}
+
+    private static boolean isPrefetchTabEnabled() {
+        return sPrefetchUserSettingValueForTesting == null
+                ? PrefetchConfiguration.isPrefetchingFlagEnabled()
+                        && PrefetchConfiguration.isPrefetchingEnabledInSettings()
+                : sPrefetchUserSettingValueForTesting;
+    }
+}

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

@@ -152,6 +152,7 @@ private void initializeView(Context context) {
 
     /** Tears down this coordinator. */
     public void destroy() {
+        mFilterCoordinator.destroy();
         mMediator.destroy();
         mRenameDialogManager.destroy();
     }

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

@@ -20,6 +20,7 @@
 import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.UrlConstants;
+import org.chromium.chrome.browser.download.home.filter.FilterCoordinator;
 import org.chromium.chrome.browser.download.home.list.UiUtils;
 import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
 import org.chromium.chrome.browser.download.ui.StubbedProvider;
@@ -94,6 +95,7 @@ public void setUpTest() throws Exception {
     }
 
     private void setUpUi() {
+        FilterCoordinator.setPrefetchUserSettingValueForTesting(true);
         DownloadManagerUiConfig config = new DownloadManagerUiConfig.Builder()
                                                  .setIsOffTheRecord(false)
                                                  .setIsSeparateActivity(true)

chrome/browser/android/preferences/prefs.h

@@ -17,6 +17,7 @@
 #include "components/feed/core/pref_names.h"
 #endif  // BUILDFLAG(ENABLE_FEED_IN_CHROME)
 #include "components/ntp_snippets/pref_names.h"
+#include "components/offline_pages/core/prefetch/prefetch_prefs.h"
 #include "components/payments/core/payment_prefs.h"
 
 // A preference exposed to Java.
@@ -36,6 +37,7 @@ enum Pref {
   AUTOFILL_PROFILE_ENABLED,
   AUTOFILL_CREDIT_CARD_ENABLED,
   USAGE_STATS_ENABLED,
+  OFFLINE_PREFETCH_USER_SETTING_ENABLED,
   // PREF_NUM_PREFS must be the last entry.
   PREF_NUM_PREFS
 };
@@ -63,6 +65,7 @@ const char* const kPrefsExposedToJava[] = {
     autofill::prefs::kAutofillProfileEnabled,
     autofill::prefs::kAutofillCreditCardEnabled,
     prefs::kUsageStatsEnabled,
+    offline_pages::prefetch_prefs::kUserSettingEnabled,
 };
 
 #endif  // CHROME_BROWSER_ANDROID_PREFERENCES_PREFS_H_

chrome/browser/android/preferences/prefs_unittest.cc

@@ -62,6 +62,8 @@ TEST_F(PrefsTest, TestIndex) {
   EXPECT_EQ(autofill::prefs::kAutofillCreditCardEnabled,
             GetPrefName(AUTOFILL_CREDIT_CARD_ENABLED));
   EXPECT_EQ(prefs::kUsageStatsEnabled, GetPrefName(USAGE_STATS_ENABLED));
+  EXPECT_EQ(offline_pages::prefetch_prefs::kUserSettingEnabled,
+            GetPrefName(OFFLINE_PREFETCH_USER_SETTING_ENABLED));
 
   // If this check fails, a pref is missing a test case above.
   EXPECT_EQ(Pref::PREF_NUM_PREFS, pref_count_);

components/offline_pages/core/prefetch/prefetch_prefs.cc

@@ -13,7 +13,6 @@ namespace offline_pages {
 namespace prefetch_prefs {
 namespace {
 // Prefs only accessed in this file
-const char kEnabled[] = "offline_prefetch.enabled";
 const char kLimitlessPrefetchingEnabledTimePref[] =
     "offline_prefetch.limitless_prefetching_enabled_time";
 const char kPrefetchTestingHeaderPref[] =
@@ -24,11 +23,12 @@ const base::TimeDelta kForbiddenCheckDelay = base::TimeDelta::FromDays(7);
 
 }  // namespace
 
+const char kUserSettingEnabled[] = "offline_prefetch.enabled";
 const char kBackoff[] = "offline_prefetch.backoff";
 
 void RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(kBackoff);
-  registry->RegisterBooleanPref(kEnabled, true);
+  registry->RegisterBooleanPref(kUserSettingEnabled, true);
   registry->RegisterTimePref(kLimitlessPrefetchingEnabledTimePref,
                              base::Time());
   registry->RegisterStringPref(kPrefetchTestingHeaderPref, std::string());
@@ -37,16 +37,16 @@ void RegisterPrefs(PrefRegistrySimple* registry) {
 }
 
 void SetPrefetchingEnabledInSettings(PrefService* prefs, bool enabled) {
-  prefs->SetBoolean(kEnabled, enabled);
+  prefs->SetBoolean(kUserSettingEnabled, enabled);
 }
 
 bool IsPrefetchingEnabledInSettings(PrefService* prefs) {
-  return prefs->GetBoolean(kEnabled);
+  return prefs->GetBoolean(kUserSettingEnabled);
 }
 
 bool IsEnabled(PrefService* prefs) {
-  return IsPrefetchingOfflinePagesEnabled() && prefs->GetBoolean(kEnabled) &&
-         IsEnabledByServer(prefs);
+  return IsPrefetchingOfflinePagesEnabled() &&
+         prefs->GetBoolean(kUserSettingEnabled) && IsEnabledByServer(prefs);
 }
 
 void SetLimitlessPrefetchingEnabled(PrefService* prefs, bool enabled) {
@@ -85,8 +85,8 @@ std::string GetPrefetchTestingHeader(PrefService* prefs) {
 bool IsForbiddenCheckDue(PrefService* prefs) {
   DCHECK(prefs);
   base::Time checkTime = prefs->GetTime(kNextForbiddenCheckTimePref);
-  return IsPrefetchingOfflinePagesEnabled() && prefs->GetBoolean(kEnabled) &&
-         !IsEnabledByServer(prefs) &&
+  return IsPrefetchingOfflinePagesEnabled() &&
+         prefs->GetBoolean(kUserSettingEnabled) && !IsEnabledByServer(prefs) &&
          (checkTime < OfflineTimeNow() ||  // did the delay expire?
           checkTime >
               OfflineTimeNow() +

components/offline_pages/core/prefetch/prefetch_prefs.h

@@ -17,6 +17,7 @@ namespace offline_pages {
 namespace prefetch_prefs {
 
 extern const char kBackoff[];
+extern const char kUserSettingEnabled[];
 
 void RegisterPrefs(PrefRegistrySimple* registry);