Cap number of concurrent page navigation fetches that can happen

Bug: 1036490
Change-Id: Ib8bec196e208c0bbb86ec670ba48a7b2c859d565
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025914Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736199}

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -1051,8 +1051,14 @@ void OptimizationGuideHintsManager::MaybeFetchHintsForNavigation(
     return;
   }
 
-  // TODO(crbug/1036490): Add Finch feature to control number of concurrent
-  // fetches.
+  if (page_navigation_hints_fetchers_.size() >=
+      optimization_guide::features::MaxConcurrentPageNavigationFetches()) {
+    race_navigation_recorder.set_race_attempt_status(
+        optimization_guide::RaceNavigationFetchAttemptStatus::
+            kRaceNavigationFetchNotAttemptedTooManyConcurrentFetches);
+    return;
+  }
+
   DCHECK(hints_fetcher_factory_);
   page_navigation_hints_fetchers_[url] =
       hints_fetcher_factory_->BuildInstance();

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -1854,8 +1854,9 @@ class OptimizationGuideHintsManagerFetchingTest
     : public OptimizationGuideHintsManagerTest {
  public:
   OptimizationGuideHintsManagerFetchingTest() {
-    scoped_list_.InitAndEnableFeature(
-        optimization_guide::features::kRemoteOptimizationGuideFetching);
+    scoped_list_.InitAndEnableFeatureWithParameters(
+        optimization_guide::features::kRemoteOptimizationGuideFetching,
+        {{"max_concurrent_page_navigation_fetches", "2"}});
   }
 
  private:
@@ -2752,16 +2753,30 @@ TEST_F(OptimizationGuideHintsManagerFetchingTest,
   std::unique_ptr<content::MockNavigationHandle> navigation_handle2 =
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_without_hints());
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle3 =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          GURL("https://doesntmatter.com/"));
 
-  // Attempt to fetch a hint but initiate the next navigation right away to
+  // Attempt to fetch a hint but initiate the next navigations right away to
   // simulate being mid-fetch.
   base::HistogramTester histogram_tester;
   hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                base::DoNothing());
   hints_manager()->OnNavigationStartOrRedirect(navigation_handle2.get(),
                                                base::DoNothing());
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle3.get(),
+                                               base::DoNothing());
+  // The third one is over the max so should not be recorded.
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsManager.ConcurrentPageNavigationFetches", 2);
   histogram_tester.ExpectBucketCount(
       "OptimizationGuide.HintsManager.ConcurrentPageNavigationFetches", 1, 1);
   histogram_tester.ExpectBucketCount(
       "OptimizationGuide.HintsManager.ConcurrentPageNavigationFetches", 2, 1);
+  // We expect a sample to be recorded with too many concurrent fetches.
+  histogram_tester.ExpectBucketCount(
+      "OptimizationGuide.HintsManager.RaceNavigationFetchAttemptStatus",
+      optimization_guide::RaceNavigationFetchAttemptStatus::
+          kRaceNavigationFetchNotAttemptedTooManyConcurrentFetches,
+      1);
 }

components/optimization_guide/optimization_guide_enums.h

@@ -113,9 +113,12 @@ enum class RaceNavigationFetchAttemptStatus {
   kRaceNavigationFetchHostAndURL,
   // A race for the current navigation's URL is already in progress.
   kRaceNavigationFetchAlreadyInProgress,
+  // A race for the current navigation's URL was not attempted because there
+  // were too many concurrent page navigation fetches in flight.
+  kRaceNavigationFetchNotAttemptedTooManyConcurrentFetches,
 
   // Add new values above this line.
-  kMaxValue = kRaceNavigationFetchAlreadyInProgress,
+  kMaxValue = kRaceNavigationFetchNotAttemptedTooManyConcurrentFetches,
 };
 
 // The statuses for a prediction model in the prediction manager when requested

components/optimization_guide/optimization_guide_features.cc

@@ -184,7 +184,15 @@ GetMaxEffectiveConnectionTypeForNavigationHintsFetch() {
 }
 
 base::TimeDelta GetHintsFetchRefreshDuration() {
-  return base::TimeDelta::FromHours(72);
+  return base::TimeDelta::FromHours(GetFieldTrialParamByFeatureAsInt(
+      kRemoteOptimizationGuideFetching, "hints_fetch_refresh_duration_in_hours",
+      72));
+}
+
+size_t MaxConcurrentPageNavigationFetches() {
+  return GetFieldTrialParamByFeatureAsInt(
+      kRemoteOptimizationGuideFetching,
+      "max_concurrent_page_navigation_fetches", 20);
 }
 
 base::TimeDelta StoredHostModelFeaturesFreshnessDuration() {

components/optimization_guide/optimization_guide_features.h

@@ -92,6 +92,10 @@ GetMaxEffectiveConnectionTypeForNavigationHintsFetch();
 // GetHintsFetchRefreshDuration().
 base::TimeDelta GetHintsFetchRefreshDuration();
 
+// Returns the max number of concurrent fetches to the remote Optimization Guide
+// Service that should be allowed.
+size_t MaxConcurrentPageNavigationFetches();
+
 // Returns true if optimization target prediction is enabled.
 bool IsOptimizationTargetPredictionEnabled();
 

tools/metrics/histograms/enums.xml

@@ -47887,6 +47887,13 @@ Called by update_net_trust_anchors.py.-->
     A fetch race for the host and URL of the current navigation is already in
     progress.
   </int>
+  <int value="6"
+      label="Fetch race not attempted due to too many concurrent page
+             navigation fetches">
+    The fetch race was not attempted for the host and URL of the current
+    navigation since there are too many page navigation fetches currently in
+    flight.
+  </int>
 </enum>
 
 <enum name="OptOutBlacklistReason">