Add async CanApplyOptimization method to public interface

This also migrates subresource redirect to use the new API

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

chrome/browser/data_saver/subresource_redirect_browsertest.cc

@@ -153,15 +153,6 @@ class SubresourceRedirectBrowserTest : public InProcessBrowserTest {
     RetryForHistogramUntilCountReached(
         &histogram_tester_,
         optimization_guide::kComponentHintsUpdatedResultHistogramString, 1);
-
-    // Navigate to |hint_setup_url| to prime the OptimizationGuide hints for the
-    // url's host and ensure that they have been loaded from the store (via
-    // histogram) prior to the navigation that tests functionality.
-    ui_test_utils::NavigateToURL(browser(), hint_setup_url);
-
-    RetryForHistogramUntilCountReached(
-        &histogram_tester_, optimization_guide::kLoadedHintLocalHistogramString,
-        1);
   }
 
   GURL http_url() const { return http_url_; }

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -876,7 +876,7 @@ OptimizationGuideHintsManager::CanApplyOptimization(
 void OptimizationGuideHintsManager::CanApplyOptimizationAsync(
     const GURL& navigation_url,
     optimization_guide::proto::OptimizationType optimization_type,
-    OptimizationGuideDecisionCallback callback) {
+    optimization_guide::OptimizationGuideDecisionCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   optimization_guide::OptimizationMetadata metadata;

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -22,6 +22,7 @@
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service.h"
 #include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/hints_fetcher.h"
+#include "components/optimization_guide/optimization_guide_decider.h"
 #include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/optimization_guide/proto/models.pb.h"
@@ -47,11 +48,8 @@ class SharedURLLoaderFactory;
 namespace optimization_guide {
 class HintCache;
 class HintsFetcherFactory;
-enum class OptimizationGuideDecision;
 class OptimizationFilter;
-struct OptimizationMetadata;
 class OptimizationGuideService;
-enum class OptimizationTarget;
 enum class OptimizationTargetDecision;
 enum class OptimizationTypeDecision;
 class StoreUpdateData;
@@ -62,10 +60,6 @@ class OptimizationGuideNavigationData;
 class PrefService;
 class Profile;
 
-using OptimizationGuideDecisionCallback =
-    base::OnceCallback<void(optimization_guide::OptimizationGuideDecision,
-                            const optimization_guide::OptimizationMetadata&)>;
-
 class OptimizationGuideHintsManager
     : public optimization_guide::OptimizationGuideServiceObserver,
       public network::NetworkQualityTracker::EffectiveConnectionTypeObserver,
@@ -131,7 +125,7 @@ class OptimizationGuideHintsManager
   void CanApplyOptimizationAsync(
       const GURL& navigation_url,
       optimization_guide::proto::OptimizationType optimization_type,
-      OptimizationGuideDecisionCallback callback);
+      optimization_guide::OptimizationGuideDecisionCallback callback);
 
   // Clears fetched hints from |hint_cache_|.
   void ClearFetchedHints();
@@ -354,9 +348,11 @@ class OptimizationGuideHintsManager
       blacklist_optimization_filters_ GUARDED_BY(optimization_filters_lock_);
 
   // A map from URL to a map of callbacks keyed by their optimization type.
-  base::flat_map<GURL,
-                 base::flat_map<optimization_guide::proto::OptimizationType,
-                                std::vector<OptimizationGuideDecisionCallback>>>
+  base::flat_map<
+      GURL,
+      base::flat_map<
+          optimization_guide::proto::OptimizationType,
+          std::vector<optimization_guide::OptimizationGuideDecisionCallback>>>
       registered_callbacks_;
 
   // Background thread where hints processing should be performed.

chrome/browser/optimization_guide/optimization_guide_keyed_service.cc

@@ -179,6 +179,9 @@ optimization_guide::OptimizationGuideDecision
 OptimizationGuideKeyedService::ShouldTargetNavigation(
     content::NavigationHandle* navigation_handle,
     optimization_guide::proto::OptimizationTarget optimization_target) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(navigation_handle->IsInMainFrame());
+
   if (!hints_manager_) {
     // We are not initialized yet, just return unknown.
     LogOptimizationTargetDecision(
@@ -208,6 +211,9 @@ OptimizationGuideKeyedService::CanApplyOptimization(
     content::NavigationHandle* navigation_handle,
     optimization_guide::proto::OptimizationType optimization_type,
     optimization_guide::OptimizationMetadata* optimization_metadata) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(navigation_handle->IsInMainFrame());
+
   if (!hints_manager_) {
     // We are not initialized yet, just return unknown.
     LogOptimizationTypeDecision(
@@ -226,6 +232,24 @@ OptimizationGuideKeyedService::CanApplyOptimization(
       optimization_type_decision);
 }
 
+void OptimizationGuideKeyedService::CanApplyOptimizationAsync(
+    content::NavigationHandle* navigation_handle,
+    optimization_guide::proto::OptimizationType optimization_type,
+    optimization_guide::OptimizationGuideDecisionCallback callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(navigation_handle->IsInMainFrame());
+
+  if (!hints_manager_) {
+    std::move(callback).Run(
+        optimization_guide::OptimizationGuideDecision::kUnknown,
+        /*metadata=*/{});
+    return;
+  }
+
+  hints_manager_->CanApplyOptimizationAsync(
+      navigation_handle->GetURL(), optimization_type, std::move(callback));
+}
+
 void OptimizationGuideKeyedService::ClearData() {
   if (hints_manager_)
     hints_manager_->ClearFetchedHints();

chrome/browser/optimization_guide/optimization_guide_keyed_service.h

@@ -92,6 +92,10 @@ class OptimizationGuideKeyedService
       content::NavigationHandle* navigation_handle,
       optimization_guide::proto::OptimizationType optimization_type,
       optimization_guide::OptimizationMetadata* optimization_metadata) override;
+  void CanApplyOptimizationAsync(
+      content::NavigationHandle* navigation_handle,
+      optimization_guide::proto::OptimizationType optimization_type,
+      optimization_guide::OptimizationGuideDecisionCallback callback) override;
 
   // KeyedService implementation:
   void Shutdown() override;

chrome/browser/optimization_guide/optimization_guide_keyed_service_browsertest.cc

@@ -89,6 +89,12 @@ class OptimizationGuideConsumerWebContentsObserver
     last_can_apply_optimization_decision_ = service->CanApplyOptimization(
         navigation_handle, optimization_guide::proto::NOSCRIPT,
         /*optimization_metadata=*/nullptr);
+
+    if (callback_) {
+      service->CanApplyOptimizationAsync(navigation_handle,
+                                         optimization_guide::proto::NOSCRIPT,
+                                         std::move(callback_));
+    }
   }
 
   // Returns the last optimization guide decision that was returned by the
@@ -105,6 +111,11 @@ class OptimizationGuideConsumerWebContentsObserver
     return last_can_apply_optimization_decision_;
   }
 
+  void set_callback(
+      optimization_guide::OptimizationGuideDecisionCallback callback) {
+    callback_ = std::move(callback);
+  }
+
  private:
   optimization_guide::OptimizationGuideDecision
       last_should_target_navigation_decision_ =
@@ -112,6 +123,7 @@ class OptimizationGuideConsumerWebContentsObserver
   optimization_guide::OptimizationGuideDecision
       last_can_apply_optimization_decision_ =
           optimization_guide::OptimizationGuideDecision::kUnknown;
+  optimization_guide::OptimizationGuideDecisionCallback callback_;
 };
 
 }  // namespace
@@ -175,7 +187,8 @@ class OptimizationGuideKeyedServiceBrowserTest
   void RegisterWithKeyedService() {
     OptimizationGuideKeyedServiceFactory::GetForProfile(browser()->profile())
         ->RegisterOptimizationTypesAndTargets(
-            {optimization_guide::proto::NOSCRIPT}, /*optimization_targets=*/{});
+            {optimization_guide::proto::NOSCRIPT},
+            /*optimization_targets=*/{});
 
     // Set up an OptimizationGuideKeyedService consumer.
     consumer_.reset(new OptimizationGuideConsumerWebContentsObserver(
@@ -207,6 +220,15 @@ class OptimizationGuideKeyedServiceBrowserTest
         ->ReportEffectiveConnectionTypeForTesting(effective_connection_type);
   }
 
+  // Sets the callback on the consumer of the OptimizationGuideKeyedService. If
+  // set, this will call the async version of CanApplyOptimization.
+  void SetCallbackOnConsumer(
+      optimization_guide::OptimizationGuideDecisionCallback callback) {
+    ASSERT_TRUE(consumer_);
+
+    consumer_->set_callback(std::move(callback));
+  }
+
   // Returns the last decision from the CanApplyOptimization() method seen by
   // the consumer of the OptimizationGuideKeyedService.
   optimization_guide::OptimizationGuideDecision
@@ -274,6 +296,48 @@ IN_PROC_BROWSER_TEST_F(
   histogram_tester.ExpectTotalCount("OptimizationGuide.LoadedHint.Result", 0);
 }
 
+IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
+                       NavigateToPageWithAsyncCallbackReturnsAnswer) {
+  PushHintsComponentAndWaitForCompletion();
+  RegisterWithKeyedService();
+
+  std::unique_ptr<base::RunLoop> run_loop = std::make_unique<base::RunLoop>();
+  SetCallbackOnConsumer(base::BindOnce(
+      [](base::RunLoop* run_loop,
+         optimization_guide::OptimizationGuideDecision decision,
+         const optimization_guide::OptimizationMetadata& metadata) {
+        EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kTrue,
+                  decision);
+        EXPECT_EQ(metadata.previews_metadata.max_ect_trigger(),
+                  optimization_guide::proto::EFFECTIVE_CONNECTION_TYPE_2G);
+        run_loop->Quit();
+      },
+      run_loop.get()));
+
+  ui_test_utils::NavigateToURL(browser(), url_with_hints());
+  run_loop->Run();
+}
+
+IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
+                       NavigateToPageWithAsyncCallbackReturnsAnswerEventually) {
+  PushHintsComponentAndWaitForCompletion();
+  RegisterWithKeyedService();
+
+  std::unique_ptr<base::RunLoop> run_loop = std::make_unique<base::RunLoop>();
+  SetCallbackOnConsumer(base::BindOnce(
+      [](base::RunLoop* run_loop,
+         optimization_guide::OptimizationGuideDecision decision,
+         const optimization_guide::OptimizationMetadata& metadata) {
+        EXPECT_EQ(optimization_guide::OptimizationGuideDecision::kFalse,
+                  decision);
+        run_loop->Quit();
+      },
+      run_loop.get()));
+
+  ui_test_utils::NavigateToURL(browser(), GURL("https://nohints.com/"));
+  run_loop->Run();
+}
+
 IN_PROC_BROWSER_TEST_F(OptimizationGuideKeyedServiceBrowserTest,
                        NavigateToPageWithHintsLoadsHint) {
   PushHintsComponentAndWaitForCompletion();

chrome/browser/subresource_redirect/subresource_redirect_observer.cc

@@ -12,6 +12,7 @@
 #include "components/optimization_guide/proto/performance_hints_metadata.pb.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
@@ -42,23 +43,51 @@ GetOptimizationGuideDeciderFromWebContents(content::WebContents* web_contents) {
   return nullptr;
 }
 
-// Pass down the |images_hints| to the appropriate renderer for the navigation
-// |navigation_handle|.
+// Pass down the |images_hints| to |render_frame_host|.
 void SetResourceLoadingImageHints(
-    content::NavigationHandle* navigation_handle,
+    content::RenderFrameHost* render_frame_host,
     blink::mojom::CompressPublicImagesHintsPtr images_hints) {
   mojo::AssociatedRemote<blink::mojom::PreviewsResourceLoadingHintsReceiver>
       loading_hints_agent;
 
-  if (navigation_handle->GetRenderFrameHost()
-          ->GetRemoteAssociatedInterfaces()) {
-    navigation_handle->GetRenderFrameHost()
-        ->GetRemoteAssociatedInterfaces()
-        ->GetInterface(&loading_hints_agent);
+  if (render_frame_host->GetRemoteAssociatedInterfaces()) {
+    render_frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
+        &loading_hints_agent);
     loading_hints_agent->SetCompressPublicImagesHints(std::move(images_hints));
   }
 }
 
+// Invoked when the OptimizationGuideKeyedService has sufficient information
+// to make a decision for whether we can send resource loading image hints.
+// If |decision| is true, public image URLs contained in |metadata| will be
+// sent to the render frame host as specified by
+// |render_frame_host_routing_id| to later be compressed.
+void OnReadyToSendResourceLoadingImageHints(
+    content::GlobalFrameRoutingId render_frame_host_routing_id,
+    optimization_guide::OptimizationGuideDecision decision,
+    const optimization_guide::OptimizationMetadata& optimization_metadata) {
+  content::RenderFrameHost* current_render_frame_host =
+      content::RenderFrameHost::FromID(render_frame_host_routing_id);
+  // Check if the same render frame host is still valid.
+  if (!current_render_frame_host)
+    return;
+
+  if (decision != optimization_guide::OptimizationGuideDecision::kTrue)
+    return;
+
+  std::vector<std::string> public_image_urls;
+  public_image_urls.reserve(
+      optimization_metadata.public_image_metadata.url_size());
+  for (const auto& url : optimization_metadata.public_image_metadata.url())
+    public_image_urls.push_back(url);
+  // Pass down the image URLs to renderer even if it could be empty. This acts
+  // as a signal that the image hint fetch has finished, for coverage metrics
+  // purposes.
+  SetResourceLoadingImageHints(
+      current_render_frame_host,
+      blink::mojom::CompressPublicImagesHints::New(public_image_urls));
+}
+
 }  // namespace
 
 // static
@@ -80,6 +109,8 @@ SubresourceRedirectObserver::SubresourceRedirectObserver(
   }
 }
 
+SubresourceRedirectObserver::~SubresourceRedirectObserver() = default;
+
 void SubresourceRedirectObserver::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   DCHECK(navigation_handle);
@@ -92,25 +123,14 @@ void SubresourceRedirectObserver::ReadyToCommitNavigation(
   if (!optimization_guide_decider)
     return;
 
-  optimization_guide::OptimizationMetadata optimization_metadata;
-  if (optimization_guide_decider->CanApplyOptimization(
-          navigation_handle, optimization_guide::proto::COMPRESS_PUBLIC_IMAGES,
-          &optimization_metadata) !=
-      optimization_guide::OptimizationGuideDecision::kTrue) {
-    return;
-  }
-
-  std::vector<std::string> public_image_urls;
-  public_image_urls.reserve(
-      optimization_metadata.public_image_metadata.url_size());
-  for (const auto& url : optimization_metadata.public_image_metadata.url())
-    public_image_urls.push_back(url);
-  // Pass down the image URLs to renderer even if it could be empty. This acts
-  // as a signal that the image hint fetch has finished, for coverage metrics
-  // purposes.
-  SetResourceLoadingImageHints(
-      navigation_handle,
-      blink::mojom::CompressPublicImagesHints::New(public_image_urls));
+  content::RenderFrameHost* render_frame_host =
+      navigation_handle->GetRenderFrameHost();
+  optimization_guide_decider->CanApplyOptimizationAsync(
+      navigation_handle, optimization_guide::proto::COMPRESS_PUBLIC_IMAGES,
+      base::BindOnce(&OnReadyToSendResourceLoadingImageHints,
+                     content::GlobalFrameRoutingId(
+                         render_frame_host->GetProcess()->GetID(),
+                         render_frame_host->GetRoutingID())));
 }
 
 WEB_CONTENTS_USER_DATA_KEY_IMPL(SubresourceRedirectObserver)

chrome/browser/subresource_redirect/subresource_redirect_observer.h

@@ -23,7 +23,7 @@ class SubresourceRedirectObserver
  public:
   static void MaybeCreateForWebContents(content::WebContents* web_contents);
 
-  ~SubresourceRedirectObserver() override = default;
+  ~SubresourceRedirectObserver() override;
   SubresourceRedirectObserver(const SubresourceRedirectObserver&) = delete;
   SubresourceRedirectObserver& operator=(const SubresourceRedirectObserver&) =
       delete;

components/optimization_guide/optimization_guide_decider.h

@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "base/callback_forward.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 #include "components/optimization_guide/proto/models.pb.h"
 
@@ -43,6 +44,10 @@ struct OptimizationMetadata {
   proto::PublicImageMetadata public_image_metadata;
 };
 
+using OptimizationGuideDecisionCallback =
+    base::OnceCallback<void(optimization_guide::OptimizationGuideDecision,
+                            const optimization_guide::OptimizationMetadata&)>;
+
 class OptimizationGuideDecider {
  public:
   // Registers the optimization types and targets that intend to be queried
@@ -52,18 +57,29 @@ class OptimizationGuideDecider {
       const std::vector<proto::OptimizationType>& optimization_types,
       const std::vector<proto::OptimizationTarget>& optimization_targets) = 0;
 
-  // Returns whether the current conditions match |optimization_target|.
+  // Returns whether the current conditions match |optimization_target|. This
+  // should only be called for main frame navigations.
   virtual OptimizationGuideDecision ShouldTargetNavigation(
       content::NavigationHandle* navigation_handle,
       proto::OptimizationTarget optimization_target) = 0;
 
   // Returns whether |optimization_type| can be applied for the URL associated
-  // with |navigation_handle|.
+  // with |navigation_handle|. This should only be called for main frame
+  // navigations.
   virtual OptimizationGuideDecision CanApplyOptimization(
       content::NavigationHandle* navigation_handle,
       proto::OptimizationType optimization_type,
       OptimizationMetadata* optimization_metadata) = 0;
 
+  // Invokes |callback| with the decision for the URL contained in
+  // |navigation_handle| and |optimization_type|, when sufficient information
+  // has been collected to make the decision. This should only be called for
+  // main frame navigations.
+  virtual void CanApplyOptimizationAsync(
+      content::NavigationHandle* navigation_handle,
+      proto::OptimizationType optimization_type,
+      OptimizationGuideDecisionCallback callback) = 0;
+
  protected:
   OptimizationGuideDecider() {}
   virtual ~OptimizationGuideDecider() {}

components/previews/content/previews_decider_impl_unittest.cc

@@ -172,6 +172,11 @@ class TestOptimizationGuideDecider
       override {
     return optimization_guide::OptimizationGuideDecision::kFalse;
   }
+  void CanApplyOptimizationAsync(
+      content::NavigationHandle* navigation_handle,
+      optimization_guide::proto::OptimizationType optimization_type,
+      optimization_guide::OptimizationGuideDecisionCallback callback) override {
+  }
 };
 
 // Stub class of PreviewsOptimizationGuide to control what is allowed when

components/previews/content/previews_optimization_guide_unittest.cc

@@ -94,6 +94,13 @@ class TestOptimizationGuideDecider
     return std::get<0>(response);
   }
 
+  void CanApplyOptimizationAsync(
+      content::NavigationHandle* navigation_handle,
+      optimization_guide::proto::OptimizationType,
+      optimization_guide::OptimizationGuideDecisionCallback callback) override {
+    NOTREACHED();
+  }
+
   void SetResponses(
       std::map<std::tuple<GURL, optimization_guide::proto::OptimizationType>,
                std::tuple<optimization_guide::OptimizationGuideDecision,