Revert "Reland "Add support for loading predictor to use hints from optimization guide""

This reverts commit 9a31f5ab.

Reason for revert: Speculative revert for LoadingPredictorBrowserTestWithOptimizationGuide.UsesPredictionsFromOptimizationGuideIfAvailable failures

https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/3477

Original change's description:
> Reland "Add support for loading predictor to use hints from optimization guide"
> 
> This is a reland of eed7024d
> TBR=tbansal@chromium.org for chrome/browser/predictors/...
> 
> Original change's description:
> > 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}
> 
> Bug: 1050101
> Change-Id: I796d2bbd322fee835bfc42f3f3bc903b65554291
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2068313
> Commit-Queue: Sophie Chang <sophiechang@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#744088}

TBR=tbansal@chromium.org,sophiechang@chromium.org,ryansturm@chromium.org

Change-Id: If2d29382d644733504141aed662fe7c570cd8db8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1050101
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071178Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744133}

chrome/browser/predictors/loading_predictor.cc

@@ -70,48 +70,37 @@ LoadingPredictor::~LoadingPredictor() {
   DCHECK(shutdown_);
 }
 
-bool LoadingPredictor::PrepareForPageLoad(
-    const GURL& url,
-    HintOrigin origin,
-    bool preconnectable,
-    base::Optional<PreconnectPrediction> preconnect_prediction) {
+void LoadingPredictor::PrepareForPageLoad(const GURL& url,
+                                          HintOrigin origin,
+                                          bool preconnectable) {
   if (shutdown_)
-    return true;
+    return;
 
   if (origin == HintOrigin::OMNIBOX) {
     // Omnibox hints are lightweight and need a special treatment.
     HandleOmniboxHint(url, preconnectable);
-    return true;
+    return;
   }
 
+  if (active_hints_.find(url) != active_hints_.end())
+    return;
+
+  bool has_preconnect_prediction = false;
   PreconnectPrediction prediction;
-  bool has_local_preconnect_prediction =
+  has_preconnect_prediction =
       resource_prefetch_predictor_->PredictPreconnectOrigins(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 (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);
-  }
+  // Try to preconnect to the |url| even if the predictor has no
+  // prediction.
+  has_preconnect_prediction =
+      AddInitialUrlToPreconnectPrediction(url, &prediction);
 
-  // Return early if we do not have any preconnect requests.
-  if (prediction.requests.empty())
-    return false;
+  if (!has_preconnect_prediction)
+    return;
 
   ++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) {
@@ -154,15 +143,14 @@ void LoadingPredictor::Shutdown() {
   shutdown_ = true;
 }
 
-bool LoadingPredictor::OnNavigationStarted(const NavigationID& navigation_id) {
+void LoadingPredictor::OnNavigationStarted(const NavigationID& navigation_id) {
   if (shutdown_)
-    return true;
+    return;
 
   loading_data_collector()->RecordStartNavigation(navigation_id);
   CleanupAbandonedHintsAndNavigations(navigation_id);
   active_navigations_.emplace(navigation_id);
-  return PrepareForPageLoad(navigation_id.main_frame_url,
-                            HintOrigin::NAVIGATION);
+  PrepareForPageLoad(navigation_id.main_frame_url, HintOrigin::NAVIGATION);
 }
 
 void LoadingPredictor::OnNavigationFinished(

chrome/browser/predictors/loading_predictor.h

@@ -14,7 +14,6 @@
 
 #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"
@@ -50,15 +49,10 @@ class LoadingPredictor : public KeyedService,
   ~LoadingPredictor() override;
 
   // Hints that a page load is expected for |url|, with the hint coming from a
-  // 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,
+  // given |origin|. May trigger actions, such as prefetch and/or preconnect.
+  void PrepareForPageLoad(const GURL& url,
                           HintOrigin origin,
-                          bool preconnectable = false,
-                          base::Optional<PreconnectPrediction>
-                              preconnect_prediction = base::nullopt);
+                          bool preconnectable = false);
 
   // Indicates that a page load hint is no longer active.
   void CancelPageLoadHint(const GURL& url);
@@ -74,10 +68,7 @@ class LoadingPredictor : public KeyedService,
   // KeyedService:
   void Shutdown() override;
 
-  // 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 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

@@ -17,11 +17,8 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/task/post_task.h"
-#include "base/task/thread_pool/thread_pool_instance.h"
-#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/browser_features.h"
-#include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/browser/predictors/loading_predictor_factory.h"
@@ -33,10 +30,6 @@
 #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_constants.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"
@@ -92,29 +85,6 @@ GURL GetDataURLWithContent(const std::string& content) {
   return GURL(data_uri_content);
 }
 
-void RetryForHistogramUntilCountReached(
-    const base::HistogramTester& histogram_tester,
-    const std::string& histogram_name,
-    size_t count) {
-  while (true) {
-    base::ThreadPoolInstance::Get()->FlushForTesting();
-    base::RunLoop().RunUntilIdle();
-
-    content::FetchHistogramsFromChildProcesses();
-    SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
-
-    const std::vector<base::Bucket> buckets =
-        histogram_tester.GetAllSamples(histogram_name);
-    size_t total_count = 0;
-    for (const auto& bucket : buckets) {
-      total_count += bucket.count;
-    }
-    if (total_count >= count) {
-      break;
-    }
-  }
-}
-
 // Helper class to track and allow waiting for ResourcePrefetchPredictor
 // initialization. WARNING: OnPredictorInitialized event will not be fired if
 // ResourcePrefetchPredictor is initialized before the observer creation.
@@ -1518,155 +1488,4 @@ 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) {
-  base::HistogramTester histogram_tester;
-
-  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"));
-  RetryForHistogramUntilCountReached(
-      histogram_tester, optimization_guide::kLoadedHintLocalHistogramString, 1);
-  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,10 +47,7 @@ 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,
-
-  // Triggered by optimization guide.
-  OPTIMIZATION_GUIDE,
+  OMNIBOX_PRERENDER_FALLBACK
 };
 
 // Represents the config for the Loading predictor.

chrome/browser/predictors/loading_predictor_tab_helper.cc

@@ -4,17 +4,11 @@
 
 #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"
@@ -71,21 +65,10 @@ bool IsHandledNavigation(content::NavigationHandle* navigation_handle) {
 LoadingPredictorTabHelper::LoadingPredictorTabHelper(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents) {
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  auto* predictor = LoadingPredictorFactory::GetForProfile(profile);
+  auto* predictor = LoadingPredictorFactory::GetForProfile(
+      Profile::FromBrowserContext(web_contents->GetBrowserContext()));
   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;
@@ -103,36 +86,8 @@ 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;
 
-  auto navigation_id = NavigationID(web_contents(), navigation_handle->GetURL(),
-                                    navigation_handle->NavigationStart());
-  if (!navigation_id.is_valid())
-    return;
-  current_navigation_id_ = navigation_id;
+  predictor_->OnNavigationStarted(navigation_id);
 }
 
 void LoadingPredictorTabHelper::DidFinishNavigation(
@@ -144,9 +99,6 @@ 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());
@@ -219,62 +171,6 @@ 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,7 +6,6 @@
 #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"
 
@@ -14,15 +13,10 @@ 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.
@@ -37,8 +31,6 @@ 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(
@@ -60,25 +52,9 @@ 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,18 +158,6 @@ 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);
@@ -364,12 +352,11 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlToEmptyPrediction) {
                  std::vector<PreconnectRequest>(
                      {{url::Origin::Create(GURL("http://search.com")), 2,
                        CreateNetworkIsolationKey(main_frame_url)}})));
-  EXPECT_FALSE(
-      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::NAVIGATION));
+  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 contains the origin.
+// if the list already containts the origin.
 TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlMatchesPrediction) {
   GURL main_frame_url("http://search.com/kittens");
   net::NetworkIsolationKey network_isolation_key =
@@ -394,8 +381,7 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlMatchesPrediction) {
                        network_isolation_key},
                       {url::Origin::Create(GURL("http://ads.search.com")), 0,
                        network_isolation_key}})));
-  EXPECT_TRUE(
-      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
+  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
 }
 
 // Checks that the predictor adds an initial origin to a preconnect list if the
@@ -427,8 +413,7 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInitialUrlDoesntMatchPrediction) {
                        network_isolation_key},
                       {url::Origin::Create(GURL("http://ads.search.com")), 0,
                        network_isolation_key}})));
-  EXPECT_TRUE(
-      predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL));
+  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
 }
 
 // Checks that the predictor doesn't preconnect to a bad url.
@@ -436,140 +421,7 @@ TEST_F(LoadingPredictorPreconnectTest, TestAddInvalidInitialUrl) {
   GURL main_frame_url("file:///tmp/index.html");
   EXPECT_CALL(*mock_predictor_, PredictPreconnectOrigins(main_frame_url, _))
       .WillOnce(Return(false));
-  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));
+  predictor_->PrepareForPageLoad(main_frame_url, HintOrigin::EXTERNAL);
 }
 
 }  // namespace predictors

chrome/browser/predictors/navigation_id.cc

@@ -52,8 +52,4 @@ 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,7 +29,6 @@ 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,19 +82,18 @@ PreconnectManager::~PreconnectManager() = default;
 void PreconnectManager::Start(const GURL& url,
                               std::vector<PreconnectRequest> requests) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  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();
-  }
+  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();
 
-  for (auto& request : requests) {
+  for (auto request_it = requests.begin(); request_it != requests.end();
+       ++request_it) {
     PreresolveJobId job_id = preresolve_jobs_.Add(
-        std::make_unique<PreresolveJob>(std::move(request), info));
+        std::make_unique<PreresolveJob>(std::move(*request_it), info));
     queued_jobs_.push_back(job_id);
   }
 
@@ -148,7 +147,7 @@ void PreconnectManager::StartPreconnectUrl(
 
 void PreconnectManager::Stop(const GURL& url) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto it = preresolve_info_.find(url);
+  auto it = preresolve_info_.find(url.host());
   if (it == preresolve_info_.end()) {
     return;
   }
@@ -316,7 +315,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);
+  auto it = preresolve_info_.find(info->url.host());
   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<GURL, std::unique_ptr<PreresolveInfo>> preresolve_info_;
+  std::map<std::string, 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 queues up preconnect requests for URLs
-// with same host.
+// Checks that the PreconnectManager handles no more than one URL per host
+// simultaneously.
 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)});
-  EXPECT_CALL(*mock_network_context_,
-              ResolveHostProxy(origin_to_preconnect2.host()));
+  // This suggestion should be dropped because the PreconnectManager already has
+  // a job for the "google.com" host.
   preconnect_manager_->Start(
       main_frame_url2,
       {PreconnectRequest(origin_to_preconnect2, 1, network_isolation_key2)});
@@ -676,13 +676,6 @@ 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,9 +26,4 @@ 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,8 +20,6 @@ extern const base::Feature kLoadingPreconnectToRedirectTarget;
 
 extern const base::Feature kLoadingPredictorDisregardAlwaysAccessesNetwork;
 
-extern const base::Feature kLoadingPredictorUseOptimizationGuide;
-
 }  // namespace features
 
 #endif  // CHROME_BROWSER_PREDICTORS_PREDICTORS_FEATURES_H_