Cap the maximum number of entries in the PreviewsOfflineHelper pref

The max size is specified by variations.

Bug: 914577
Change-Id: Iddc694ee5b6a4abbd3435956df23a586a918b547
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1577931
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653436}

chrome/browser/previews/previews_offline_helper.cc

@@ -52,8 +52,8 @@ base::Optional<base::Time> TimeFromDictionaryValue(std::string value) {
 // Cleans up the given dictionary by removing all stale (expiry has passed)
 // entries.
 void RemoveStaleEntries(base::DictionaryValue* dict) {
-  // TODO(crbug.com/914577): Cap the size of the pref by deleting the oldest one
-  // above a certain size.
+  base::Time earliest_expiry = base::Time::Max();
+  std::string earliest_key;
   std::vector<std::string> keys_to_delete;
   for (const auto& iter : dict->DictItems()) {
     // Check for a corrupted value and throw it out if so.
@@ -73,12 +73,23 @@ void RemoveStaleEntries(base::DictionaryValue* dict) {
         time.value() + previews::params::OfflinePreviewFreshnessDuration();
     bool is_expired = expiry <= base::Time::Now();
 
-    if (is_expired)
+    if (is_expired) {
       keys_to_delete.push_back(iter.first);
+      continue;
+    }
+
+    if (expiry < earliest_expiry)
+      earliest_key = iter.first;
   }
 
   for (const std::string& key : keys_to_delete)
     dict->RemoveKey(key);
+
+  // RemoveStaleEntries is called for every new added page, so it's fine to just
+  // remove one at a time to keep the pref size below a threshold.
+  if (dict->DictSize() > previews::params::OfflinePreviewsHelperMaxPrefSize()) {
+    dict->RemoveKey(earliest_key);
+  }
 }
 
 }  // namespace

chrome/browser/previews/previews_offline_helper_unittest.cc

@@ -216,3 +216,27 @@ TEST_F(PreviewsOfflineHelperTest, TestAddRemovePages) {
     }
   }
 }
+
+TEST_F(PreviewsOfflineHelperTest, TestMaxPrefSize) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeatureWithParameters(
+      previews::features::kOfflinePreviewsFalsePositivePrevention,
+      {{"max_pref_entries", "1"}});
+
+  PreviewsOfflineHelper* helper = NewHelper();
+
+  base::Time first = base::Time::Now();
+  base::Time second = first + base::TimeDelta::FromMinutes(1);
+
+  helper->OfflinePageAdded(
+      nullptr, MakeAddedPageItem("http://test.first.com", "", first));
+  EXPECT_TRUE(
+      helper->ShouldAttemptOfflinePreview(GURL("http://test.first.com")));
+
+  helper->OfflinePageAdded(
+      nullptr, MakeAddedPageItem("http://test.second.com", "", second));
+  EXPECT_FALSE(
+      helper->ShouldAttemptOfflinePreview(GURL("http://test.first.com")));
+  EXPECT_TRUE(
+      helper->ShouldAttemptOfflinePreview(GURL("http://test.second.com")));
+}

components/previews/core/previews_experiments.cc

@@ -387,6 +387,12 @@ int ResourceLoadingHintsPreviewsInflationBytes() {
       features::kResourceLoadingHints, kResourceLoadingHintsInflationBytes, 0);
 }
 
+size_t OfflinePreviewsHelperMaxPrefSize() {
+  return GetFieldTrialParamByFeatureAsInt(
+      features::kOfflinePreviewsFalsePositivePrevention, "max_pref_entries",
+      100);
+}
+
 }  // namespace params
 
 std::string GetStringNameForType(PreviewsType type) {

components/previews/core/previews_experiments.h

@@ -195,6 +195,10 @@ int ResourceLoadingHintsPreviewsInflationPercent();
 // bytes to for inflating the original_bytes count.
 int ResourceLoadingHintsPreviewsInflationBytes();
 
+// The maximum number of pref entries that should be kept by
+// PreviewsOfflineHelper.
+size_t OfflinePreviewsHelperMaxPrefSize();
+
 }  // namespace params
 
 }  // namespace previews