Adds callback support for loading Hint proto from HintCache.

Bug: 867651, 856243
Change-Id: I8d96e49aaf814041954deef540bffe025e983f03
Reviewed-on: https://chromium-review.googlesource.com/1152358
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580343}

components/previews/content/hint_cache.cc

@@ -22,8 +22,13 @@ bool HintCache::HasHint(const std::string& host) const {
   return !DetermineHostSuffix(host).empty();
 }
 
-void HintCache::LoadHint(const std::string& host) {
+void HintCache::LoadHint(const std::string& host, HintLoadedCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  const optimization_guide::proto::Hint* hint = GetHint(host);
+  if (hint) {
+    // Hint already loaded in memory.
+    std::move(callback).Run(*hint);
+  }
   // TODO(dougarnett): Add backing store support to load from.
 }
 

components/previews/content/hint_cache.h

@@ -10,12 +10,16 @@
 #include <unordered_set>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/sequence_checker.h"
 #include "components/optimization_guide/proto/hints.pb.h"
 
 namespace previews {
 
+using HintLoadedCallback =
+    base::OnceCallback<void(const optimization_guide::proto::Hint&)>;
+
 // Holds a set of optimization hints received from the Cacao service.
 // This may include hints received from the ComponentUpdater and hints
 // fetched from a Cacao Optimization Guide Service API. It will hold
@@ -31,8 +35,10 @@ class HintCache {
   // in memory or persisted on disk).
   bool HasHint(const std::string& host) const;
 
-  // Requests that hint data for |host| be loaded from disk.
-  void LoadHint(const std::string& host);
+  // Requests that hint data for |host| be loaded asynchronously and passed to
+  // |callback| if/when loaded. |callback| will not be called if no hint data
+  // is found for |host|.
+  void LoadHint(const std::string& host, HintLoadedCallback callback);
 
   // Returns whether there is hint data available for |host| in memory.
   bool IsHintLoaded(const std::string& host);

components/previews/content/hint_cache_unittest.cc

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/macros.h"
 #include "components/previews/core/previews_experiments.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -22,6 +23,17 @@ class HintCacheTest : public testing::Test {
 
   ~HintCacheTest() override {}
 
+  void LoadCallback(const optimization_guide::proto::Hint& hint) {
+    loaded_hint_ = std::make_unique<optimization_guide::proto::Hint>(hint);
+  }
+
+  const optimization_guide::proto::Hint* GetLoadedHint() const {
+    return loaded_hint_.get();
+  }
+
+ private:
+  std::unique_ptr<optimization_guide::proto::Hint> loaded_hint_;
+
   DISALLOW_COPY_AND_ASSIGN(HintCacheTest);
 };
 
@@ -65,6 +77,23 @@ TEST_F(HintCacheTest, TestMemoryCache) {
             hint_cache.GetHint("host.subdomain.domain.org")->key());
 }
 
+TEST_F(HintCacheTest, TestMemoryCacheLoadCallback) {
+  HintCache hint_cache;
+
+  optimization_guide::proto::Hint hint1;
+  hint1.set_key("subdomain.domain.org");
+  hint1.set_key_representation(optimization_guide::proto::HOST_SUFFIX);
+  hint_cache.AddHint(hint1);
+
+  EXPECT_TRUE(hint_cache.IsHintLoaded("host.subdomain.domain.org"));
+  hint_cache.LoadHint(
+      "host.subdomain.domain.org",
+      base::BindOnce(&HintCacheTest::LoadCallback, base::Unretained(this)));
+
+  EXPECT_TRUE(GetLoadedHint());
+  EXPECT_EQ(hint1.key(), GetLoadedHint()->key());
+}
+
 }  // namespace
 
 }  // namespace previews

components/previews/content/previews_content_util.cc

@@ -18,7 +18,7 @@ bool HasEnabledPreviews(content::PreviewsState previews_state) {
 
 content::PreviewsState DetermineEnabledClientPreviewsState(
     const net::URLRequest& url_request,
-    const previews::PreviewsDecider* previews_decider) {
+    previews::PreviewsDecider* previews_decider) {
   content::PreviewsState previews_state = content::PREVIEWS_UNSPECIFIED;
 
   if (!previews::params::ArePreviewsAllowed()) {
@@ -32,6 +32,8 @@ content::PreviewsState DetermineEnabledClientPreviewsState(
   if (previews_decider->ShouldAllowPreview(
           url_request, previews::PreviewsType::RESOURCE_LOADING_HINTS)) {
     previews_state |= content::RESOURCE_LOADING_HINTS_ON;
+    // Initiate load of any applicable hint details.
+    previews_decider->LoadResourceHints(url_request);
   }
 
   if (previews_decider->ShouldAllowPreview(url_request,
@@ -100,7 +102,7 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
   // decided at Commit time.
   if (previews_state & content::RESOURCE_LOADING_HINTS_ON) {
     // Resource loading hints was chosen for the original URL but only continue
-    //  with it if the committed URL has HTTPS scheme and is allowed by decider.
+    // with it if the committed URL has HTTPS scheme and is allowed by decider.
     if (is_https && previews_decider &&
         previews_decider->IsURLAllowedForPreview(
             url_request, previews::PreviewsType::RESOURCE_LOADING_HINTS)) {

components/previews/content/previews_content_util.h

@@ -19,7 +19,7 @@ bool HasEnabledPreviews(content::PreviewsState previews_state);
 // definitions for content::PreviewsState.
 content::PreviewsState DetermineEnabledClientPreviewsState(
     const net::URLRequest& url_request,
-    const previews::PreviewsDecider* previews_decider);
+    previews::PreviewsDecider* previews_decider);
 
 // Returns an updated PreviewsState given |previews_state| that has already
 // been updated wrt server previews. This should be called at Navigation Commit

components/previews/content/previews_content_util_unittest.cc

@@ -50,6 +50,8 @@ class PreviewEnabledPreviewsDecider : public PreviewsDecider {
     return IsEnabled(type);
   }
 
+  void LoadResourceHints(const net::URLRequest& request) override {}
+
  private:
   bool IsEnabled(PreviewsType type) const {
     switch (type) {

components/previews/content/previews_decider_impl.cc

@@ -26,7 +26,6 @@
 #include "net/nqe/network_quality_estimator.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
-#include "url/gurl.h"
 
 namespace previews {
 
@@ -162,6 +161,17 @@ void PreviewsDeciderImpl::InitializeOnIOThread(
                      weak_factory_.GetWeakPtr()));
 }
 
+void PreviewsDeciderImpl::OnResourceLoadingHints(
+    const GURL& document_gurl,
+    const std::vector<std::string>& patterns_to_block) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  ui_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &PreviewsUIService::SetResourceLoadingHintsResourcePatternsToBlock,
+          previews_ui_service_, document_gurl, patterns_to_block));
+}
+
 void PreviewsDeciderImpl::SetPreviewsBlacklistForTesting(
     std::unique_ptr<PreviewsBlackList> previews_back_list) {
   previews_black_list_ = std::move(previews_back_list);
@@ -385,6 +395,13 @@ bool PreviewsDeciderImpl::ShouldAllowPreviewAtECT(
   return true;
 }
 
+void PreviewsDeciderImpl::LoadResourceHints(const net::URLRequest& request) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  previews_opt_guide_->MaybeLoadOptimizationHints(
+      request, base::BindOnce(&PreviewsDeciderImpl::OnResourceLoadingHints,
+                              weak_factory_.GetWeakPtr()));
+}
+
 bool PreviewsDeciderImpl::IsURLAllowedForPreview(const net::URLRequest& request,
                                                  PreviewsType type) const {
   DCHECK(PreviewsType::NOSCRIPT == type ||

components/previews/content/previews_decider_impl.h

@@ -25,8 +25,7 @@
 #include "components/previews/core/previews_experiments.h"
 #include "components/previews/core/previews_logger.h"
 #include "net/nqe/effective_connection_type.h"
-
-class GURL;
+#include "url/gurl.h"
 
 namespace base {
 class Clock;
@@ -122,6 +121,8 @@ class PreviewsDeciderImpl : public PreviewsDecider,
   bool IsURLAllowedForPreview(const net::URLRequest& request,
                               PreviewsType type) const override;
 
+  void LoadResourceHints(const net::URLRequest& request) override;
+
   // Generates a page ID that is guaranteed to be unique from any other page ID
   // generated in this browser session. Also, guaranteed to be non-zero.
   uint64_t GeneratePageId();
@@ -133,6 +134,11 @@ class PreviewsDeciderImpl : public PreviewsDecider,
       std::unique_ptr<blacklist::OptOutStore> previews_opt_out_store,
       blacklist::BlacklistData::AllowedTypesAndVersions allowed_previews);
 
+  // Posts a task to deliver the resource patterns to the PreviewsUIService.
+  void OnResourceLoadingHints(
+      const GURL& document_gurl,
+      const std::vector<std::string>& patterns_to_block);
+
   // Sets a blacklist for testing.
   void SetPreviewsBlacklistForTesting(
       std::unique_ptr<PreviewsBlackList> previews_back_list);

components/previews/content/previews_hints.cc

@@ -276,13 +276,15 @@ bool PreviewsHints::IsWhitelisted(const GURL& url,
   return false;
 }
 
-bool PreviewsHints::MaybeLoadOptimizationHints(const GURL& url) const {
+bool PreviewsHints::MaybeLoadOptimizationHints(
+    const GURL& url,
+    HintLoadedCallback callback) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!hint_cache_ || !url.has_host())
     return false;
 
-  // TODO(dougarnett): Request loading of hints if not cached in memory.
+  hint_cache_->LoadHint(url.host(), std::move(callback));
   return hint_cache_->HasHint(url.host());
 }
 

components/previews/content/previews_hints.h

@@ -46,8 +46,10 @@ class PreviewsHints {
                      int* out_inflation_percent);
 
   // Returns whether |url| may have PageHints and triggers asynchronous load
-  // of such hints are not currently available synchronously.
-  bool MaybeLoadOptimizationHints(const GURL& url) const;
+  // of such hints are not currently available synchronously. |callback| is
+  // called if any applicable hint data is loaded and available for |url|.
+  bool MaybeLoadOptimizationHints(const GURL& url,
+                                  HintLoadedCallback callback) const;
 
  private:
   PreviewsHints();

components/previews/content/previews_optimization_guide.cc

@@ -50,13 +50,54 @@ bool PreviewsOptimizationGuide::IsWhitelisted(const net::URLRequest& request,
   return true;
 }
 
+void PreviewsOptimizationGuide::OnLoadedHint(
+    ResourceLoadingHintsCallback callback,
+    const GURL& document_url,
+    const optimization_guide::proto::Hint& loaded_hint) const {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  std::string url = document_url.spec();
+  std::vector<std::string> resource_patterns_to_block;
+  for (const auto& page_hint : loaded_hint.page_hints()) {
+    // TODO(dougarnett): Support wildcards. crbug.com/870039
+    if (!page_hint.page_pattern().empty() &&
+        url.find(page_hint.page_pattern()) != std::string::npos) {
+      // Matched the page pattern so now check for resource loading
+      // optimization.
+      for (const auto& optimization : page_hint.whitelisted_optimizations()) {
+        if (optimization.optimization_type() ==
+                optimization_guide::proto::RESOURCE_LOADING) {
+          for (const auto& resource_loading_hint :
+               optimization.resource_loading_hints()) {
+            if (!resource_loading_hint.resource_pattern().empty() &&
+                resource_loading_hint.loading_optimization_type() ==
+                    optimization_guide::proto::LOADING_BLOCK_RESOURCE) {
+              resource_patterns_to_block.push_back(
+                  resource_loading_hint.resource_pattern());
+            }
+          }
+        }
+      }
+      // Only use this first matching page hint.
+      break;
+    }
+  }
+  if (!resource_patterns_to_block.empty()) {
+    std::move(callback).Run(document_url, resource_patterns_to_block);
+  }
+}
+
 bool PreviewsOptimizationGuide::MaybeLoadOptimizationHints(
-    const net::URLRequest& request) const {
+    const net::URLRequest& request,
+    ResourceLoadingHintsCallback callback) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
 
   if (!hints_)
     return false;
-  return hints_->MaybeLoadOptimizationHints(request.url());
+
+  return hints_->MaybeLoadOptimizationHints(
+      request.url(), base::BindOnce(&PreviewsOptimizationGuide::OnLoadedHint,
+                                    io_weak_ptr_factory_.GetWeakPtr(),
+                                    std::move(callback), request.url()));
 }
 
 void PreviewsOptimizationGuide::OnHintsProcessed(

components/previews/content/previews_optimization_guide.h

@@ -5,6 +5,9 @@
 #ifndef COMPONENTS_PREVIEWS_CONTENT_PREVIEWS_OPTIMIZATION_GUIDE_H_
 #define COMPONENTS_PREVIEWS_CONTENT_PREVIEWS_OPTIMIZATION_GUIDE_H_
 
+#include <string>
+#include <vector>
+
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -14,6 +17,7 @@
 #include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/previews/content/previews_optimization_guide.h"
 #include "components/previews/core/previews_experiments.h"
+#include "url/gurl.h"
 
 namespace net {
 class URLRequest;
@@ -29,6 +33,10 @@ namespace previews {
 
 class PreviewsHints;
 
+using ResourceLoadingHintsCallback = base::OnceCallback<void(
+    const GURL& document_gurl,
+    const std::vector<std::string>& resource_patterns_to_block)>;
+
 // A Previews optimization guide that makes decisions guided by hints received
 // from the OptimizationGuideService.
 class PreviewsOptimizationGuide
@@ -50,7 +58,8 @@ class PreviewsOptimizationGuide
   // (specifically, PageHints). If so, but the hints are not available
   // synchronously, this method will request that they be loaded (from disk or
   // network).
-  bool MaybeLoadOptimizationHints(const net::URLRequest& request) const;
+  bool MaybeLoadOptimizationHints(const net::URLRequest& request,
+                                  ResourceLoadingHintsCallback callback);
 
   // optimization_guide::OptimizationGuideServiceObserver implementation:
   void OnHintsProcessed(
@@ -61,6 +70,14 @@ class PreviewsOptimizationGuide
   // Updates the hints to the latest hints sent by the Component Updater.
   void UpdateHints(std::unique_ptr<PreviewsHints> hints);
 
+  // Handles a loaded hint. It checks if the |loaded_hint| has any page hint
+  // that apply to |doucment_url|. If so, it looks for any applicable resource
+  // loading hints and will call |callback| with the applicable resource loading
+  // details if found.
+  void OnLoadedHint(ResourceLoadingHintsCallback callback,
+                    const GURL& document_url,
+                    const optimization_guide::proto::Hint& loaded_hint) const;
+
   // The OptimizationGuideService that this guide is listening to. Not owned.
   optimization_guide::OptimizationGuideService* optimization_guide_service_;
 

components/previews/content/previews_optimization_guide_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -86,6 +87,13 @@ class PreviewsOptimizationGuideTest : public testing::Test {
                                   TRAFFIC_ANNOTATION_FOR_TESTS);
   }
 
+  void MaybeLoadOptimizationHintsCallback(
+      const GURL& document_gurl,
+      const std::vector<std::string>& resource_patterns) {
+    loaded_hints_document_gurl_ = document_gurl;
+    loaded_hints_resource_patterns_ = resource_patterns;
+  }
+
   void ResetGuide() {
     guide_.reset();
     RunUntilIdle();
@@ -93,6 +101,13 @@ class PreviewsOptimizationGuideTest : public testing::Test {
 
   base::FilePath temp_dir() const { return temp_dir_.GetPath(); }
 
+  const GURL& loaded_hints_document_gurl() const {
+    return loaded_hints_document_gurl_;
+  }
+  const std::vector<std::string>& loaded_hints_resource_patterns() const {
+    return loaded_hints_resource_patterns_;
+  }
+
  protected:
   void RunUntilIdle() {
     scoped_task_environment_.RunUntilIdle();
@@ -113,6 +128,9 @@ class PreviewsOptimizationGuideTest : public testing::Test {
 
   net::TestURLRequestContext context_;
 
+  GURL loaded_hints_document_gurl_;
+  std::vector<std::string> loaded_hints_resource_patterns_;
+
   DISALLOW_COPY_AND_ASSIGN(PreviewsOptimizationGuideTest);
 };
 
@@ -586,12 +604,32 @@ void PreviewsOptimizationGuideTest::InitializeResourceLoadingHints() {
   optimization_guide::proto::Hint* hint1 = config.add_hints();
   hint1->set_key("somedomain.org");
   hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
+
+  // Page hint for "/news/"
   optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
   page_hint1->set_page_pattern("/news/");
   optimization_guide::proto::Optimization* optimization1 =
-      hint1->add_whitelisted_optimizations();
+      page_hint1->add_whitelisted_optimizations();
   optimization1->set_optimization_type(
       optimization_guide::proto::RESOURCE_LOADING);
+  optimization_guide::proto::ResourceLoadingHint* resource_loading_hint1 =
+      optimization1->add_resource_loading_hints();
+  resource_loading_hint1->set_loading_optimization_type(
+      optimization_guide::proto::LOADING_BLOCK_RESOURCE);
+  resource_loading_hint1->set_resource_pattern("news_cruft.js");
+
+  // Page hint for "football"
+  optimization_guide::proto::PageHint* page_hint2 = hint1->add_page_hints();
+  page_hint2->set_page_pattern("football");
+  optimization_guide::proto::Optimization* optimization2 =
+      page_hint2->add_whitelisted_optimizations();
+  optimization2->set_optimization_type(
+      optimization_guide::proto::RESOURCE_LOADING);
+  optimization_guide::proto::ResourceLoadingHint* resource_loading_hint2 =
+      optimization2->add_resource_loading_hints();
+  resource_loading_hint2->set_loading_optimization_type(
+      optimization_guide::proto::LOADING_BLOCK_RESOURCE);
+  resource_loading_hint2->set_resource_pattern("football_cruft.js");
   ProcessHints(config, "2.0.0");
 
   RunUntilIdle();
@@ -604,11 +642,24 @@ TEST_F(PreviewsOptimizationGuideTest, MaybeLoadOptimizationHints) {
   InitializeResourceLoadingHints();
 
   EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
-      *CreateRequestWithURL(GURL("https://somedomain.org/"))));
+      *CreateRequestWithURL(GURL("https://somedomain.org/")),
+      base::DoNothing()));
   EXPECT_TRUE(guide()->MaybeLoadOptimizationHints(
-      *CreateRequestWithURL(GURL("https://www.somedomain.org"))));
+      *CreateRequestWithURL(GURL("https://www.somedomain.org/news/football")),
+      base::BindOnce(
+          &PreviewsOptimizationGuideTest::MaybeLoadOptimizationHintsCallback,
+          base::Unretained(this))));
   EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
-      *CreateRequestWithURL(GURL("https://www.unknown.com"))));
+      *CreateRequestWithURL(GURL("https://www.unknown.com")),
+      base::DoNothing()));
+
+  RunUntilIdle();
+
+  // Verify loaded hint data for www.somedomain.org
+  EXPECT_EQ(GURL("https://www.somedomain.org/news/football"),
+            loaded_hints_document_gurl());
+  EXPECT_EQ(1ul, loaded_hints_resource_patterns().size());
+  EXPECT_EQ("news_cruft.js", loaded_hints_resource_patterns().front());
 }
 
 TEST_F(PreviewsOptimizationGuideTest,
@@ -621,7 +672,8 @@ TEST_F(PreviewsOptimizationGuideTest,
   InitializeResourceLoadingHints();
 
   EXPECT_FALSE(guide()->MaybeLoadOptimizationHints(
-      *CreateRequestWithURL(GURL("https://www.somedomain.org"))));
+      *CreateRequestWithURL(GURL("https://www.somedomain.org")),
+      base::DoNothing()));
 }
 
 TEST_F(PreviewsOptimizationGuideTest, RemoveObserverCalledAtDestruction) {

components/previews/core/previews_decider.h

@@ -45,6 +45,9 @@ class PreviewsDecider {
   virtual bool IsURLAllowedForPreview(const net::URLRequest& request,
                                       PreviewsType type) const = 0;
 
+  // Requests that any applicable detailed resource hints be loaded.
+  virtual void LoadResourceHints(const net::URLRequest& request) = 0;
+
  protected:
   PreviewsDecider() {}
   virtual ~PreviewsDecider() {}

components/previews/core/test_previews_decider.cc

@@ -31,4 +31,6 @@ bool TestPreviewsDecider::IsURLAllowedForPreview(const net::URLRequest& request,
   return allow_previews_;
 }
 
+void TestPreviewsDecider::LoadResourceHints(const net::URLRequest& request) {}
+
 }  // namespace previews

components/previews/core/test_previews_decider.h

@@ -26,6 +26,7 @@ class TestPreviewsDecider : public previews::PreviewsDecider {
                           previews::PreviewsType type) const override;
   bool IsURLAllowedForPreview(const net::URLRequest& request,
                               PreviewsType type) const override;
+  void LoadResourceHints(const net::URLRequest& request) override;
 
  private:
   bool allow_previews_;