Migrate previews over to the async model evaluation API

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

components/previews/content/previews_decider_impl.cc

@@ -160,6 +160,11 @@ bool PreviewsDeciderImpl::ShouldAllowPreviewAtNavigationStart(
       previews_data, navigation_handle, is_reload, type, &passed_reasons);
   LogPreviewDecisionMade(eligibility, url, clock_->Now(), type,
                          std::move(passed_reasons), previews_data);
+  if (previews_opt_guide_ &&
+      eligibility == PreviewsEligibilityReason::ALLOWED) {
+    // Kick off model prediction.
+    previews_opt_guide_->StartCheckingIfShouldShowPreview(navigation_handle);
+  }
   return eligibility == PreviewsEligibilityReason::ALLOWED;
 }
 

components/previews/content/previews_optimization_guide.cc

@@ -40,6 +40,10 @@ bool ShouldApplyPreviewWithDecision(
 // The default max size of the cache holding resource loading hints by URL.
 size_t kDefaultMaxResourceLoadingHintsCacheSize = 10;
 
+// The max size of the cache holding painful page load decisions by the
+// navigation ID of the navigation handle.
+size_t kDefaultPainfulPageLoadDecisionsCacheSize = 10;
+
 // Returns base::nullopt if |previews_type| can't be converted.
 base::Optional<optimization_guide::proto::OptimizationType>
 ConvertPreviewsTypeToOptimizationType(PreviewsType previews_type) {
@@ -100,6 +104,7 @@ PreviewsOptimizationGuide::PreviewsOptimizationGuide(
     optimization_guide::OptimizationGuideDecider* optimization_guide_decider)
     : optimization_guide_decider_(optimization_guide_decider),
       resource_loading_hints_cache_(kDefaultMaxResourceLoadingHintsCacheSize),
+      painful_page_load_decisions_(kDefaultPainfulPageLoadDecisionsCacheSize),
       registered_optimization_types_(GetOptimizationTypesToRegister()) {
   DCHECK(optimization_guide_decider_);
 
@@ -113,17 +118,50 @@ PreviewsOptimizationGuide::PreviewsOptimizationGuide(
 
 PreviewsOptimizationGuide::~PreviewsOptimizationGuide() = default;
 
+void PreviewsOptimizationGuide::StartCheckingIfShouldShowPreview(
+    content::NavigationHandle* navigation_handle) {
+  if (params::OverrideShouldShowPreviewCheck()) {
+    // We are not going to use the decision from |optimization_guide_decider_|,
+    // so just return.
+    return;
+  }
+
+  if (painful_page_load_decisions_.Get(navigation_handle->GetNavigationId()) !=
+      painful_page_load_decisions_.end()) {
+    // We have either already evaluated the model or have kicked off a model
+    // evaluation, so just return.
+    return;
+  }
+
+  painful_page_load_decisions_.Put(
+      navigation_handle->GetNavigationId(),
+      optimization_guide::OptimizationGuideDecision::kUnknown);
+  optimization_guide_decider_->ShouldTargetNavigationAsync(
+      navigation_handle,
+      optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD,
+      /*client_model_features=*/{},
+      base::BindOnce(&PreviewsOptimizationGuide::OnPainfulPageLoadDecision,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     navigation_handle->GetNavigationId()));
+}
+
+void PreviewsOptimizationGuide::OnPainfulPageLoadDecision(
+    int64_t navigation_id,
+    optimization_guide::OptimizationGuideDecision decision) {
+  painful_page_load_decisions_.Put(navigation_id, decision);
+}
+
 bool PreviewsOptimizationGuide::ShouldShowPreview(
     content::NavigationHandle* navigation_handle) {
   // See if we should override the optimization guide and always show a preview.
   if (params::OverrideShouldShowPreviewCheck())
     return true;
 
-  optimization_guide::OptimizationGuideDecision decision =
-      optimization_guide_decider_->ShouldTargetNavigation(
-          navigation_handle,
-          optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD);
-  return decision == optimization_guide::OptimizationGuideDecision::kTrue;
+  auto ppd_iter =
+      painful_page_load_decisions_.Get(navigation_handle->GetNavigationId());
+  return ppd_iter != painful_page_load_decisions_.end() &&
+         ppd_iter->second ==
+             optimization_guide::OptimizationGuideDecision::kTrue;
 }
 
 bool PreviewsOptimizationGuide::CanApplyPreview(

components/previews/content/previews_optimization_guide.h

@@ -10,6 +10,7 @@
 
 #include "base/containers/flat_set.h"
 #include "base/containers/mru_cache.h"
+#include "base/memory/weak_ptr.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 
 namespace content {
@@ -20,6 +21,7 @@ class GURL;
 
 namespace optimization_guide {
 class OptimizationGuideDecider;
+enum class OptimizationGuideDecision;
 }  // namespace optimization_guide
 
 namespace previews {
@@ -37,6 +39,11 @@ class PreviewsOptimizationGuide {
       delete;
   virtual ~PreviewsOptimizationGuide();
 
+  // Kicks off the call to |optimization_guide_decider_| for whether a Preview
+  // should be shown for the current conditions.
+  virtual void StartCheckingIfShouldShowPreview(
+      content::NavigationHandle* navigation_handle);
+
   // Returns whether a Preview should be shown for the current conditions.
   virtual bool ShouldShowPreview(content::NavigationHandle* navigation_handle);
 
@@ -54,6 +61,13 @@ class PreviewsOptimizationGuide {
       std::vector<std::string>* out_resource_patterns_to_block);
 
  private:
+  // Invoked when |optimization_guide_decider_| sends the decision for whether
+  // the page load for the navigation, as expressed by |navigation_id|, is
+  // painful or not.
+  void OnPainfulPageLoadDecision(
+      int64_t navigation_id,
+      optimization_guide::OptimizationGuideDecision decision);
+
   // The Optimization Guide Decider to consult for whether an optimization can
   // be applied. Not owned.
   optimization_guide::OptimizationGuideDecider* optimization_guide_decider_;
@@ -62,9 +76,16 @@ class PreviewsOptimizationGuide {
   // us to avoid making too many calls to |optimization_guide_decider_|.
   base::MRUCache<GURL, std::vector<std::string>> resource_loading_hints_cache_;
 
+  // An in-memory cache of painful page load decisions keyed by the navigation
+  // ID of the navigation handle that the decision was evaluated on.
+  base::MRUCache<int64_t, optimization_guide::OptimizationGuideDecision>
+      painful_page_load_decisions_;
+
   // The optimization types registered with |optimization_guide_decider_|.
   const base::flat_set<optimization_guide::proto::OptimizationType>
       registered_optimization_types_;
+
+  base::WeakPtrFactory<PreviewsOptimizationGuide> weak_ptr_factory_{this};
 };
 
 }  // namespace previews

components/previews/content/previews_optimization_guide_unittest.cc

@@ -61,22 +61,32 @@ class TestOptimizationGuideDecider
     return registered_optimization_targets_;
   }
 
-  optimization_guide::OptimizationGuideDecision ShouldTargetNavigation(
+  void ShouldTargetNavigationAsync(
       content::NavigationHandle* navigation_handle,
-      optimization_guide::proto::OptimizationTarget optimization_target)
+      optimization_guide::proto::OptimizationTarget optimization_target,
+      const base::flat_map<optimization_guide::proto::ClientModelFeature,
+                           float>& client_model_features,
+      optimization_guide::OptimizationGuideTargetDecisionCallback callback)
       override {
     // This method should always be called with the painful page load target.
     EXPECT_EQ(optimization_target,
               optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD);
+    // We expect that the opt guide will calculate all features for us and we
+    // will not override with anything.
+    EXPECT_TRUE(client_model_features.empty());
 
     net::EffectiveConnectionType ect =
         network_quality_tracker_.GetEffectiveConnectionType();
     if (ect == net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {
-      return optimization_guide::OptimizationGuideDecision::kUnknown;
+      std::move(callback).Run(
+          optimization_guide::OptimizationGuideDecision::kUnknown);
+    } else if (ect <= net::EFFECTIVE_CONNECTION_TYPE_2G) {
+      std::move(callback).Run(
+          optimization_guide::OptimizationGuideDecision::kTrue);
+    } else {
+      std::move(callback).Run(
+          optimization_guide::OptimizationGuideDecision::kFalse);
     }
-    if (ect <= net::EFFECTIVE_CONNECTION_TYPE_2G)
-      return optimization_guide::OptimizationGuideDecision::kTrue;
-    return optimization_guide::OptimizationGuideDecision::kFalse;
   }
 
   optimization_guide::OptimizationGuideDecision CanApplyOptimization(
@@ -373,6 +383,10 @@ TEST_F(PreviewsOptimizationGuideTest,
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("doesntmatter"));
 
+  // This should be a no-op, but call it just to make sure any decisions are
+  // overridden.
+  guide.StartCheckingIfShouldShowPreview(&navigation_handle);
+
   EXPECT_TRUE(guide.ShouldShowPreview(&navigation_handle));
 }
 
@@ -393,6 +407,8 @@ TEST_F(PreviewsOptimizationGuideTest,
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("doesntmatter"));
 
+  guide.StartCheckingIfShouldShowPreview(&navigation_handle);
+
   EXPECT_FALSE(guide.ShouldShowPreview(&navigation_handle));
 }
 
@@ -406,6 +422,8 @@ TEST_F(PreviewsOptimizationGuideTest,
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("doesntmatter"));
 
+  guide.StartCheckingIfShouldShowPreview(&navigation_handle);
+
   EXPECT_TRUE(guide.ShouldShowPreview(&navigation_handle));
 }
 
@@ -419,6 +437,8 @@ TEST_F(PreviewsOptimizationGuideTest,
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("doesntmatter"));
 
+  guide.StartCheckingIfShouldShowPreview(&navigation_handle);
+
   EXPECT_FALSE(guide.ShouldShowPreview(&navigation_handle));
 }
 
@@ -432,6 +452,8 @@ TEST_F(PreviewsOptimizationGuideTest,
   content::MockNavigationHandle navigation_handle;
   navigation_handle.set_url(GURL("doesntmatter"));
 
+  guide.StartCheckingIfShouldShowPreview(&navigation_handle);
+
   EXPECT_FALSE(guide.ShouldShowPreview(&navigation_handle));
 }