Log OptimizationGuideAutotuning event to feed autotuning infra loop

This is only logged if we have a navigation to tie the event to and if
there is actually a tuning version associated with the allowed
optimization.

Privacy doc: https://docs.google.com/document/d/1XnnJrIt6Uy81_H8FBclTRS5AI7XeZcHzZNshO-DMtrU/edit

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

chrome/browser/optimization_guide/android/optimization_guide_bridge.cc

@@ -124,6 +124,7 @@ void OptimizationGuideBridge::CanApplyOptimization(
   optimization_guide_keyed_service_->GetHintsManager()
       ->CanApplyOptimizationAsync(
           GURL(ConvertJavaStringToUTF8(env, url)),
+          /*navigation_id=*/base::nullopt,
           static_cast<optimization_guide::proto::OptimizationType>(
               optimization_type),
           base::BindOnce(&OnOptimizationGuideDecision,

chrome/browser/optimization_guide/android/optimization_guide_bridge_unittest.cc

@@ -26,6 +26,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::ByRef;
+using ::testing::Eq;
 using ::testing::Return;
 using ::testing::UnorderedElementsAre;
 
@@ -49,8 +50,9 @@ class MockOptimizationGuideHintsManager : public OptimizationGuideHintsManager {
                                       /*top_host_provider=*/nullptr,
                                       /*url_loader_factory=*/nullptr) {}
   ~MockOptimizationGuideHintsManager() override = default;
-  MOCK_METHOD3(CanApplyOptimizationAsync,
+  MOCK_METHOD4(CanApplyOptimizationAsync,
                void(const GURL&,
+                    const base::Optional<int64_t>&,
                     optimization_guide::proto::OptimizationType,
                     optimization_guide::OptimizationGuideDecisionCallback));
 };
@@ -168,10 +170,10 @@ TEST_F(OptimizationGuideBridgeTest, CanApplyOptimizationHasHint) {
   metadata.set_performance_hints_metadata(hints_metadata);
   EXPECT_CALL(
       *optimization_guide_hints_manager_,
-      CanApplyOptimizationAsync(GURL("https://example.com/"),
+      CanApplyOptimizationAsync(GURL("https://example.com/"), Eq(base::nullopt),
                                 optimization_guide::proto::PERFORMANCE_HINTS,
                                 base::test::IsNotNullCallback()))
-      .WillOnce(base::test::RunOnceCallback<2>(
+      .WillOnce(base::test::RunOnceCallback<3>(
           optimization_guide::OptimizationGuideDecision::kTrue,
           ByRef(metadata)));
 

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -51,6 +51,10 @@
 #include "components/prefs/scoped_user_pref_update.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
+#include "services/metrics/public/cpp/ukm_recorder.h"
+#include "services/metrics/public/cpp/ukm_source.h"
+#include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace {
@@ -124,7 +128,11 @@ bool IsOptimizationTypeAllowed(
     const google::protobuf::RepeatedPtrField<
         optimization_guide::proto::Optimization>& optimizations,
     optimization_guide::proto::OptimizationType optimization_type,
-    optimization_guide::OptimizationMetadata* optimization_metadata) {
+    optimization_guide::OptimizationMetadata* optimization_metadata,
+    base::Optional<int64_t>* tuning_version) {
+  DCHECK(tuning_version);
+  *tuning_version = base::nullopt;
+
   for (const auto& optimization : optimizations) {
     if (optimization_type != optimization.optimization_type())
       continue;
@@ -134,6 +142,9 @@ bool IsOptimizationTypeAllowed(
       continue;
     }
 
+    if (optimization.has_tuning_version())
+      *tuning_version = optimization.tuning_version();
+
     // We found an optimization that can be applied. Populate optimization
     // metadata if applicable and return.
     if (optimization_metadata) {
@@ -174,6 +185,25 @@ bool IsOptimizationTypeAllowed(
   return false;
 }
 
+// Logs an OptimizationAutotuning event for the navigation with |navigation_id|,
+// if |navigation_id| and |tuning_version| are non-null.
+void MaybeLogOptimizationAutotuningUKMForNavigation(
+    base::Optional<int64_t> navigation_id,
+    optimization_guide::proto::OptimizationType optimization_type,
+    base::Optional<int64_t> tuning_version) {
+  if (!navigation_id || !tuning_version) {
+    // Only log if we can correlate the tuning event with a navigation.
+    return;
+  }
+
+  ukm::SourceId ukm_source_id =
+      ukm::ConvertToSourceId(*navigation_id, ukm::SourceIdType::NAVIGATION_ID);
+  ukm::builders::OptimizationGuideAutotuning builder(ukm_source_id);
+  builder.SetOptimizationType(optimization_type)
+      .SetTuningVersion(*tuning_version)
+      .Record(ukm::UkmRecorder::Get());
+}
+
 // Util class for recording whether a hints fetch race against the current
 // navigation was attempted. The result is recorded when it goes out of scope
 // and its destructor is called.
@@ -981,13 +1011,15 @@ bool OptimizationGuideHintsManager::HasLoadedOptimizationBlocklist(
 
 void OptimizationGuideHintsManager::CanApplyOptimizationAsync(
     const GURL& navigation_url,
+    const base::Optional<int64_t>& navigation_id,
     optimization_guide::proto::OptimizationType optimization_type,
     optimization_guide::OptimizationGuideDecisionCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   optimization_guide::OptimizationMetadata metadata;
   optimization_guide::OptimizationTypeDecision type_decision =
-      CanApplyOptimization(navigation_url, optimization_type, &metadata);
+      CanApplyOptimization(navigation_url, navigation_id, optimization_type,
+                           &metadata);
   optimization_guide::OptimizationGuideDecision decision = optimization_guide::
       GetOptimizationGuideDecisionFromOptimizationTypeDecision(type_decision);
   // It's possible that a hint that applies to |navigation_url| will come in
@@ -1005,12 +1037,13 @@ void OptimizationGuideHintsManager::CanApplyOptimizationAsync(
   }
 
   registered_callbacks_[navigation_url][optimization_type].push_back(
-      std::move(callback));
+      std::make_pair(navigation_id, std::move(callback)));
 }
 
 optimization_guide::OptimizationTypeDecision
 OptimizationGuideHintsManager::CanApplyOptimization(
     const GURL& navigation_url,
+    const base::Optional<int64_t>& navigation_id,
     optimization_guide::proto::OptimizationType optimization_type,
     optimization_guide::OptimizationMetadata* optimization_metadata) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -1070,6 +1103,8 @@ OptimizationGuideHintsManager::CanApplyOptimization(
     }
   }
 
+  base::Optional<int64_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);
@@ -1078,7 +1113,9 @@ OptimizationGuideHintsManager::CanApplyOptimization(
     if (url_keyed_hint->page_hints_size() > 0 &&
         IsOptimizationTypeAllowed(
             url_keyed_hint->page_hints(0).whitelisted_optimizations(),
-            optimization_type, optimization_metadata)) {
+            optimization_type, optimization_metadata, &tuning_version)) {
+      MaybeLogOptimizationAutotuningUKMForNavigation(
+          navigation_id, optimization_type, tuning_version);
       return optimization_guide::OptimizationTypeDecision::kAllowedByHint;
     }
   }
@@ -1104,7 +1141,10 @@ OptimizationGuideHintsManager::CanApplyOptimization(
   }
 
   if (IsOptimizationTypeAllowed(loaded_hint->whitelisted_optimizations(),
-                                optimization_type, optimization_metadata)) {
+                                optimization_type, optimization_metadata,
+                                &tuning_version)) {
+    MaybeLogOptimizationAutotuningUKMForNavigation(
+        navigation_id, optimization_type, tuning_version);
     return optimization_guide::OptimizationTypeDecision::kAllowedByHint;
   }
 
@@ -1115,11 +1155,14 @@ OptimizationGuideHintsManager::CanApplyOptimization(
   if (!matched_page_hint)
     return optimization_guide::OptimizationTypeDecision::kNotAllowedByHint;
 
-  return IsOptimizationTypeAllowed(
-             matched_page_hint->whitelisted_optimizations(), optimization_type,
-             optimization_metadata)
-             ? optimization_guide::OptimizationTypeDecision::kAllowedByHint
-             : 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;
 }
 
 void OptimizationGuideHintsManager::PrepareToInvokeRegisteredCallbacks(
@@ -1149,20 +1192,22 @@ void OptimizationGuideHintsManager::OnReadyToInvokeRegisteredCallbacks(
        registered_callbacks_.at(navigation_url)) {
     optimization_guide::proto::OptimizationType opt_type =
         opt_type_and_callbacks.first;
-    optimization_guide::OptimizationMetadata metadata;
-    optimization_guide::OptimizationTypeDecision type_decision =
-        CanApplyOptimization(navigation_url, opt_type, &metadata);
-    optimization_guide::OptimizationGuideDecision decision =
-        optimization_guide::
-            GetOptimizationGuideDecisionFromOptimizationTypeDecision(
-                type_decision);
-
-    for (auto& callback : opt_type_and_callbacks.second) {
+
+    for (auto& navigation_id_and_callback : opt_type_and_callbacks.second) {
+      base::Optional<int64_t> navigation_id = navigation_id_and_callback.first;
+      optimization_guide::OptimizationMetadata metadata;
+      optimization_guide::OptimizationTypeDecision type_decision =
+          CanApplyOptimization(navigation_url, navigation_id, opt_type,
+                               &metadata);
+      optimization_guide::OptimizationGuideDecision decision =
+          optimization_guide::
+              GetOptimizationGuideDecisionFromOptimizationTypeDecision(
+                  type_decision);
       base::UmaHistogramEnumeration(
           "OptimizationGuide.ApplyDecisionAsync." +
               optimization_guide::GetStringNameForOptimizationType(opt_type),
           type_decision);
-      std::move(callback).Run(decision, metadata);
+      std::move(navigation_id_and_callback.second).Run(decision, metadata);
     }
   }
   registered_callbacks_.erase(navigation_url);

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -116,6 +116,7 @@ class OptimizationGuideHintsManager
   // |optimization_metadata| will be populated, if applicable.
   optimization_guide::OptimizationTypeDecision CanApplyOptimization(
       const GURL& navigation_url,
+      const base::Optional<int64_t>& navigation_id,
       optimization_guide::proto::OptimizationType optimization_type,
       optimization_guide::OptimizationMetadata* optimization_metadata);
 
@@ -124,6 +125,7 @@ class OptimizationGuideHintsManager
   // |this| to make the decision. Virtual for testing.
   virtual void CanApplyOptimizationAsync(
       const GURL& navigation_url,
+      const base::Optional<int64_t>& navigation_id,
       optimization_guide::proto::OptimizationType optimization_type,
       optimization_guide::OptimizationGuideDecisionCallback callback);
 
@@ -406,12 +408,15 @@ class OptimizationGuideHintsManager
                  std::unique_ptr<optimization_guide::OptimizationFilter>>
       blocklist_optimization_filters_ GUARDED_BY(optimization_filters_lock_);
 
-  // A map from URL to a map of callbacks keyed by their optimization type.
+  // A map from URL to a map of callbacks (along with the navigation IDs that
+  // they were called for) keyed by their optimization type.
   base::flat_map<
       GURL,
       base::flat_map<
           optimization_guide::proto::OptimizationType,
-          std::vector<optimization_guide::OptimizationGuideDecisionCallback>>>
+          std::vector<std::pair<
+              base::Optional<int64_t>,
+              optimization_guide::OptimizationGuideDecisionCallback>>>>
       registered_callbacks_;
 
   // Background thread where hints processing should be performed.

chrome/browser/optimization_guide/optimization_guide_keyed_service.cc

@@ -238,7 +238,8 @@ OptimizationGuideKeyedService::CanApplyOptimization(
   }
 
   optimization_guide::OptimizationTypeDecision optimization_type_decision =
-      hints_manager_->CanApplyOptimization(url, optimization_type,
+      hints_manager_->CanApplyOptimization(url, /*navigation_id=*/base::nullopt,
+                                           optimization_type,
                                            optimization_metadata);
   base::UmaHistogramEnumeration(
       "OptimizationGuide.ApplyDecision." +
@@ -264,7 +265,8 @@ void OptimizationGuideKeyedService::CanApplyOptimizationAsync(
   }
 
   hints_manager_->CanApplyOptimizationAsync(
-      navigation_handle->GetURL(), optimization_type, std::move(callback));
+      navigation_handle->GetURL(), navigation_handle->GetNavigationId(),
+      optimization_type, std::move(callback));
 }
 
 void OptimizationGuideKeyedService::AddHintForTesting(

components/optimization_guide/optimization_guide_decider.h

@@ -85,6 +85,9 @@ class OptimizationGuideDecider {
 
   // Returns whether |optimization_type| can be applied for |url|. This should
   // only be called for main frame navigations or future main frame navigations.
+  //
+  // Note: DO NOT USE this method if you intend to opt into the Optimization
+  // Guide's autotuning framework at any point.
   virtual OptimizationGuideDecision CanApplyOptimization(
       const GURL& url,
       proto::OptimizationType optimization_type,

components/optimization_guide/proto/hints.proto

@@ -192,6 +192,10 @@ message Optimization {
   // same name, the optimization will be disabled and skipped for that client.
   // An empty name is not experimental.
   optional string excluded_experiment_name = 5;
+  // The version of the tuning group for the optimization type.
+  //
+  // This will only be populated if this optimization is being autotuned.
+  optional uint64 tuning_version = 6;
   // The metadata associated with the optimization type.
   //
   // It is expected that the client and server have agreed upon the appropriate

tools/metrics/ukm/ukm.xml

@@ -7906,6 +7906,25 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="OptimizationGuideAutotuning">
+  <owner>sophiechang@chromium.org</owner>
+  <owner>mcrouse@chromium.org</owner>
+  <summary>
+    Metrics associated with an autotuning event initiated by the Optimization
+    Guide.
+  </summary>
+  <metric name="OptimizationType">
+    <summary>
+      The |optimization_guide::proto::OptimizationType| that is being tuned.
+    </summary>
+  </metric>
+  <metric name="TuningVersion">
+    <summary>
+      The version of the tuning group for the optimization type that was served.
+    </summary>
+  </metric>
+</event>
+
 <event name="PageDomainInfo">
   <owner>uthakore@chromium.org</owner>
   <owner>invernizzi@chromium.org</owner>