Limit the number of page hints that can be loaded to the memory

The threshold is controlled using field trial.
Also, add few more histograms to track any regressions.

Change-Id: I7901f70196dd23de5eec6fc9574602c71fbe0235
Bug: 882913
Reviewed-on: https://chromium-review.googlesource.com/1214748
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590550}

components/previews/content/previews_hints.cc

@@ -176,6 +176,8 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
   url_matcher::URLMatcherConditionSet::Vector all_conditions;
   std::set<std::string> seen_host_suffixes;
 
+  size_t total_resource_loading_hints_received = 0;
+  size_t total_page_patterns_with_resource_loading_hints_received = 0;
   // Process hint configuration.
   for (const auto hint : config.hints()) {
     // We only support host suffixes at the moment. Skip anything else.
@@ -223,9 +225,37 @@ std::unique_ptr<PreviewsHints> PreviewsHints::CreateFromConfig(
     if (ShouldProcessPageHints() && !hint.page_hints().empty()) {
       UMA_HISTOGRAM_COUNTS("ResourceLoadingHints.PageHints.ProcessedCount",
                            hint.page_hints().size());
-      hints->initial_hints_.push_back(hint);
+
+      for (const auto& page_hint : hint.page_hints()) {
+        for (const auto& optimization : page_hint.whitelisted_optimizations()) {
+          base::Optional<PreviewsType> previews_type =
+              ConvertProtoOptimizationTypeToPreviewsOptimizationType(
+                  optimization.optimization_type());
+
+          if (!previews_type ||
+              previews_type != PreviewsType::RESOURCE_LOADING_HINTS) {
+            continue;
+          }
+          total_page_patterns_with_resource_loading_hints_received++;
+          total_resource_loading_hints_received +=
+              optimization.resource_loading_hints().size();
+        }
+      }
+
+      if (total_page_patterns_with_resource_loading_hints_received <=
+          previews::params::GetMaxPageHintsInMemoryThreshhold()) {
+        hints->initial_hints_.push_back(hint);
+      }
     }
   }
+  if (ShouldProcessPageHints()) {
+    UMA_HISTOGRAM_COUNTS_100000(
+        "ResourceLoadingHints.ResourceHints.TotalReceived",
+        total_resource_loading_hints_received);
+    UMA_HISTOGRAM_COUNTS_1000(
+        "ResourceLoadingHints.PageHints.TotalReceived",
+        total_page_patterns_with_resource_loading_hints_received);
+  }
   hints->url_matcher_.AddConditionSets(all_conditions);
 
   // Completed processing hints data without crashing so clear sentinel.

components/previews/content/previews_optimization_guide_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/optional.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/test/gtest_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
@@ -117,7 +118,16 @@ class PreviewsOptimizationGuideTest : public testing::Test {
   void DoExperimentFlagTest(base::Optional<std::string> experiment_name,
                             bool expect_enabled);
 
-  void InitializeResourceLoadingHints();
+  // This is a helper function for initializing fixed number of ResourceLoading
+  // hints.
+  void InitializeFixedCountResourceLoadingHints();
+
+  // This is a helper function for initializing multiple ResourceLoading hints.
+  // The generated hint proto contains hints for |key_count| keys.
+  // |page_patterns_per_key| page patterns are specified per key.
+  // For each page pattern, 2 resource loading hints are specified in the proto.
+  void InitializeMultipleResourceLoadingHints(size_t key_count,
+                                              size_t page_patterns_per_key);
 
  private:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
@@ -598,8 +608,7 @@ TEST_F(PreviewsOptimizationGuideTest, IsWhitelistedWithMultipleHintMatches) {
   EXPECT_FALSE(guide()->IsWhitelisted(*request5, PreviewsType::NOSCRIPT));
 }
 
-// This is a helper function for initializing some ResourceLoading hints.
-void PreviewsOptimizationGuideTest::InitializeResourceLoadingHints() {
+void PreviewsOptimizationGuideTest::InitializeFixedCountResourceLoadingHints() {
   optimization_guide::proto::Configuration config;
   optimization_guide::proto::Hint* hint1 = config.add_hints();
   hint1->set_key("somedomain.org");
@@ -630,6 +639,51 @@ void PreviewsOptimizationGuideTest::InitializeResourceLoadingHints() {
   resource_loading_hint2->set_loading_optimization_type(
       optimization_guide::proto::LOADING_BLOCK_RESOURCE);
   resource_loading_hint2->set_resource_pattern("football_cruft.js");
+
+  optimization_guide::proto::ResourceLoadingHint* resource_loading_hint3 =
+      optimization2->add_resource_loading_hints();
+  resource_loading_hint3->set_loading_optimization_type(
+      optimization_guide::proto::LOADING_BLOCK_RESOURCE);
+  resource_loading_hint3->set_resource_pattern("barball_cruft.js");
+  ProcessHints(config, "2.0.0");
+
+  RunUntilIdle();
+}
+
+void PreviewsOptimizationGuideTest::InitializeMultipleResourceLoadingHints(
+    size_t key_count,
+    size_t page_patterns_per_key) {
+  optimization_guide::proto::Configuration config;
+
+  for (size_t key_index = 0; key_index < key_count; ++key_index) {
+    optimization_guide::proto::Hint* hint = config.add_hints();
+    hint->set_key("somedomain" + base::NumberToString(key_index) + ".org");
+    hint->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
+
+    for (size_t page_pattern_index = 0;
+         page_pattern_index < page_patterns_per_key; ++page_pattern_index) {
+      // Page hint for "/news/"
+      optimization_guide::proto::PageHint* page_hint = hint->add_page_hints();
+      page_hint->set_page_pattern("/news/" +
+                                  base::NumberToString(page_pattern_index));
+      optimization_guide::proto::Optimization* optimization1 =
+          page_hint->add_whitelisted_optimizations();
+      optimization1->set_optimization_type(
+          optimization_guide::proto::RESOURCE_LOADING);
+
+      optimization_guide::proto::ResourceLoadingHint* resource_loading_hint_1 =
+          optimization1->add_resource_loading_hints();
+      resource_loading_hint_1->set_loading_optimization_type(
+          optimization_guide::proto::LOADING_BLOCK_RESOURCE);
+      resource_loading_hint_1->set_resource_pattern("news_cruft_1.js");
+
+      optimization_guide::proto::ResourceLoadingHint* resource_loading_hint_2 =
+          optimization1->add_resource_loading_hints();
+      resource_loading_hint_2->set_loading_optimization_type(
+          optimization_guide::proto::LOADING_BLOCK_RESOURCE);
+      resource_loading_hint_2->set_resource_pattern("news_cruft_2.js");
+    }
+  }
   ProcessHints(config, "2.0.0");
 
   RunUntilIdle();
@@ -640,7 +694,7 @@ TEST_F(PreviewsOptimizationGuideTest, MaybeLoadOptimizationHints) {
   base::test::ScopedFeatureList scoped_list;
   scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
 
-  InitializeResourceLoadingHints();
+  InitializeFixedCountResourceLoadingHints();
 
   EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
       *CreateRequestWithURL(GURL("https://somedomain.org/")),
@@ -657,6 +711,10 @@ TEST_F(PreviewsOptimizationGuideTest, MaybeLoadOptimizationHints) {
   RunUntilIdle();
   histogram_tester.ExpectUniqueSample(
       "ResourceLoadingHints.PageHints.ProcessedCount", 2, 1);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.ResourceHints.TotalReceived", 3, 1);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.PageHints.TotalReceived", 2, 1);
 
   // Verify loaded hint data for www.somedomain.org
   EXPECT_EQ(GURL("https://www.somedomain.org/news/football"),
@@ -678,6 +736,132 @@ TEST_F(PreviewsOptimizationGuideTest, MaybeLoadOptimizationHints) {
       PreviewsType::RESOURCE_LOADING_HINTS));
 }
 
+// Test that optimization hints with multiple page patterns is processed
+// correctly.
+TEST_F(PreviewsOptimizationGuideTest,
+       LoadManyResourceLoadingOptimizationHints) {
+  base::HistogramTester histogram_tester;
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
+
+  const size_t key_count = 20;
+  const size_t page_patterns_per_key = 25;
+
+  ASSERT_EQ(previews::params::GetMaxPageHintsInMemoryThreshhold(),
+            key_count * page_patterns_per_key);
+
+  // Count of page patterns is within the threshold.
+  ASSERT_LE(key_count * page_patterns_per_key,
+            previews::params::GetMaxPageHintsInMemoryThreshhold());
+
+  InitializeMultipleResourceLoadingHints(key_count, page_patterns_per_key);
+
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain0.org/")),
+      base::DoNothing()));
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://www.somedomain0.org/news0/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(
+          GURL("https://www.somedomain0.org/news499/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(
+          GURL("https://www.somedomain0.org/news500/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain19.org/")),
+      base::DoNothing()));
+  EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain20.org/")),
+      base::DoNothing()));
+  EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://www.unknown.com")),
+      base::DoNothing()));
+
+  RunUntilIdle();
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.PageHints.ProcessedCount", page_patterns_per_key,
+      key_count);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.ResourceHints.TotalReceived",
+      key_count * page_patterns_per_key * 2, 1);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.PageHints.TotalReceived",
+      key_count * page_patterns_per_key, 1);
+}
+
+// Test that only up to GetMaxPageHintsInMemoryThreshhold() page hints
+// are loaded to the memory.
+TEST_F(PreviewsOptimizationGuideTest,
+       LoadTooManyResourceLoadingOptimizationHints) {
+  base::HistogramTester histogram_tester;
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(features::kResourceLoadingHints);
+
+  const size_t key_count = 21;
+  const size_t page_patterns_per_key = 25;
+
+  ASSERT_EQ(previews::params::GetMaxPageHintsInMemoryThreshhold(),
+            20u * page_patterns_per_key);
+
+  // Provide more page patterns than the threshold.
+  ASSERT_GT(key_count * page_patterns_per_key,
+            previews::params::GetMaxPageHintsInMemoryThreshhold());
+
+  InitializeMultipleResourceLoadingHints(key_count, page_patterns_per_key);
+
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain0.org/")),
+      base::DoNothing()));
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://www.somedomain0.org/news0/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain19.org/")),
+      base::DoNothing()));
+  EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(
+          GURL("https://www.somedomain19.org/news0/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+
+  // The last page pattern should be dropped since it exceeds the threshold
+  // count.
+  EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(GURL("https://somedomain20.org/")),
+      base::DoNothing()));
+  EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
+      *CreateRequestWithURL(
+          GURL("https://www.somedomain20.org/news0/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
+
+  RunUntilIdle();
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.PageHints.ProcessedCount", page_patterns_per_key,
+      key_count);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.ResourceHints.TotalReceived",
+      key_count * page_patterns_per_key * 2, 1);
+  histogram_tester.ExpectUniqueSample(
+      "ResourceLoadingHints.PageHints.TotalReceived",
+      key_count * page_patterns_per_key, 1);
+}
+
 TEST_F(PreviewsOptimizationGuideTest,
        MaybeLoadOptimizationHintsWithoutEnabledPageHintsFeature) {
   // Without PageHints-oriented feature enabled, never see
@@ -685,7 +869,7 @@ TEST_F(PreviewsOptimizationGuideTest,
   base::test::ScopedFeatureList scoped_list;
   scoped_list.InitAndDisableFeature(features::kResourceLoadingHints);
 
-  InitializeResourceLoadingHints();
+  InitializeFixedCountResourceLoadingHints();
 
   EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
       *CreateRequestWithURL(GURL("https://www.somedomain.org")),

components/previews/core/previews_experiments.cc

@@ -245,6 +245,12 @@ int ResourceLoadingHintsVersion() {
                                           kVersion, 0);
 }
 
+size_t GetMaxPageHintsInMemoryThreshhold() {
+  return GetFieldTrialParamByFeatureAsInt(features::kResourceLoadingHints,
+                                          "max_page_hints_in_memory_threshold",
+                                          500);
+}
+
 bool IsOptimizationHintsEnabled() {
   return base::FeatureList::IsEnabled(features::kOptimizationHints);
 }

components/previews/core/previews_experiments.h

@@ -128,6 +128,9 @@ int LitePageServerPreviewsVersion();
 int NoScriptPreviewsVersion();
 int ResourceLoadingHintsVersion();
 
+// The maximum number of page hints that should be loaded to memory.
+size_t GetMaxPageHintsInMemoryThreshhold();
+
 // Whether server optimization hints are enabled.
 bool IsOptimizationHintsEnabled();
 

tools/metrics/histograms/histograms.xml

@@ -84999,6 +84999,30 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="ResourceLoadingHints.PageHints.TotalReceived"
+    units="total page hints count">
+  <owner>tbansal@chromium.org</owner>
+  <summary>
+    The total count of page hints received by the browser from the component
+    updater. Only the page hints that have at least one resource loading hint
+    are counted. One sample is recorded every time optimization guide service
+    processes the optimization hints provided by the component updater. The
+    count of hints loaded to memory may be lower than this.
+  </summary>
+</histogram>
+
+<histogram name="ResourceLoadingHints.ResourceHints.TotalReceived"
+    units="total resource loading hints count">
+  <owner>tbansal@chromium.org</owner>
+  <summary>
+    The total count of resource loading hints across all page hints received by
+    the browser from the component updater. One sample is recorded every time
+    optimization guide service processes the optimization hints provided by the
+    component updater. The count of hints loaded to memory may be lower than
+    this.
+  </summary>
+</histogram>
+
 <histogram name="ResourceLoadingHints.ResourceLoadingBlocked"
     units="loading blocked">
   <owner>tbansal@chromium.org</owner>