Add support for loading predictor to use hints from optimization guide

Will add metrics in a subsequent CL. This shouldn't affect any current
metrics since the support is all behind a feature flag.

Bug: 1050101
Change-Id: If6f98dbae86bb5842803cdb3c1b052d99711e62f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067241
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743767}

chrome/browser/predictors/loading_predictor.cc

@@ -70,37 +70,48 @@ LoadingPredictor::~LoadingPredictor() {
   DCHECK(shutdown_);
 }
 
-void LoadingPredictor::PrepareForPageLoad(const GURL& url,
-                                          HintOrigin origin,
-                                          bool preconnectable) {
+bool LoadingPredictor::PrepareForPageLoad(
+    const GURL& url,
+    HintOrigin origin,
+    bool preconnectable,
+    base::Optional<PreconnectPrediction> preconnect_prediction) {
   if (shutdown_)
-    return;
+    return true;
 
   if (origin == HintOrigin::OMNIBOX) {
     // Omnibox hints are lightweight and need a special treatment.
     HandleOmniboxHint(url, preconnectable);
-    return;
+    return true;
   }
 
-  if (active_hints_.find(url) != active_hints_.end())
-    return;
-
-  bool has_preconnect_prediction = false;
   PreconnectPrediction prediction;
-  has_preconnect_prediction =
+  bool has_local_preconnect_prediction =
       resource_prefetch_predictor_->PredictPreconnectOrigins(url, &prediction);
-  // Try to preconnect to the |url| even if the predictor has no
-  // prediction.
-  has_preconnect_prediction =
-      AddInitialUrlToPreconnectPrediction(url, &prediction);
+  if (active_hints_.find(url) != active_hints_.end() &&
+      has_local_preconnect_prediction) {
+    // We are currently preconnecting using the local preconnect prediction. Do
+    // not proceed further.
+    return true;
+  }
 
-  if (!has_preconnect_prediction)
-    return;
+  if (preconnect_prediction) {
+    // Overwrite the prediction if we were provided with a non-empty one.
+    prediction = *preconnect_prediction;
+  } else {
+    // Try to preconnect to the |url| even if the predictor has no
+    // prediction.
+    AddInitialUrlToPreconnectPrediction(url, &prediction);
+  }
+
+  // Return early if we do not have any preconnect requests.
+  if (prediction.requests.empty())
+    return false;
 
   ++total_hints_activated_;
   active_hints_.emplace(url, base::TimeTicks::Now());
   if (IsPreconnectAllowed(profile_))
     MaybeAddPreconnect(url, std::move(prediction.requests), origin);
+  return has_local_preconnect_prediction || preconnect_prediction;
 }
 
 void LoadingPredictor::CancelPageLoadHint(const GURL& url) {
@@ -143,14 +154,15 @@ void LoadingPredictor::Shutdown() {
   shutdown_ = true;
 }
 
-void LoadingPredictor::OnNavigationStarted(const NavigationID& navigation_id) {
+bool LoadingPredictor::OnNavigationStarted(const NavigationID& navigation_id) {
   if (shutdown_)
-    return;
+    return true;
 
   loading_data_collector()->RecordStartNavigation(navigation_id);
   CleanupAbandonedHintsAndNavigations(navigation_id);
   active_navigations_.emplace(navigation_id);
-  PrepareForPageLoad(navigation_id.main_frame_url, HintOrigin::NAVIGATION);
+  return PrepareForPageLoad(navigation_id.main_frame_url,
+                            HintOrigin::NAVIGATION);
 }
 
 void LoadingPredictor::OnNavigationFinished(

chrome/browser/predictors/loading_predictor.h

@@ -14,6 +14,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "chrome/browser/predictors/loading_data_collector.h"
 #include "chrome/browser/predictors/navigation_id.h"
@@ -49,10 +50,15 @@ class LoadingPredictor : public KeyedService,
   ~LoadingPredictor() override;
 
   // Hints that a page load is expected for |url|, with the hint coming from a
-  // given |origin|. May trigger actions, such as prefetch and/or preconnect.
-  void PrepareForPageLoad(const GURL& url,
+  // given |origin|. If |preconnect_prediction| is provided, this will use it
+  // over local predictions to trigger actions, such as prefetch and/or
+  // preconnect. Returns true if no more preconnect actions should be taken by
+  // the caller.
+  bool PrepareForPageLoad(const GURL& url,
                           HintOrigin origin,
-                          bool preconnectable = false);
+                          bool preconnectable = false,
+                          base::Optional<PreconnectPrediction>
+                              preconnect_prediction = base::nullopt);
 
   // Indicates that a page load hint is no longer active.
   void CancelPageLoadHint(const GURL& url);
@@ -68,7 +74,10 @@ class LoadingPredictor : public KeyedService,
   // KeyedService:
   void Shutdown() override;
 
-  void OnNavigationStarted(const NavigationID& navigation_id);
+  // OnNavigationStarted is invoked when a navigation with |navigation_id| has
+  // started. It returns whether any actions were taken, such as preconnecting
+  // to known resource hosts, at that time.
+  bool OnNavigationStarted(const NavigationID& navigation_id);
   void OnNavigationFinished(const NavigationID& old_navigation_id,
                             const NavigationID& new_navigation_id,
                             bool is_error_page);

chrome/browser/predictors/loading_predictor_browsertest.cc

@@ -30,6 +30,9 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/optimization_guide/optimization_guide_features.h"
+#include "components/optimization_guide/optimization_guide_switches.h"
+#include "components/optimization_guide/proto/hints.pb.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
@@ -1488,4 +1491,151 @@ IN_PROC_BROWSER_TEST_F(LoadingPredictorBrowserTestWithProxy,
   EXPECT_EQ(0u, connection_tracker()->GetReadSocketCount());
 }
 
+class LoadingPredictorBrowserTestWithOptimizationGuide
+    : public LoadingPredictorBrowserTest {
+ public:
+  LoadingPredictorBrowserTestWithOptimizationGuide() {
+    feature_list_.InitWithFeatures(
+        {features::kLoadingPredictorUseOptimizationGuide,
+         optimization_guide::features::kOptimizationHints},
+        {});
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // TODO(crbug/1035698): Make this simpler when Optimization Guide has better
+    // test support.
+    optimization_guide::proto::Configuration config;
+    optimization_guide::proto::Hint* hint1 = config.add_hints();
+    hint1->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
+    hint1->set_key("hints.com");
+    optimization_guide::proto::PageHint* page_hint1 = hint1->add_page_hints();
+    page_hint1->set_page_pattern("*");
+    optimization_guide::proto::Optimization* opt1 =
+        page_hint1->add_whitelisted_optimizations();
+    opt1->set_optimization_type(optimization_guide::proto::LOADING_PREDICTOR);
+    opt1->mutable_loading_predictor_metadata()->add_subresources()->set_url(
+        "http://subresource.com/1");
+    opt1->mutable_loading_predictor_metadata()->add_subresources()->set_url(
+        "http://subresource.com/2");
+    opt1->mutable_loading_predictor_metadata()->add_subresources()->set_url(
+        "http://otherresource.com/2");
+    opt1->mutable_loading_predictor_metadata()->add_subresources()->set_url(
+        "skipsoverinvalidurl////");
+    optimization_guide::proto::Hint* hint2 = config.add_hints();
+    hint2->set_key_representation(optimization_guide::proto::HOST_SUFFIX);
+    hint2->set_key("test.com");
+    optimization_guide::proto::PageHint* page_hint2 = hint2->add_page_hints();
+    page_hint2->set_page_pattern("*");
+    optimization_guide::proto::Optimization* opt2 =
+        page_hint2->add_whitelisted_optimizations();
+    opt2->set_optimization_type(optimization_guide::proto::LOADING_PREDICTOR);
+    opt2->mutable_loading_predictor_metadata()->add_subresources()->set_url(
+        "https://doesntmatter.com/alsodoesntmatter");
+    std::string config_string;
+    config.SerializeToString(&config_string);
+    base::Base64Encode(config_string, &config_string);
+    command_line->AppendSwitchASCII(
+        optimization_guide::switches::kHintsProtoOverride, config_string);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(LoadingPredictorBrowserTestWithOptimizationGuide,
+                       LocalPredictionTakesPrecedence) {
+  // Navigate the first time to fill the predictor's database and the HTTP
+  // cache.
+  GURL url = embedded_test_server()->GetURL(
+      "test.com", GetPathWithPortReplacement(kHtmlSubresourcesPath,
+                                             embedded_test_server()->port()));
+  url::Origin origin = url::Origin::Create(url);
+  net::NetworkIsolationKey network_isolation_key(origin, origin);
+  ui_test_utils::NavigateToURL(browser(), url);
+  ResetNetworkState();
+
+  auto observer = NavigateToURLAsync(url);
+  EXPECT_TRUE(observer->WaitForRequestStart());
+  for (auto* const host : kHtmlSubresourcesHosts) {
+    preconnect_manager_observer()->WaitUntilHostLookedUp(host,
+                                                         network_isolation_key);
+    EXPECT_TRUE(
+        preconnect_manager_observer()->HostFound(host, network_isolation_key));
+  }
+  // 2 connections to the main frame host + 1 connection per host for others.
+  const size_t expected_connections = base::size(kHtmlSubresourcesHosts) + 1;
+  connection_tracker()->WaitForAcceptedConnections(expected_connections);
+  EXPECT_EQ(expected_connections,
+            connection_tracker()->GetAcceptedSocketCount());
+  // No reads since all resources should be cached.
+  EXPECT_EQ(0u, connection_tracker()->GetReadSocketCount());
+}
+
+IN_PROC_BROWSER_TEST_F(LoadingPredictorBrowserTestWithOptimizationGuide,
+                       UsesPredictionsFromOptimizationGuideIfAvailable) {
+  GURL url = embedded_test_server()->GetURL("hints.com", "/");
+  url::Origin origin = url::Origin::Create(url);
+  net::NetworkIsolationKey network_isolation_key(origin, origin);
+  // Navigate to URL on same host so hint will be available prior to commit.
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL(
+                                              "hints.com", "/hint_setup.html"));
+  ResetNetworkState();
+
+  auto observer = NavigateToURLAsync(url);
+  EXPECT_TRUE(observer->WaitForRequestStart());
+
+  // The initial URL should be preconnected to.
+  preconnect_manager_observer()->WaitUntilHostLookedUp("hints.com",
+                                                       network_isolation_key);
+  EXPECT_TRUE(preconnect_manager_observer()->HostFound("hints.com",
+                                                       network_isolation_key));
+  EXPECT_TRUE(preconnect_manager_observer()->HasOriginAttemptedToPreconnect(
+      url.GetOrigin()));
+
+  // Both subresource hosts should be preconnected to.
+  for (auto* const host : {"subresource.com", "otherresource.com"}) {
+    preconnect_manager_observer()->WaitUntilHostLookedUp(host,
+                                                         network_isolation_key);
+    EXPECT_TRUE(
+        preconnect_manager_observer()->HostFound(host, network_isolation_key));
+
+    EXPECT_TRUE(preconnect_manager_observer()->HasOriginAttemptedToPreconnect(
+        GURL(base::StringPrintf("http://%s/", host))));
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(
+    LoadingPredictorBrowserTestWithOptimizationGuide,
+    OptimizationGuidePredictionsNotAppliedForAlreadyCommittedNavigation) {
+  GURL url = embedded_test_server()->GetURL("hints.com", "/");
+  url::Origin origin = url::Origin::Create(url);
+  net::NetworkIsolationKey network_isolation_key(origin, origin);
+  // Navigate to URL with hints, the hints will come back eventually but
+  // after commit.
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  EXPECT_FALSE(preconnect_manager_observer()->HasHostBeenLookedUp(
+      "subresource.com", network_isolation_key));
+  EXPECT_FALSE(preconnect_manager_observer()->HasHostBeenLookedUp(
+      "otheresource.com", network_isolation_key));
+}
+
+IN_PROC_BROWSER_TEST_F(LoadingPredictorBrowserTestWithOptimizationGuide,
+                       OptimizationGuidePredictionsNotAppliedForRedirect) {
+  GURL destination_url =
+      embedded_test_server()->GetURL("hints.com", "/cachetime");
+  GURL redirecting_url = embedded_test_server()->GetURL(
+      "hints.com", "/cached-redirect?" + destination_url.spec());
+
+  url::Origin origin = url::Origin::Create(redirecting_url);
+  net::NetworkIsolationKey network_isolation_key(origin, origin);
+  // Navigate to URL with hints but is redirected, hints should not be applied.
+  ui_test_utils::NavigateToURL(browser(), redirecting_url);
+
+  EXPECT_FALSE(preconnect_manager_observer()->HasHostBeenLookedUp(
+      "subresource.com", network_isolation_key));
+  EXPECT_FALSE(preconnect_manager_observer()->HasHostBeenLookedUp(
+      "otheresource.com", network_isolation_key));
+}
+
 }  // namespace predictors

chrome/browser/predictors/loading_predictor_config.h

@@ -47,7 +47,10 @@ enum class HintOrigin {
   // preconnect is initiated. Preconnect triggered by
   // OMNIBOX_PRERENDER_FALLBACK may be handled differently than preconnects
   // triggered by OMNIBOX since the former are triggered at higher confidence.
-  OMNIBOX_PRERENDER_FALLBACK
+  OMNIBOX_PRERENDER_FALLBACK,
+
+  // Triggered by optimization guide.
+  OPTIMIZATION_GUIDE,
 };
 
 // Represents the config for the Loading predictor.

chrome/browser/predictors/loading_predictor_tab_helper.cc

@@ -4,11 +4,17 @@
 
 #include "chrome/browser/predictors/loading_predictor_tab_helper.h"
 
+#include <set>
 #include <string>
 
+#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
+#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/browser/predictors/loading_predictor_factory.h"
+#include "chrome/browser/predictors/predictors_features.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/optimization_guide/optimization_guide_decider.h"
+#include "components/optimization_guide/proto/hints.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
@@ -65,10 +71,21 @@ bool IsHandledNavigation(content::NavigationHandle* navigation_handle) {
 LoadingPredictorTabHelper::LoadingPredictorTabHelper(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {
-  auto* predictor = LoadingPredictorFactory::GetForProfile(
-      Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  auto* predictor = LoadingPredictorFactory::GetForProfile(profile);
   if (predictor)
     predictor_ = predictor->GetWeakPtr();
+  if (base::FeatureList::IsEnabled(
+          features::kLoadingPredictorUseOptimizationGuide)) {
+    optimization_guide_decider_ =
+        OptimizationGuideKeyedServiceFactory::GetForProfile(profile);
+    if (optimization_guide_decider_) {
+      optimization_guide_decider_->RegisterOptimizationTypesAndTargets(
+          {optimization_guide::proto::LOADING_PREDICTOR},
+          /*optimization_targets=*/{});
+    }
+  }
 }
 
 LoadingPredictorTabHelper::~LoadingPredictorTabHelper() = default;
@@ -86,8 +103,36 @@ void LoadingPredictorTabHelper::DidStartNavigation(
                                     navigation_handle->NavigationStart());
   if (!navigation_id.is_valid())
     return;
+  current_navigation_id_ = navigation_id;
+
+  if (predictor_->OnNavigationStarted(navigation_id))
+    return;
+
+  if (!optimization_guide_decider_)
+    return;
+
+  // We do not have any predictions on device, so consult the optimization
+  // guide.
+  optimization_guide_decider_->CanApplyOptimizationAsync(
+      navigation_handle, optimization_guide::proto::LOADING_PREDICTOR,
+      base::BindOnce(&LoadingPredictorTabHelper::OnOptimizationGuideDecision,
+                     weak_ptr_factory_.GetWeakPtr(), navigation_id));
+}
+
+void LoadingPredictorTabHelper::DidRedirectNavigation(
+    content::NavigationHandle* navigation_handle) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!predictor_)
+    return;
+
+  if (!IsHandledNavigation(navigation_handle))
+    return;
 
-  predictor_->OnNavigationStarted(navigation_id);
+  auto navigation_id = NavigationID(web_contents(), navigation_handle->GetURL(),
+                                    navigation_handle->NavigationStart());
+  if (!navigation_id.is_valid())
+    return;
+  current_navigation_id_ = navigation_id;
 }
 
 void LoadingPredictorTabHelper::DidFinishNavigation(
@@ -99,6 +144,9 @@ void LoadingPredictorTabHelper::DidFinishNavigation(
   if (!IsHandledNavigation(navigation_handle))
     return;
 
+  // Clear out the current navigation since there is not one in flight anymore.
+  current_navigation_id_ = NavigationID();
+
   auto old_navigation_id = NavigationID(
       web_contents(), navigation_handle->GetRedirectChain().front(),
       navigation_handle->NavigationStart());
@@ -171,6 +219,62 @@ void LoadingPredictorTabHelper::DocumentOnLoadCompletedInMainFrame() {
       navigation_id);
 }
 
+void LoadingPredictorTabHelper::OnOptimizationGuideDecision(
+    const NavigationID& navigation_id,
+    optimization_guide::OptimizationGuideDecision decision,
+    const optimization_guide::OptimizationMetadata& metadata) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(navigation_id.is_valid());
+
+  if (!predictor_)
+    return;
+
+  if (decision != optimization_guide::OptimizationGuideDecision::kTrue)
+    return;
+
+  if (!metadata.loading_predictor_metadata())
+    return;
+
+  if (!current_navigation_id_.is_valid()) {
+    // There is not a pending navigation, so return.
+    return;
+  }
+  if (current_navigation_id_ != navigation_id) {
+    // The current navigation has either redirected or a new one has started, so
+    // return.
+    return;
+  }
+  auto last_committed_navigation_id = NavigationID(web_contents());
+  if (last_committed_navigation_id.is_valid() &&
+      navigation_id == last_committed_navigation_id) {
+    // The navigation has already committed, so all the connections have already
+    // started.
+    return;
+  }
+
+  PreconnectPrediction prediction;
+  url::Origin main_frame_origin =
+      url::Origin::Create(navigation_id.main_frame_url);
+  net::NetworkIsolationKey network_isolation_key(main_frame_origin,
+                                                 main_frame_origin);
+  std::set<url::Origin> predicted_origins;
+  const auto lp_metadata = metadata.loading_predictor_metadata();
+  for (const auto& subresource : lp_metadata->subresources()) {
+    GURL subresource_url(subresource.url());
+    if (!subresource_url.is_valid())
+      continue;
+    url::Origin subresource_origin = url::Origin::Create(subresource_url);
+    if (predicted_origins.find(subresource_origin) != predicted_origins.end())
+      continue;
+    predicted_origins.insert(subresource_origin);
+    prediction.requests.emplace_back(subresource_origin, 1,
+                                     network_isolation_key);
+  }
+  predictor_->PrepareForPageLoad(navigation_id.main_frame_url,
+                                 HintOrigin::OPTIMIZATION_GUIDE,
+                                 /*preconnectable=*/false, prediction);
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(LoadingPredictorTabHelper)
 
 }  // namespace predictors

chrome/browser/predictors/loading_predictor_tab_helper.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_PREDICTORS_LOADING_PREDICTOR_TAB_HELPER_H_
 
 #include "base/macros.h"
+#include "chrome/browser/predictors/navigation_id.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
@@ -13,10 +14,15 @@ namespace content {
 class NavigationHandle;
 }  // namespace content
 
+namespace optimization_guide {
+class OptimizationGuideDecider;
+enum class OptimizationGuideDecision;
+class OptimizationMetadata;
+}  // namespace optimization_guide
+
 namespace predictors {
 
 class LoadingPredictor;
-
 // Observes various page load events from the navigation start to onload
 // completed and notifies the LoadingPredictor associated with the current
 // profile.
@@ -31,6 +37,8 @@ class LoadingPredictorTabHelper
   // content::WebContentsObserver implementation
   void DidStartNavigation(
       content::NavigationHandle* navigation_handle) override;
+  void DidRedirectNavigation(
+      content::NavigationHandle* navigation_handle) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
   void ResourceLoadComplete(
@@ -52,9 +60,25 @@ class LoadingPredictorTabHelper
   explicit LoadingPredictorTabHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<LoadingPredictorTabHelper>;
 
+  // Callback invoked when |optimization_guide_decider_| has the information
+  // required to decide if it has remote predictions for the page load.
+  void OnOptimizationGuideDecision(
+      const NavigationID& navigation_id,
+      optimization_guide::OptimizationGuideDecision decision,
+      const optimization_guide::OptimizationMetadata& metadata);
+
   // Owned by profile.
   base::WeakPtr<LoadingPredictor> predictor_;
 
+  NavigationID current_navigation_id_;
+
+  // The optimization guide decider to consult for remote predictions.
+  optimization_guide::OptimizationGuideDecider* optimization_guide_decider_ =
+      nullptr;
+
+  // Used to get a weak pointer to |this|.
+  base::WeakPtrFactory<LoadingPredictorTabHelper> weak_ptr_factory_{this};
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 
   DISALLOW_COPY_AND_ASSIGN(LoadingPredictorTabHelper);

chrome/browser/predictors/loading_predictor_unittest.cc

@@ -158,6 +158,18 @@ void LoadingPredictorPreconnectTest::SetPreference() {
       chrome_browser_net::NETWORK_PREDICTION_ALWAYS);
 }
 
+TEST_F(LoadingPredictorTest, TestOnNavigationStarted) {
+  const SessionID tab_id = SessionID::FromSerializedValue(12);
+
+  // Should return true if there are predictions.
+  auto navigation_id = CreateNavigationID(tab_id, kUrl);
+  EXPECT_TRUE(predictor_->OnNavigationStarted(navigation_id));
+
+  // Should return false since there are no predictions.
+  auto navigation_id2 = CreateNavigationID(tab_id, kUrl3);
+  EXPECT_FALSE(predictor_->OnNavigationStarted(navigation_id2));
+}
+
 TEST_F(LoadingPredictorTest, TestMainFrameResponseCancelsHint) {
   const GURL url = GURL(kUrl);
   predictor_->PrepareForPageLoad(url, HintOrigin::EXTERNAL);
@@ -352,11 +364,12 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlToEmptyPrediction) {
                  std::vector<PreconnectRequest>(
                      {{url::Origin::Create(GURL("http://search.com")), 2,
                        CreateNetworkIsolationKey(main_frame_url)}})));
-  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::NAVIGATION);
+  EXPECT_FALSE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::NAVIGATION));
 }
 
 // Checks that the predictor doesn't add an initial origin to a preconnect list
-// if the list already containts the origin.
+// if the list already contains the origin.
 TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlMatchesPrediction) {
   GURL main_frame_url("http://search.com/kittens");
   net::NetworkIsolationKey network_isolation_key =
@@ -381,7 +394,8 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlMatchesPrediction) {
                        network_isolation_key},
                       {url::Origin::Create(GURL("http://ads.search.com")), 0,
                        network_isolation_key}})));
-  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
+  EXPECT_TRUE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
 }
 
 // Checks that the predictor adds an initial origin to a preconnect list if the
@@ -413,7 +427,8 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlDoesntMatchPrediction) {
                        network_isolation_key},
                       {url::Origin::Create(GURL("http://ads.search.com")), 0,
                        network_isolation_key}})));
-  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
+  EXPECT_TRUE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
 }
 
 // Checks that the predictor doesn't preconnect to a bad url.
@@ -421,7 +436,140 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInvalidInitialUrl) {
   GURL main_frame_url("file:///tmp/index.html");
   EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
       .WillOnce(Return(false));
-  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
+  EXPECT_FALSE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
+}
+
+// Checks that the predictor uses the provided prediction if there isn't an
+// active hint initiated via a local prediction happening already.
+TEST_F(LoadingPredictorPreconnectTest,
+       TestPrepareForPageLoadPredictionProvided) {
+  GURL main_frame_url("http://search.com/kittens");
+  net::NetworkIsolationKey network_isolation_key =
+      CreateNetworkIsolationKey(main_frame_url);
+  EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
+      .WillOnce(Return(false));
+  PreconnectPrediction prediction = CreatePreconnectPrediction(
+      "search.com", true,
+      {{url::Origin::Create(GURL("http://cdn1.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn2.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn3.search.com")), 1,
+        network_isolation_key}});
+  EXPECT_CALL(
+      *mock_preconnect_manager_,
+      StartProxy(main_frame_url,
+                 std::vector<PreconnectRequest>(
+                     {{url::Origin::Create(GURL("http://cdn1.search.com")), 1,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://cdn2.search.com")), 1,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://cdn3.search.com")), 1,
+                       network_isolation_key}})));
+  EXPECT_TRUE(predictor_->PrepareForPageLoad(
+      main_frame_url, HintOrigin::OPTIMIZATION_GUIDE, false, prediction));
+}
+
+// Checks that the predictor does not proceed with an empty request.
+TEST_F(LoadingPredictorPreconnectTest,
+       TestPrepareForPageLoadPredictionWithEmptyRequestsProvided) {
+  GURL main_frame_url("http://nopredictions.com/");
+  net::NetworkIsolationKey network_isolation_key =
+      CreateNetworkIsolationKey(main_frame_url);
+  EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
+      .WillOnce(Return(false));
+  PreconnectPrediction prediction;
+  EXPECT_FALSE(predictor_->PrepareForPageLoad(
+      main_frame_url, HintOrigin::OPTIMIZATION_GUIDE, false, prediction));
+}
+
+// Checks that the predictor preconnects to an initial origin even when it
+// doesn't have any historical data for this host, but still allows subsequent
+// calls to PrepareForPageLoad with a provided prediction.
+TEST_F(LoadingPredictorPreconnectTest,
+       TestPrepareForPageLoadPreconnectsUsingPredictionWhenNoLocalPrediction) {
+  GURL main_frame_url("http://search.com/kittens");
+  EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
+      .WillRepeatedly(Return(false));
+  net::NetworkIsolationKey network_isolation_key =
+      CreateNetworkIsolationKey(main_frame_url);
+  EXPECT_CALL(*mock_preconnect_manager_,
+              StartProxy(main_frame_url,
+                         std::vector<PreconnectRequest>(
+                             {{url::Origin::Create(GURL("http://search.com")),
+                               2, network_isolation_key}})));
+  EXPECT_FALSE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::NAVIGATION));
+
+  // A second call to PrepareForPageLoad using a provided prediction should
+  // fire requests.
+  PreconnectPrediction prediction = CreatePreconnectPrediction(
+      "search.com", true,
+      {{url::Origin::Create(GURL("http://cdn1.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn2.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn3.search.com")), 1,
+        network_isolation_key}});
+  EXPECT_CALL(
+      *mock_preconnect_manager_,
+      StartProxy(main_frame_url,
+                 std::vector<PreconnectRequest>(
+                     {{url::Origin::Create(GURL("http://cdn1.search.com")), 1,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://cdn2.search.com")), 1,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://cdn3.search.com")), 1,
+                       network_isolation_key}})));
+  EXPECT_TRUE(predictor_->PrepareForPageLoad(
+      main_frame_url, HintOrigin::OPTIMIZATION_GUIDE, false, prediction));
+}
+
+// Checks that the predictor doesn't use the provided prediction if there is
+// already in flight and there was a local preconnect prediction.
+TEST_F(
+    LoadingPredictorPreconnectTest,
+    TestPrepareForPageLoadPredictionProvidedButHasLocalPreconnectPrediction) {
+  GURL main_frame_url("http://search.com/kittens");
+  net::NetworkIsolationKey network_isolation_key =
+      CreateNetworkIsolationKey(main_frame_url);
+  PreconnectPrediction prediction = CreatePreconnectPrediction(
+      "search.com", true,
+      {{url::Origin::Create(GURL("http://search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://ads.search.com")), 0,
+        network_isolation_key}});
+  EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
+      .WillRepeatedly(DoAll(SetArgPointee<1>(prediction), Return(true)));
+  EXPECT_CALL(
+      *mock_preconnect_manager_,
+      StartProxy(main_frame_url,
+                 std::vector<PreconnectRequest>(
+                     {{url::Origin::Create(GURL("http://search.com")), 2,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://cdn.search.com")), 1,
+                       network_isolation_key},
+                      {url::Origin::Create(GURL("http://ads.search.com")), 0,
+                       network_isolation_key}})));
+  EXPECT_TRUE(
+      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
+
+  // A second call to PrepareForPageLoad using a provided prediction should not
+  // fire requests.
+  prediction = CreatePreconnectPrediction(
+      "search.com", true,
+      {{url::Origin::Create(GURL("http://cdn1.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn2.search.com")), 1,
+        network_isolation_key},
+       {url::Origin::Create(GURL("http://cdn3.search.com")), 1,
+        network_isolation_key}});
+  EXPECT_CALL(*mock_preconnect_manager_, StartProxy(_, _)).Times(0);
+  EXPECT_TRUE(predictor_->PrepareForPageLoad(
+      main_frame_url, HintOrigin::OPTIMIZATION_GUIDE, false, prediction));
 }
 
 }  // namespace predictors

chrome/browser/predictors/navigation_id.cc

@@ -52,4 +52,8 @@ bool NavigationID::operator==(const NavigationID& rhs) const {
   return tab_id == rhs.tab_id && main_frame_url == rhs.main_frame_url;
 }
 
+bool NavigationID::operator!=(const NavigationID& rhs) const {
+  return !(*this == rhs);
+}
+
 }  // namespace predictors

chrome/browser/predictors/navigation_id.h

@@ -29,6 +29,7 @@ struct NavigationID {
 
   bool operator<(const NavigationID& rhs) const;
   bool operator==(const NavigationID& rhs) const;
+  bool operator!=(const NavigationID& rhs) const;
 
   // Returns true iff the tab_id is valid and the Main frame URL is set.
   bool is_valid() const;

chrome/browser/predictors/preconnect_manager.cc

@@ -82,18 +82,19 @@ PreconnectManager::~PreconnectManager() = default;
 void PreconnectManager::Start(const GURL& url,
                               std::vector<PreconnectRequest> requests) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  const std::string host = url.host();
-  if (preresolve_info_.find(host) != preresolve_info_.end())
-    return;
-
-  auto iterator_and_whether_inserted = preresolve_info_.emplace(
-      host, std::make_unique<PreresolveInfo>(url, requests.size()));
-  PreresolveInfo* info = iterator_and_whether_inserted.first->second.get();
+  PreresolveInfo* info;
+  if (preresolve_info_.find(url) == preresolve_info_.end()) {
+    auto iterator_and_whether_inserted = preresolve_info_.emplace(
+        url, std::make_unique<PreresolveInfo>(url, requests.size()));
+    info = iterator_and_whether_inserted.first->second.get();
+  } else {
+    info = preresolve_info_.find(url)->second.get();
+    info->queued_count += requests.size();
+  }
 
-  for (auto request_it = requests.begin(); request_it != requests.end();
-       ++request_it) {
+  for (auto& request : requests) {
     PreresolveJobId job_id = preresolve_jobs_.Add(
-        std::make_unique<PreresolveJob>(std::move(*request_it), info));
+        std::make_unique<PreresolveJob>(std::move(request), info));
     queued_jobs_.push_back(job_id);
   }
 
@@ -147,7 +148,7 @@ void PreconnectManager::StartPreconnectUrl(
 
 void PreconnectManager::Stop(const GURL& url) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto it = preresolve_info_.find(url.host());
+  auto it = preresolve_info_.find(url);
   if (it == preresolve_info_.end()) {
     return;
   }
@@ -315,7 +316,7 @@ void PreconnectManager::FinishPreresolveJob(PreresolveJobId job_id,
 void PreconnectManager::AllPreresolvesForUrlFinished(PreresolveInfo* info) {
   DCHECK(info);
   DCHECK(info->is_done());
-  auto it = preresolve_info_.find(info->url.host());
+  auto it = preresolve_info_.find(info->url);
   DCHECK(it != preresolve_info_.end());
   DCHECK(info == it->second.get());
   if (delegate_)

chrome/browser/predictors/preconnect_manager.h

@@ -211,7 +211,7 @@ class PreconnectManager {
   Profile* const profile_;
   std::list<PreresolveJobId> queued_jobs_;
   PreresolveJobMap preresolve_jobs_;
-  std::map<std::string, std::unique_ptr<PreresolveInfo>> preresolve_info_;
+  std::map<GURL, std::unique_ptr<PreresolveInfo>> preresolve_info_;
   size_t inflight_preresolves_count_ = 0;
 
   // Only used in tests.

chrome/browser/predictors/preconnect_manager_unittest.cc

@@ -644,8 +644,8 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentMainFrameUrls) {
                                             network_isolation_key2, net::OK);
 }
 
-// Checks that the PreconnectManager handles no more than one URL per host
-// simultaneously.
+// Checks that the PreconnectManager queues up preconnect requests for URLs
+// with same host.
 TEST_F(PreconnectManagerTest, TestTwoConcurrentSameHostMainFrameUrls) {
   GURL main_frame_url1("http://google.com/search?query=cats");
   net::NetworkIsolationKey network_isolation_key1 =
@@ -663,8 +663,8 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentSameHostMainFrameUrls) {
   preconnect_manager_->Start(
       main_frame_url1,
       {PreconnectRequest(origin_to_preconnect1, 1, network_isolation_key1)});
-  // This suggestion should be dropped because the PreconnectManager already has
-  // a job for the "google.com" host.
+  EXPECT_CALL(*mock_network_context_,
+              ResolveHostProxy(origin_to_preconnect2.host()));
   preconnect_manager_->Start(
       main_frame_url2,
       {PreconnectRequest(origin_to_preconnect2, 1, network_isolation_key2)});
@@ -676,6 +676,13 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentSameHostMainFrameUrls) {
   EXPECT_CALL(*mock_delegate_, PreconnectFinishedProxy(main_frame_url1));
   mock_network_context_->CompleteHostLookup(origin_to_preconnect1.host(),
                                             network_isolation_key1, net::OK);
+  EXPECT_CALL(
+      *mock_network_context_,
+      PreconnectSockets(1, origin_to_preconnect2.GetURL(),
+                        true /* allow credentials */, network_isolation_key2));
+  EXPECT_CALL(*mock_delegate_, PreconnectFinishedProxy(main_frame_url2));
+  mock_network_context_->CompleteHostLookup(origin_to_preconnect2.host(),
+                                            network_isolation_key2, net::OK);
 }
 
 TEST_F(PreconnectManagerTest, TestStartPreresolveHost) {

chrome/browser/predictors/predictors_features.cc

@@ -26,4 +26,9 @@ const base::Feature kLoadingPredictorDisregardAlwaysAccessesNetwork{
     "LoadingPredictorDisregardAlwaysAccessesNetwork",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Modifies loading predictor so that it can also use predictions coming from
+// the optimization guide.
+const base::Feature kLoadingPredictorUseOptimizationGuide{
+    "LoadingPredictorUseOptimizationGuide", base::FEATURE_DISABLED_BY_DEFAULT};
+
 }  // namespace features

chrome/browser/predictors/predictors_features.h

@@ -20,6 +20,8 @@ extern const base::Feature kLoadingPreconnectToRedirectTarget;
 
 extern const base::Feature kLoadingPredictorDisregardAlwaysAccessesNetwork;
 
+extern const base::Feature kLoadingPredictorUseOptimizationGuide;
+
 }  // namespace features
 
 #endif  // CHROME_BROWSER_PREDICTORS_PREDICTORS_FEATURES_H_