Adding a new optimization type, LINK_PERFORMANCE.

Link hints only vary per-host, so splitting link hints into their own
optimization type will allow us to send them at host-granularity rather
than copying them for each pattern on a host. This will significantly
reduce hint size.

This change is behind a flag so we can prepare the server-side changes
in the meantime.

One-pager: https://docs.google.com/document/d/1ZUI6V7jxT0WgTTC8mTUcM91XZW8Yv6skqHFh0NuF4gk/edit

BUG=1139455

Change-Id: I24f1519ae3f1719d445c5ef4a67e571602d2306b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533800Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Megan Jablonski <megjablon@chromium.org>
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Commit-Queue: Josh Simmons <jds@google.com>
Cr-Commit-Position: refs/heads/master@{#826954}

chrome/browser/performance_hints/performance_hints_features.cc

@@ -22,6 +22,8 @@ constexpr base::FeatureParam<std::string> kRewriteConfig{
 
 constexpr base::FeatureParam<bool> kUseFastHostHints{
     &kPerformanceHintsObserver, "use_fast_host_hints", true};
+constexpr base::FeatureParam<bool> kUseLinkPerformanceHints{
+    &kPerformanceHintsObserver, "use_link_performance_hints", false};
 
 const base::Feature kContextMenuPerformanceInfo{
     "ContextMenuPerformanceInfo", base::FEATURE_DISABLED_BY_DEFAULT};
@@ -53,6 +55,10 @@ bool AreFastHostHintsEnabled() {
   return kUseFastHostHints.Get();
 }
 
+bool AreLinkPerformanceHintsEnabled() {
+  return kUseLinkPerformanceHints.Get();
+}
+
 bool IsContextMenuPerformanceInfoEnabled() {
   return base::FeatureList::IsEnabled(kContextMenuPerformanceInfo) ||
          base::FeatureList::IsEnabled(

chrome/browser/performance_hints/performance_hints_features.h

@@ -45,6 +45,9 @@ std::string GetRewriteConfigString();
 // Returns true if FAST_HOST_HINTS should be checked if available.
 bool AreFastHostHintsEnabled();
 
+// Returns true if LINK_PERFORMANCE hints should be requested and used.
+bool AreLinkPerformanceHintsEnabled();
+
 // Returns true if performance info should be shown in the context menu.
 bool IsContextMenuPerformanceInfoEnabled();
 

chrome/browser/performance_hints/performance_hints_observer.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/performance_hints/performance_hints_features.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/optimization_guide/optimization_guide_decider.h"
+#include "components/optimization_guide/proto/performance_hints_metadata.pb.h"
 #include "components/optimization_guide/url_pattern_with_wildcards.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
@@ -28,6 +29,7 @@
 
 using optimization_guide::OptimizationGuideDecision;
 using optimization_guide::URLPatternWithWildcards;
+using optimization_guide::proto::LinkPerformanceMetadata;
 using optimization_guide::proto::PerformanceClass;
 using optimization_guide::proto::PerformanceHint;
 
@@ -110,6 +112,9 @@ PerformanceHintsObserver::PerformanceHintsObserver(
   if (features::AreFastHostHintsEnabled()) {
     opts.push_back(optimization_guide::proto::FAST_HOST_HINTS);
   }
+  if (features::AreLinkPerformanceHintsEnabled()) {
+    opts.push_back(optimization_guide::proto::LINK_PERFORMANCE);
+  }
   if (optimization_guide_decider_) {
     optimization_guide_decider_->RegisterOptimizationTypes(opts);
   }
@@ -140,7 +145,8 @@ PerformanceClass PerformanceHintsObserver::PerformanceClassForURL(
 
   PerformanceHintsObserver* performance_hints_observer =
       PerformanceHintsObserver::FromWebContents(web_contents);
-  if (performance_hints_observer == nullptr) {
+  if (performance_hints_observer == nullptr ||
+      !performance_hints_observer->DoesPageSupportHints()) {
     return PerformanceClass::PERFORMANCE_UNKNOWN;
   }
 
@@ -203,7 +209,7 @@ PerformanceHintsObserver::HintForURLResult::~HintForURLResult() = default;
 
 PerformanceHintsObserver::HintForURLResult PerformanceHintsObserver::HintForURL(
     const GURL& url,
-    bool record_metrics) const {
+    bool record_metrics) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   HintForURLResult result;
@@ -282,28 +288,82 @@ PerformanceHintsObserver::HintForURLResult PerformanceHintsObserver::HintForURL(
   return result;
 }
 
+bool PerformanceHintsObserver::DoesPageSupportHints() {
+  // page_url_ is not set for error pages.
+  return page_url_.has_value() && page_url_->is_valid() &&
+         page_url_->SchemeIsHTTPOrHTTPS();
+}
+
+void PerformanceHintsObserver::PopulateLinkHints() {
+  DCHECK(page_url_);
+  if (!page_url_)
+    return;
+
+  const google::protobuf::RepeatedPtrField<PerformanceHint>* link_hints =
+      nullptr;
+  // Metadata variables are scoped here to share the same scope as link_hints.
+  optimization_guide::OptimizationMetadata metadata;
+  base::Optional<LinkPerformanceMetadata> link_metadata;
+  if (features::AreLinkPerformanceHintsEnabled()) {
+    link_hints_decision_ = optimization_guide_decider_->CanApplyOptimization(
+        page_url_.value(), optimization_guide::proto::LINK_PERFORMANCE,
+        &metadata);
+    link_metadata = metadata.ParsedMetadata<LinkPerformanceMetadata>();
+    if (!link_metadata)
+      return;
+    link_hints = &link_metadata->link_hints();
+  } else {
+    link_hints_decision_ = optimization_guide_decider_->CanApplyOptimization(
+        page_url_.value(), optimization_guide::proto::PERFORMANCE_HINTS,
+        &metadata);
+    if (!metadata.performance_hints_metadata())
+      return;
+    link_hints =
+        &metadata.performance_hints_metadata().value().performance_hints();
+  }
+
+  DCHECK(link_hints);
+  if (link_hints_decision_ == OptimizationGuideDecision::kTrue) {
+    for (const PerformanceHint& link_hint : *link_hints) {
+      link_hints_.emplace_back(
+          URLPatternWithWildcards(link_hint.wildcard_pattern()), link_hint);
+    }
+  }
+}
+
 std::tuple<PerformanceHintsObserver::SourceLookupStatus,
            base::Optional<optimization_guide::proto::PerformanceHint>>
-PerformanceHintsObserver::LinkHintForURL(const GURL& url) const {
-  if (!hint_processed_) {
-    return {SourceLookupStatus::kNotReady, base::nullopt};
-  } else {
-    // Link hints only contain scheme, host, and path, so remove other
-    // components.
-    url::Replacements<char> replacements;
-    replacements.ClearUsername();
-    replacements.ClearPassword();
-    replacements.ClearQuery();
-    replacements.ClearPort();
-    replacements.ClearRef();
-    GURL scheme_host_path = url.ReplaceComponents(replacements);
-
-    for (const auto& pattern_hint : hints_) {
-      if (pattern_hint.first.Matches(scheme_host_path.spec())) {
-        return {SourceLookupStatus::kHintFound, pattern_hint.second};
+PerformanceHintsObserver::LinkHintForURL(const GURL& url) {
+  if (!optimization_guide_decider_) {
+    return {SourceLookupStatus::kNoMatch, base::nullopt};
+  }
+
+  if (link_hints_decision_ == OptimizationGuideDecision::kUnknown) {
+    PopulateLinkHints();
+  }
+  switch (link_hints_decision_) {
+    case OptimizationGuideDecision::kUnknown:
+      return {SourceLookupStatus::kNotReady, base::nullopt};
+    case OptimizationGuideDecision::kFalse:
+      return {SourceLookupStatus::kNoMatch, base::nullopt};
+    case OptimizationGuideDecision::kTrue: {
+      // Link hints only contain scheme, host, and path, so remove other
+      // components.
+      url::Replacements<char> replacements;
+      replacements.ClearUsername();
+      replacements.ClearPassword();
+      replacements.ClearQuery();
+      replacements.ClearPort();
+      replacements.ClearRef();
+      GURL scheme_host_path = url.ReplaceComponents(replacements);
+
+      for (const auto& pattern_hint : link_hints_) {
+        if (pattern_hint.first.Matches(scheme_host_path.spec())) {
+          return {SourceLookupStatus::kHintFound, pattern_hint.second};
+        }
       }
+      return {SourceLookupStatus::kNoMatch, base::nullopt};
     }
-    return {SourceLookupStatus::kNoMatch, base::nullopt};
   }
 }
 
@@ -367,10 +427,10 @@ void PerformanceHintsObserver::DidFinishNavigation(
     return;
   }
 
-  // We've navigated to a new page, so clear out any hints from the previous
-  // page.
-  hints_.clear();
-  hint_processed_ = false;
+  // We've navigated to a new page, so clear out any existing cached hints.
+  link_hints_.clear();
+  link_hints_decision_ = OptimizationGuideDecision::kUnknown;
+  page_url_.reset();
 
   if (!optimization_guide_decider_) {
     return;
@@ -380,38 +440,7 @@ void PerformanceHintsObserver::DidFinishNavigation(
     return;
   }
 
-  // TODO(jds): Because calls to HintForURL are not asynchronous, we don't
-  // actually need to use the Async version and can instead call
-  // CanApplyOptimization directly from HintForURL to remove this complexity.
-  optimization_guide_decider_->CanApplyOptimizationAsync(
-      navigation_handle, optimization_guide::proto::PERFORMANCE_HINTS,
-      base::BindOnce(&PerformanceHintsObserver::ProcessPerformanceHint,
-                     weak_factory_.GetWeakPtr()));
-}
-
-void PerformanceHintsObserver::ProcessPerformanceHint(
-    OptimizationGuideDecision decision,
-    const optimization_guide::OptimizationMetadata& optimization_metadata) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  hint_processed_ = true;
-
-  if (decision != OptimizationGuideDecision::kTrue) {
-    // Apply results are counted under
-    // OptimizationGuide.ApplyDecision.PerformanceHints.
-    return;
-  }
-
-  if (!optimization_metadata.performance_hints_metadata())
-    return;
-
-  const optimization_guide::proto::PerformanceHintsMetadata
-      performance_hints_metadata =
-          optimization_metadata.performance_hints_metadata().value();
-  for (const PerformanceHint& hint :
-       performance_hints_metadata.performance_hints()) {
-    hints_.emplace_back(URLPatternWithWildcards(hint.wildcard_pattern()), hint);
-  }
+  page_url_ = navigation_handle->GetURL();
 }
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(PerformanceHintsObserver)

chrome/browser/performance_hints/performance_hints_observer.h

@@ -73,12 +73,13 @@ class PerformanceHintsObserver
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
 
-  // This callback populates |hints_| with performance information for links on
-  // the current page and is called by |optimization_guide_decider_| when a
-  // definite decision has been reached.
-  void ProcessPerformanceHint(
-      optimization_guide::OptimizationGuideDecision decision,
-      const optimization_guide::OptimizationMetadata& optimization_metadata);
+  // Returns true if the current page supports hints (not an error page,
+  // must be HTTP/HTTPS, etc).
+  bool DoesPageSupportHints();
+
+  // Populates |link_hints_| with performance information for links on the
+  // current page.
+  void PopulateLinkHints();
 
   // SourceLookupStatus represents the result of a querying a single source
   // (page hints or link hints) for performance information. Tracking this
@@ -106,7 +107,7 @@ class PerformanceHintsObserver
   // current page.
   std::tuple<SourceLookupStatus,
              base::Optional<optimization_guide::proto::PerformanceHint>>
-  LinkHintForURL(const GURL& url) const;
+  LinkHintForURL(const GURL& url);
 
   // Attempts to retrieve a PerformanceHint for |url| from the OptimizationGuide
   // metadata for that URL.
@@ -158,7 +159,7 @@ class PerformanceHintsObserver
   // Fetches a PerformanceHint for the given |url|.
   //
   // See HintForURLResult for details on the return value.
-  HintForURLResult HintForURL(const GURL& url, bool record_metrics) const;
+  HintForURLResult HintForURL(const GURL& url, bool record_metrics);
 
   // If kPerformanceHintsHandleRewrites is enabled, URLs that match one of the
   // configured rewrite patterns will have the inner URL extracted and used for
@@ -172,13 +173,18 @@ class PerformanceHintsObserver
   optimization_guide::OptimizationGuideDecider* optimization_guide_decider_ =
       nullptr;
 
-  // URLs that match |first| should use the Performance hint in |second|.
+  // The URL of the main frame of the associated WebContents. This is not set if
+  // the current page is an error page.
+  base::Optional<GURL> page_url_;
+
+  // Link URLs that match |first| should use the Performance hint in |second|.
   std::vector<std::pair<optimization_guide::URLPatternWithWildcards,
                         optimization_guide::proto::PerformanceHint>>
-      hints_;
+      link_hints_;
 
-  // True if the ProcessPerformanceHint callback has been run.
-  bool hint_processed_ = false;
+  // The fetch status for link hints.
+  optimization_guide::OptimizationGuideDecision link_hints_decision_ =
+      optimization_guide::OptimizationGuideDecision::kUnknown;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

components/optimization_guide/hints_processing_util.cc

@@ -51,6 +51,8 @@ std::string GetStringNameForOptimizationType(
       return "DelayCompetingLowPriorityRequests";
     case proto::OptimizationType::LITE_VIDEO:
       return "LiteVideo";
+    case proto::OptimizationType::LINK_PERFORMANCE:
+      return "LinkPerformance";
   }
   NOTREACHED();
   return std::string();

components/optimization_guide/optimization_metadata.h

@@ -57,7 +57,9 @@ class OptimizationMetadata {
       return metadata;
     return base::nullopt;
   }
-  base::Optional<proto::Any> any_metadata() const { return any_metadata_; }
+  const base::Optional<proto::Any>& any_metadata() const {
+    return any_metadata_;
+  }
   void set_any_metadata(const proto::Any& any_metadata) {
     any_metadata_ = any_metadata;
   }
@@ -65,15 +67,15 @@ class OptimizationMetadata {
   // used for testing purposes.
   void SetAnyMetadataForTesting(const google::protobuf::MessageLite& metadata);
 
-  base::Optional<proto::PreviewsMetadata> previews_metadata() const {
+  const base::Optional<proto::PreviewsMetadata>& previews_metadata() const {
     return previews_metadata_;
   }
   void set_previews_metadata(const proto::PreviewsMetadata& previews_metadata) {
     previews_metadata_ = previews_metadata;
   }
 
-  base::Optional<proto::PerformanceHintsMetadata> performance_hints_metadata()
-      const {
+  const base::Optional<proto::PerformanceHintsMetadata>&
+  performance_hints_metadata() const {
     return performance_hints_metadata_;
   }
   void set_performance_hints_metadata(
@@ -81,7 +83,8 @@ class OptimizationMetadata {
     performance_hints_metadata_ = performance_hints_metadata;
   }
 
-  base::Optional<proto::PublicImageMetadata> public_image_metadata() const {
+  const base::Optional<proto::PublicImageMetadata>& public_image_metadata()
+      const {
     return public_image_metadata_;
   }
   void set_public_image_metadata(
@@ -89,8 +92,8 @@ class OptimizationMetadata {
     public_image_metadata_ = public_image_metadata;
   }
 
-  base::Optional<proto::LoadingPredictorMetadata> loading_predictor_metadata()
-      const {
+  const base::Optional<proto::LoadingPredictorMetadata>&
+  loading_predictor_metadata() const {
     return loading_predictor_metadata_;
   }
   void set_loading_predictor_metadata(

components/optimization_guide/proto/hints.proto

@@ -131,6 +131,9 @@ enum OptimizationType {
   // This optimization provides information about how to throttle meda requests
   // to reduce the bitrate of adaptively streamed media.
   LITE_VIDEO = 13;
+  // This optimization is used to provide aggregated performance information
+  // about pages linked to from the current page.
+  LINK_PERFORMANCE = 14;
 }
 
 // Presents semantics for how page load URLs should be matched.
@@ -167,6 +170,8 @@ message Optimization {
   //
   // It is expected that the client and server have agreed upon the appropriate
   // metadata type for the optimization type.
+  //
+  // New clients should utilize any_metadata rather than adding another field.
   oneof metadata {
     PreviewsMetadata previews_metadata = 10;
     PerformanceHintsMetadata performance_hints_metadata = 11;

components/optimization_guide/proto/performance_hints_metadata.proto

@@ -42,6 +42,9 @@ message PerformanceHintsMetadata {
   // An ordered set of performance hints.
   //
   // Only the first matching hint should be applied to a given URL.
+  //
+  // These will be omitted if LINK_PERFORMANCE hints are requested. Instead,
+  // link hints will be included in LinkPerformanceMetadata, below.
   repeated PerformanceHint performance_hints = 1;
 
   // The PerformanceHint for the page pattern that matched the parent
@@ -51,3 +54,13 @@ message PerformanceHintsMetadata {
   // the pattern matching is done by OptimizationGuide.
   optional PerformanceHint page_hint = 2;
 }
+
+// Optimization metadata associated with Link Performance Hints.
+//
+// This is only populated for the LINK_PERFORMANCE optimization type.
+message LinkPerformanceMetadata {
+  // An ordered set of hints for the most popular links for this key.
+  //
+  // Only the first matching hint should be applied to a given link URL.
+  repeated PerformanceHint link_hints = 1;
+}

tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml

@@ -12036,7 +12036,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
              requests"/>
   <suffix name="FastHostHints"
       label="Provides information about hosts that historically provide a
-             fast and responsive user experience."/>
+             fast and responsive user experience"/>
+  <suffix name="LinkPerformance"
+      label="Provides aggregated performance information for links on the
+             page"/>
   <suffix name="LitePage"
       label="HTTP server preview (served from the Data Reduction Proxy)"/>
   <suffix name="LitePageRedirect"