Add support for sentinel tuning version that is used for "control"...

Add support for sentinel tuning version that is used for "control" optimizations where the type should be disabled for the navigation

Bug: 1113287
Change-Id: Id82122c8f2918300bedc151b93b58d5aa9abb1ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357558
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798802}

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -129,7 +129,7 @@ bool IsOptimizationTypeAllowed(
         optimization_guide::proto::Optimization>& optimizations,
     optimization_guide::proto::OptimizationType optimization_type,
     optimization_guide::OptimizationMetadata* optimization_metadata,
-    base::Optional<int64_t>* tuning_version) {
+    base::Optional<uint64_t>* tuning_version) {
   DCHECK(tuning_version);
   *tuning_version = base::nullopt;
 
@@ -142,9 +142,17 @@ bool IsOptimizationTypeAllowed(
       continue;
     }
 
-    if (optimization.has_tuning_version())
+    if (optimization.has_tuning_version()) {
       *tuning_version = optimization.tuning_version();
 
+      if (optimization.tuning_version() == UINT64_MAX) {
+        // UINT64_MAX is the sentinel value indicating that the optimization
+        // should not be served and was only added to the list for metrics
+        // purposes.
+        return false;
+      }
+    }
+
     // We found an optimization that can be applied. Populate optimization
     // metadata if applicable and return.
     if (optimization_metadata) {
@@ -1098,20 +1106,25 @@ OptimizationGuideHintsManager::CanApplyOptimization(
     }
   }
 
-  base::Optional<int64_t> tuning_version;
+  base::Optional<uint64_t> tuning_version;
 
   // First, check if the optimization type is whitelisted by a URL-keyed hint.
   const optimization_guide::proto::Hint* url_keyed_hint =
       hint_cache_->GetURLKeyedHint(navigation_url);
   if (url_keyed_hint) {
     DCHECK_EQ(url_keyed_hint->page_hints_size(), 1);
-    if (url_keyed_hint->page_hints_size() > 0 &&
-        IsOptimizationTypeAllowed(
-            url_keyed_hint->page_hints(0).whitelisted_optimizations(),
-            optimization_type, optimization_metadata, &tuning_version)) {
-      MaybeLogOptimizationAutotuningUKMForNavigation(
-          navigation_id, optimization_type, tuning_version);
-      return optimization_guide::OptimizationTypeDecision::kAllowedByHint;
+    if (url_keyed_hint->page_hints_size() > 0) {
+      bool is_allowed = IsOptimizationTypeAllowed(
+          url_keyed_hint->page_hints(0).whitelisted_optimizations(),
+          optimization_type, optimization_metadata, &tuning_version);
+      if (is_allowed || tuning_version) {
+        MaybeLogOptimizationAutotuningUKMForNavigation(
+            navigation_id, optimization_type, tuning_version);
+        return is_allowed ? optimization_guide::OptimizationTypeDecision::
+                                kAllowedByHint
+                          : optimization_guide::OptimizationTypeDecision::
+                                kNotAllowedByHint;
+      }
     }
   }
 
@@ -1135,12 +1148,16 @@ OptimizationGuideHintsManager::CanApplyOptimization(
     return optimization_guide::OptimizationTypeDecision::kNoHintAvailable;
   }
 
-  if (IsOptimizationTypeAllowed(loaded_hint->whitelisted_optimizations(),
-                                optimization_type, optimization_metadata,
-                                &tuning_version)) {
+  bool is_allowed = IsOptimizationTypeAllowed(
+      loaded_hint->whitelisted_optimizations(), optimization_type,
+      optimization_metadata, &tuning_version);
+  if (is_allowed || tuning_version) {
     MaybeLogOptimizationAutotuningUKMForNavigation(
         navigation_id, optimization_type, tuning_version);
-    return optimization_guide::OptimizationTypeDecision::kAllowedByHint;
+    return is_allowed
+               ? optimization_guide::OptimizationTypeDecision::kAllowedByHint
+               : optimization_guide::OptimizationTypeDecision::
+                     kNotAllowedByHint;
   }
 
   const optimization_guide::proto::PageHint* matched_page_hint =
@@ -1150,14 +1167,14 @@ OptimizationGuideHintsManager::CanApplyOptimization(
   if (!matched_page_hint)
     return optimization_guide::OptimizationTypeDecision::kNotAllowedByHint;
 
-  if (IsOptimizationTypeAllowed(matched_page_hint->whitelisted_optimizations(),
-                                optimization_type, optimization_metadata,
-                                &tuning_version)) {
-    MaybeLogOptimizationAutotuningUKMForNavigation(
-        navigation_id, optimization_type, tuning_version);
-    return optimization_guide::OptimizationTypeDecision::kAllowedByHint;
-  }
-  return optimization_guide::OptimizationTypeDecision::kNotAllowedByHint;
+  is_allowed = IsOptimizationTypeAllowed(
+      matched_page_hint->whitelisted_optimizations(), optimization_type,
+      optimization_metadata, &tuning_version);
+  MaybeLogOptimizationAutotuningUKMForNavigation(
+      navigation_id, optimization_type, tuning_version);
+  return is_allowed
+             ? optimization_guide::OptimizationTypeDecision::kAllowedByHint
+             : optimization_guide::OptimizationTypeDecision::kNotAllowedByHint;
 }
 
 void OptimizationGuideHintsManager::PrepareToInvokeRegisteredCallbacks(

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -1333,6 +1333,154 @@ TEST_F(OptimizationGuideHintsManagerTest,
       123456);
 }
 
+TEST_F(
+    OptimizationGuideHintsManagerTest,
+    CanApplyOptimizationOptimizationTypeHostHasSentinelTuningVersionShouldLogUKM) {
+  optimization_guide::proto::Configuration config;
+  optimization_guide::proto::Hint* hint1 = config.add_hints();
+  hint1->set_key("somedomain.org");
+  hint1->set_key_representation(optimization_guide::proto::HOST);
+  hint1->set_version("someversion");
+  optimization_guide::proto::Optimization* opt1 =
+      hint1->add_whitelisted_optimizations();
+  opt1->set_optimization_type(optimization_guide::proto::RESOURCE_LOADING);
+  opt1->set_tuning_version(UINT64_MAX);
+  ProcessHints(config, "1.0.0.0");
+
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::RESOURCE_LOADING});
+
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_with_hints());
+  base::RunLoop run_loop;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+
+  ukm::TestAutoSetUkmRecorder ukm_recorder;
+
+  optimization_guide::OptimizationMetadata optimization_metadata;
+  optimization_guide::OptimizationTypeDecision optimization_type_decision =
+      hints_manager()->CanApplyOptimization(
+          navigation_handle->GetURL(), navigation_handle->GetNavigationId(),
+          optimization_guide::proto::RESOURCE_LOADING, &optimization_metadata);
+  EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kNotAllowedByHint,
+            optimization_type_decision);
+
+  // Make sure autotuning UKM is recorded.
+  auto entries = ukm_recorder.GetEntriesByName(
+      ukm::builders::OptimizationGuideAutotuning::kEntryName);
+  EXPECT_EQ(1u, entries.size());
+  auto* entry = entries[0];
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kOptimizationTypeName,
+      static_cast<int64_t>(optimization_guide::proto::RESOURCE_LOADING));
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kTuningVersionName,
+      UINT64_MAX);
+}
+
+TEST_F(
+    OptimizationGuideHintsManagerTest,
+    CanApplyOptimizationOptimizationTypePatternHasSentinelTuningVersionShouldLogUKM) {
+  optimization_guide::proto::Configuration config;
+  optimization_guide::proto::Hint* hint1 = config.add_hints();
+  hint1->set_key("somedomain.org");
+  hint1->set_key_representation(optimization_guide::proto::HOST);
+  hint1->set_version("someversion");
+  optimization_guide::proto::PageHint* ph1 = hint1->add_page_hints();
+  ph1->set_page_pattern("*");
+  optimization_guide::proto::Optimization* opt1 =
+      ph1->add_whitelisted_optimizations();
+  opt1->set_optimization_type(optimization_guide::proto::RESOURCE_LOADING);
+  opt1->set_tuning_version(UINT64_MAX);
+  ProcessHints(config, "1.0.0.0");
+
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::RESOURCE_LOADING});
+
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_with_hints());
+  base::RunLoop run_loop;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+
+  ukm::TestAutoSetUkmRecorder ukm_recorder;
+
+  optimization_guide::OptimizationMetadata optimization_metadata;
+  optimization_guide::OptimizationTypeDecision optimization_type_decision =
+      hints_manager()->CanApplyOptimization(
+          navigation_handle->GetURL(), navigation_handle->GetNavigationId(),
+          optimization_guide::proto::RESOURCE_LOADING, &optimization_metadata);
+  EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kNotAllowedByHint,
+            optimization_type_decision);
+
+  // Make sure autotuning UKM is recorded.
+  auto entries = ukm_recorder.GetEntriesByName(
+      ukm::builders::OptimizationGuideAutotuning::kEntryName);
+  EXPECT_EQ(1u, entries.size());
+  auto* entry = entries[0];
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kOptimizationTypeName,
+      static_cast<int64_t>(optimization_guide::proto::RESOURCE_LOADING));
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kTuningVersionName,
+      UINT64_MAX);
+}
+
+TEST_F(
+    OptimizationGuideHintsManagerTest,
+    CanApplyOptimizationURLKeyedOptimizationTypeHasSentinelTuningVersionShouldLogUKM) {
+  optimization_guide::proto::Configuration config;
+  optimization_guide::proto::Hint* hint1 = config.add_hints();
+  hint1->set_key(url_with_hints().spec());
+  hint1->set_key_representation(optimization_guide::proto::FULL_URL);
+  hint1->set_version("someversion");
+  optimization_guide::proto::PageHint* ph1 = hint1->add_page_hints();
+  ph1->set_page_pattern(url_with_hints().spec());
+  optimization_guide::proto::Optimization* opt1 =
+      ph1->add_whitelisted_optimizations();
+  opt1->set_optimization_type(optimization_guide::proto::RESOURCE_LOADING);
+  opt1->set_tuning_version(UINT64_MAX);
+  ProcessHints(config, "1.0.0.0");
+
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::RESOURCE_LOADING});
+
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_with_hints());
+  base::RunLoop run_loop;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+
+  ukm::TestAutoSetUkmRecorder ukm_recorder;
+
+  optimization_guide::OptimizationMetadata optimization_metadata;
+  optimization_guide::OptimizationTypeDecision optimization_type_decision =
+      hints_manager()->CanApplyOptimization(
+          navigation_handle->GetURL(), navigation_handle->GetNavigationId(),
+          optimization_guide::proto::RESOURCE_LOADING, &optimization_metadata);
+  EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kNotAllowedByHint,
+            optimization_type_decision);
+
+  // Make sure autotuning UKM is recorded.
+  auto entries = ukm_recorder.GetEntriesByName(
+      ukm::builders::OptimizationGuideAutotuning::kEntryName);
+  EXPECT_EQ(1u, entries.size());
+  auto* entry = entries[0];
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kOptimizationTypeName,
+      static_cast<int64_t>(optimization_guide::proto::RESOURCE_LOADING));
+  ukm_recorder.ExpectEntryMetric(
+      entry, ukm::builders::OptimizationGuideAutotuning::kTuningVersionName,
+      UINT64_MAX);
+}
+
 TEST_F(OptimizationGuideHintsManagerTest,
        CanApplyOptimizationOptimizationTypeHasTuningVersionButNoNavigation) {
   optimization_guide::proto::Configuration config;

components/optimization_guide/proto/hints.proto

@@ -194,6 +194,9 @@ message Optimization {
   // The version of the tuning group for the optimization type.
   //
   // This will only be populated if this optimization is being autotuned.
+  //
+  // If this is set to UINT64_MAX, this optimization should be considered as
+  // not allowed and was only included for metrics purposes.
   optional uint64 tuning_version = 6;
   // The metadata associated with the optimization type.
   //