Optimization guide: Fetch hints from remote service

Fetch hints from remote service at the time of navigation.
Hints are fetched only if connection is slow.

Change-Id: I37e13072b17e1adfe9b3365e2dca04ae9bdfc246
Bug: 933898
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829474
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702116}

chrome/browser/optimization_guide/optimization_guide_hints_manager.cc

@@ -490,6 +490,8 @@ void OptimizationGuideHintsManager::OnHintsFetched(
       OnTopHostsHintsFetched(std::move(get_hints_response));
       return;
     case optimization_guide::proto::CONTEXT_PAGE_NAVIGATION:
+      OnPageNavigationHintsFetched(std::move(get_hints_response));
+      return;
     case optimization_guide::proto::CONTEXT_UNSPECIFIED:
       NOTREACHED();
   }
@@ -517,6 +519,19 @@ void OptimizationGuideHintsManager::OnTopHostsHintsFetched(
   }
 }
 
+void OptimizationGuideHintsManager::OnPageNavigationHintsFetched(
+    base::Optional<std::unique_ptr<optimization_guide::proto::GetHintsResponse>>
+        get_hints_response) {
+  if (!get_hints_response.has_value() || !get_hints_response.value())
+    return;
+
+  hint_cache_->UpdateFetchedHints(
+      std::move(*get_hints_response), clock_->Now() + kUpdateFetchedHintsDelay,
+      base::BindOnce(
+          &OptimizationGuideHintsManager::OnFetchedPageNavigationHintsStored,
+          ui_weak_ptr_factory_.GetWeakPtr()));
+}
+
 void OptimizationGuideHintsManager::OnFetchedTopHostsHintsStored() {
   LOCAL_HISTOGRAM_BOOLEAN("OptimizationGuide.FetchedHints.Stored", true);
 
@@ -526,6 +541,11 @@ void OptimizationGuideHintsManager::OnFetchedTopHostsHintsStored() {
       &OptimizationGuideHintsManager::ScheduleTopHostsHintsFetch);
 }
 
+void OptimizationGuideHintsManager::OnFetchedPageNavigationHintsStored() {
+  for (const auto& url : navigation_urls_last_fetched_real_time_)
+    LoadHintForURL(url, base::DoNothing());
+}
+
 base::Time OptimizationGuideHintsManager::GetLastHintsFetchAttemptTime() const {
   return base::Time::FromDeltaSinceWindowsEpoch(
       base::TimeDelta::FromMicroseconds(pref_service_->GetInt64(
@@ -561,6 +581,14 @@ void OptimizationGuideHintsManager::LoadHintForNavigation(
     }
   }
 
+  LoadHintForURL(url, std::move(callback));
+}
+
+void OptimizationGuideHintsManager::LoadHintForURL(const GURL& url,
+                                                   base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(url.has_host());
+
   hint_cache_->LoadHint(
       url.host(),
       base::BindOnce(&OptimizationGuideHintsManager::OnHintLoaded,
@@ -780,6 +808,58 @@ void OptimizationGuideHintsManager::OnEffectiveConnectionTypeChanged(
   current_effective_connection_type_ = effective_connection_type;
 }
 
+bool OptimizationGuideHintsManager::IsAllowedToFetchNavigationHints(
+    const GURL& url) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!IsUserPermittedToFetchHints(profile_))
+    return false;
+
+  if (!url.is_valid() || !url.SchemeIs(url::kHttpsScheme))
+    return false;
+
+  base::Optional<net::EffectiveConnectionType> ect_max_threshold =
+      optimization_guide::features::
+          GetMaxEffectiveConnectionTypeForNavigationHintsFetch();
+  // If the threshold is unavailable, return early since there is no safe way to
+  // proceed.
+  if (!ect_max_threshold.has_value())
+    return false;
+
+  if (current_effective_connection_type_ <
+          net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G ||
+      current_effective_connection_type_ > ect_max_threshold.value()) {
+    return false;
+  }
+
+  return true;
+}
+
+void OptimizationGuideHintsManager::OnNavigationStartOrRedirect(
+    content::NavigationHandle* navigation_handle,
+    base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (IsAllowedToFetchNavigationHints(navigation_handle->GetURL())) {
+    std::vector<std::string> hosts{navigation_handle->GetURL().host()};
+    navigation_urls_last_fetched_real_time_.clear();
+    navigation_urls_last_fetched_real_time_.push_back(
+        navigation_handle->GetURL());
+
+    if (!hints_fetcher_) {
+      hints_fetcher_ = std::make_unique<optimization_guide::HintsFetcher>(
+          url_loader_factory_,
+          optimization_guide::features::GetOptimizationGuideServiceURL(),
+          pref_service_);
+    }
+    hints_fetcher_->FetchOptimizationGuideServiceHints(
+        hosts, optimization_guide::proto::CONTEXT_PAGE_NAVIGATION,
+        base::BindOnce(&OptimizationGuideHintsManager::OnHintsFetched,
+                       ui_weak_ptr_factory_.GetWeakPtr()));
+  }
+  LoadHintForNavigation(navigation_handle, std::move(callback));
+}
+
 void OptimizationGuideHintsManager::ClearFetchedHints() {
   hint_cache_->ClearFetchedHints();
   optimization_guide::HintsFetcher::ClearHostsSuccessfullyFetched(

chrome/browser/optimization_guide/optimization_guide_hints_manager.h

@@ -21,6 +21,7 @@
 #include "components/optimization_guide/hints_component_info.h"
 #include "components/optimization_guide/optimization_guide_service_observer.h"
 #include "components/optimization_guide/proto/hints.pb.h"
+#include "net/nqe/effective_connection_type.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
 
 namespace base {
@@ -82,13 +83,6 @@ class OptimizationGuideHintsManager
   // is called and the corresponding hints have been updated.
   void ListenForNextUpdateForTesting(base::OnceClosure next_update_closure);
 
-  // Loads the hint if available.
-  // |callback| is run when the request has finished regardless of whether there
-  // was actually a hint for that load or not. The callback can be used as a
-  // signal for tests.
-  void LoadHintForNavigation(content::NavigationHandle* navigation_handle,
-                             base::OnceClosure callback);
-
   // Registers the optimization types that have the potential for hints to be
   // called by consumers of the Optimization Guide.
   void RegisterOptimizationTypes(
@@ -136,6 +130,13 @@ class OptimizationGuideHintsManager
   void OnEffectiveConnectionTypeChanged(
       net::EffectiveConnectionType type) override;
 
+  // Notifies |this| that a navigation with |navigation_handle| has started.
+  // |callback| is run when the request has finished regardless of whether there
+  // was actually a hint for that load or not. The callback can be used as a
+  // signal for tests.
+  void OnNavigationStartOrRedirect(content::NavigationHandle* navigation_handle,
+                                   base::OnceClosure callback);
+
  private:
   // Processes the hints component.
   //
@@ -194,16 +195,29 @@ class OptimizationGuideHintsManager
           get_hints_response);
 
   // Called when the hints for the top hosts have been fetched from the remote
-  // Optimization Guide Service and are ready for parsing.
+  // Optimization Guide Service and are ready for parsing. This is used when
+  // fetching hints in batch mode.
   void OnTopHostsHintsFetched(
       base::Optional<
           std::unique_ptr<optimization_guide::proto::GetHintsResponse>>
           get_hints_response);
 
+  // Called when the hints for a navigation have been fetched from the remote
+  // Optimization Guide Service and are ready for parsing. This is used when
+  // fetching hints in real-time.
+  void OnPageNavigationHintsFetched(
+      base::Optional<
+          std::unique_ptr<optimization_guide::proto::GetHintsResponse>>
+          get_hints_response);
+
   // Called when the fetched hints have been stored in |hint_cache| and are
-  // ready to be used.
+  // ready to be used. This is used when hints were fetched in batch mode.
   void OnFetchedTopHostsHintsStored();
 
+  // Called when the fetched hints have been stored in |hint_cache| and are
+  // ready to be used. This is used when hints were fetched in real-time.
+  void OnFetchedPageNavigationHintsStored();
+
   // Returns the time when a hints fetch request was last attempted.
   base::Time GetLastHintsFetchAttemptTime() const;
 
@@ -214,6 +228,23 @@ class OptimizationGuideHintsManager
   void OnHintLoaded(base::OnceClosure callback,
                     const optimization_guide::proto::Hint* loaded_hint) const;
 
+  // Returns true if |this| is allowed to fetch hints at the navigation time for
+  // |url|.
+  bool IsAllowedToFetchNavigationHints(const GURL& url) const;
+
+  // Loads the hint if available.
+  // |callback| is run when the request has finished regardless of whether there
+  // was actually a hint for that load or not. The callback can be used as a
+  // signal for tests.
+  void LoadHintForNavigation(content::NavigationHandle* navigation_handle,
+                             base::OnceClosure callback);
+
+  // Loads the hint for |url| if available.
+  // |callback| is run when the request has finished regardless of whether there
+  // was actually a hint for that |url| or not. The callback can be used as a
+  // signal for tests.
+  void LoadHintForURL(const GURL& url, base::OnceClosure callback);
+
   // The OptimizationGuideService that this guide is listening to. Not owned.
   optimization_guide::OptimizationGuideService* const
       optimization_guide_service_;
@@ -282,6 +313,9 @@ class OptimizationGuideHintsManager
   // Used in testing to subscribe to an update event in this class.
   base::OnceClosure next_update_closure_;
 
+  // URLs for which hints were last fetched in the real-time.
+  std::vector<GURL> navigation_urls_last_fetched_real_time_;
+
   // Used to get |weak_ptr_| to self on the UI thread.
   base::WeakPtrFactory<OptimizationGuideHintsManager> ui_weak_ptr_factory_{
       this};

chrome/browser/optimization_guide/optimization_guide_hints_manager_unittest.cc

@@ -20,9 +20,11 @@
 #include "chrome/browser/previews/previews_service_factory.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h"
 #include "components/optimization_guide/bloom_filter.h"
 #include "components/optimization_guide/hints_component_util.h"
 #include "components/optimization_guide/hints_fetcher.h"
+#include "components/optimization_guide/optimization_guide_constants.h"
 #include "components/optimization_guide/optimization_guide_decider.h"
 #include "components/optimization_guide/optimization_guide_enums.h"
 #include "components/optimization_guide/optimization_guide_features.h"
@@ -217,8 +219,9 @@ class OptimizationGuideHintsManagerTest
         data_reduction_proxy::DataReductionProxyTestContext::Builder()
             .WithMockConfig()
             .Build();
-    drp_test_context_->DisableWarmupURLFetch();
     CreateServiceAndHintsManager(/*top_host_provider=*/nullptr);
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        data_reduction_proxy::switches::kEnableDataReductionProxy);
   }
 
   void TearDown() override {
@@ -381,6 +384,10 @@ class OptimizationGuideHintsManagerTest
     return GURL("https://somedomain.org/news/whatever");
   }
 
+  GURL url_without_hints() const {
+    return GURL("https://url_without_hints.org/");
+  }
+
   base::FilePath temp_dir() const { return temp_dir_.GetPath(); }
 
   TestingPrefServiceSimple* pref_service() const { return pref_service_.get(); }
@@ -687,7 +694,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
   navigation_handle->set_has_committed(true);
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -711,7 +718,7 @@ TEST_F(OptimizationGuideHintsManagerTest, LoadHintForNavigationWithHint) {
           url_with_hints());
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -735,7 +742,7 @@ TEST_F(OptimizationGuideHintsManagerTest, LoadHintForNavigationNoHint) {
           GURL("https://notinhints.com"));
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -759,7 +766,7 @@ TEST_F(OptimizationGuideHintsManagerTest, LoadHintForNavigationNoHost) {
           GURL("blargh"));
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1470,7 +1477,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1509,7 +1516,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1548,7 +1555,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1585,7 +1592,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1624,7 +1631,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1663,7 +1670,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           url_with_hints());
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1695,6 +1702,54 @@ TEST_F(OptimizationGuideHintsManagerTest,
   EXPECT_EQ(nullptr, navigation_data->page_hint());
 }
 
+TEST_F(OptimizationGuideHintsManagerTest,
+       HintsFetchedAtNavigationTime_ECT_SLOW_2G) {
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::DEFER_ALL_SCRIPT});
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      optimization_guide::features::kOptimizationHintsFetching);
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  // Set ECT estimate so hint is activated.
+  hints_manager()->OnEffectiveConnectionTypeChanged(
+      net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_without_hints());
+  base::RunLoop run_loop;
+  base::HistogramTester histogram_tester;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+  histogram_tester.ExpectUniqueSample(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.HostCount", 1, 1);
+}
+
+TEST_F(OptimizationGuideHintsManagerTest,
+       HintsNotFetchedAtNavigationTime_ECT_4G) {
+  hints_manager()->RegisterOptimizationTypes(
+      {optimization_guide::proto::DEFER_ALL_SCRIPT});
+  base::test::ScopedFeatureList scoped_list;
+  scoped_list.InitAndEnableFeature(
+      optimization_guide::features::kOptimizationHintsFetching);
+  InitializeWithDefaultConfig("1.0.0.0");
+
+  // Set ECT estimate so hint is activated.
+  hints_manager()->OnEffectiveConnectionTypeChanged(
+      net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_4G);
+  std::unique_ptr<content::MockNavigationHandle> navigation_handle =
+      CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
+          url_without_hints());
+  base::HistogramTester histogram_tester;
+  base::RunLoop run_loop;
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
+                                               run_loop.QuitClosure());
+  run_loop.Run();
+  histogram_tester.ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.HostCount", 0);
+}
+
 TEST_F(OptimizationGuideHintsManagerTest,
        CanApplyOptimizationNoMatchingPageHint) {
   InitializeWithDefaultConfig("1.0.0.0");
@@ -1706,7 +1761,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
       CreateMockNavigationHandleWithOptimizationGuideWebContentsObserver(
           GURL("https://somedomain.org/nomatch"));
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1836,7 +1891,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
   ProcessHints(config, "1.0.0.0");
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 
@@ -1901,7 +1956,7 @@ TEST_F(OptimizationGuideHintsManagerTest,
   ProcessHints(config, "1.0.0.0");
 
   base::RunLoop run_loop;
-  hints_manager()->LoadHintForNavigation(navigation_handle.get(),
+  hints_manager()->OnNavigationStartOrRedirect(navigation_handle.get(),
                                                run_loop.QuitClosure());
   run_loop.Run();
 

chrome/browser/optimization_guide/optimization_guide_keyed_service.cc

@@ -143,8 +143,12 @@ void OptimizationGuideKeyedService::Initialize(
 
 void OptimizationGuideKeyedService::MaybeLoadHintForNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (hints_manager_ && hints_manager_->HasRegisteredOptimizationTypes())
-    hints_manager_->LoadHintForNavigation(navigation_handle, base::DoNothing());
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (hints_manager_ && hints_manager_->HasRegisteredOptimizationTypes()) {
+    hints_manager_->OnNavigationStartOrRedirect(navigation_handle,
+                                                base::DoNothing());
+  }
 }
 
 void OptimizationGuideKeyedService::RegisterOptimizationTypes(

components/optimization_guide/optimization_guide_features.cc

@@ -149,5 +149,18 @@ int MaxServerBloomFilterByteSize() {
                                                 250 * 1024 /* 250KB */);
 }
 
+base::Optional<net::EffectiveConnectionType>
+GetMaxEffectiveConnectionTypeForNavigationHintsFetch() {
+  std::string param_value = base::GetFieldTrialParamValueByFeature(
+      features::kOptimizationHintsFetching,
+      "max_effective_connection_type_for_navigation_hints_fetch");
+
+  // Use a default value.
+  if (param_value.empty())
+    return net::EFFECTIVE_CONNECTION_TYPE_3G;
+
+  return net::GetEffectiveConnectionTypeForName(param_value);
+}
+
 }  // namespace features
 }  // namespace optimization_guide

components/optimization_guide/optimization_guide_features.h

@@ -9,7 +9,9 @@
 #include <utility>
 
 #include "base/feature_list.h"
+#include "base/optional.h"
 #include "base/time/time.h"
+#include "net/nqe/effective_connection_type.h"
 #include "url/gurl.h"
 
 namespace optimization_guide {
@@ -73,6 +75,12 @@ bool IsOptimizationGuideKeyedServiceEnabled();
 // a bloom filter.
 int MaxServerBloomFilterByteSize();
 
+// Maximum effective connection type at which hints can be fetched for
+// navigations in real-time. Returns null if the hints fetching for navigations
+// is disabled.
+base::Optional<net::EffectiveConnectionType>
+GetMaxEffectiveConnectionTypeForNavigationHintsFetch();
+
 }  // namespace features
 }  // namespace optimization_guide