Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

Change-Id: I0fb183d75997c702585b294da59016131b254683
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1365076
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615163}(cherry picked from commit 1e0b2e8)
Reviewed-on: https://chromium-review.googlesource.com/c/1374967
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#317}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}

chrome/browser/navigation_predictor/navigation_predictor.cc

@@ -13,6 +13,8 @@
 #include "base/optional.h"
 #include "base/system/sys_info.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/predictors/loading_predictor.h"
+#include "chrome/browser/predictors/loading_predictor_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "components/search_engines/template_url_service.h"
@@ -115,7 +117,12 @@ NavigationPredictor::NavigationPredictor(
       preconnect_origin_score_threshold_(base::GetFieldTrialParamByFeatureAsInt(
           blink::features::kRecordAnchorMetricsVisible,
           "preconnect_origin_score_threshold",
-          0))
+          0)),
+      same_origin_preconnecting_allowed_(
+          base::GetFieldTrialParamByFeatureAsBool(
+              blink::features::kRecordAnchorMetricsVisible,
+              "same_origin_preconnecting_allowed",
+              false))
 #ifdef OS_ANDROID
       ,
       application_status_listener_(
@@ -276,20 +283,40 @@ void NavigationPredictor::OnVisibilityChanged(content::Visibility visibility) {
 
 void NavigationPredictor::TakeActionNowOnTabOrAppVisibilityChange(
     Action log_action) {
-  base::Optional<url::Origin> preconnect_origin;
-  if (prefetch_url_) {
+  MaybePreconnectNow(log_action);
+}
+
+void NavigationPredictor::MaybePreconnectNow(Action log_action) {
+  base::Optional<url::Origin> preconnect_origin = preconnect_origin_;
+
+  if (prefetch_url_ && !preconnect_origin) {
     // Preconnect to the origin of the prefetch URL.
     preconnect_origin = url::Origin::Create(prefetch_url_.value());
   }
 
   if (!preconnect_origin)
     return;
+  if (preconnect_origin->scheme() != url::kHttpScheme &&
+      preconnect_origin->scheme() != url::kHttpsScheme) {
+    return;
+  }
 
   std::string action_histogram_name =
       source_is_default_search_engine_page_
           ? "NavigationPredictor.OnDSE.ActionTaken"
           : "NavigationPredictor.OnNonDSE.ActionTaken";
   base::UmaHistogramEnumeration(action_histogram_name, log_action);
+
+  if (!same_origin_preconnecting_allowed_)
+    return;
+
+  auto* loading_predictor = predictors::LoadingPredictorFactory::GetForProfile(
+      Profile::FromBrowserContext(browser_context_));
+  GURL preconnect_url_serialized(preconnect_origin->Serialize());
+  DCHECK(preconnect_url_serialized.is_valid());
+  loading_predictor->PrepareForPageLoad(
+      preconnect_url_serialized, predictors::HintOrigin::NAVIGATION_PREDICTOR,
+      true);
 }
 
 SiteEngagementService* NavigationPredictor::GetEngagementService() const {
@@ -737,7 +764,7 @@ void NavigationPredictor::MaybeTakeActionOnLoad(
       GetOriginToPreconnect(document_origin, sorted_navigation_scores);
   if (preconnect_origin_.has_value()) {
     DCHECK_EQ(document_origin.host(), preconnect_origin_->host());
-    base::UmaHistogramEnumeration(action_histogram_name, Action::kPreconnect);
+    MaybePreconnectNow(Action::kPreconnect);
     return;
   }
 
@@ -902,6 +929,9 @@ void NavigationPredictor::OnApplicationStateChange(
     base::android::ApplicationState application_state) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (!application_status_listener_)
+    return;
+
   if (application_state_ !=
           base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES ||
       application_state !=
@@ -916,7 +946,9 @@ void NavigationPredictor::OnApplicationStateChange(
   TakeActionNowOnTabOrAppVisibilityChange(Action::kPreconnectOnAppForeground);
 
   // To keep the overhead as low, Pre* action on app foreground is taken at most
-  // once per page.
-  application_status_listener_.reset();
+  // once per page. Stop listening to application change events only if
+  // OnLoad() has been fired implying that prediction computation has finished.
+  if (document_loaded_timing_ != base::TimeTicks())
+    application_status_listener_.reset();
 }
 #endif  // OS_ANDROID

chrome/browser/navigation_predictor/navigation_predictor.h

@@ -183,6 +183,9 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost,
   // |log_action| should be set to the reason why this method was called.
   void TakeActionNowOnTabOrAppVisibilityChange(Action log_action);
 
+  // MaybePreconnectNow preconnects to an origin server if it's allowed.
+  void MaybePreconnectNow(Action log_action);
+
   // Used to get keyed services.
   content::BrowserContext* const browser_context_;
 
@@ -223,6 +226,9 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost,
   // they are not comparable.
   const int preconnect_origin_score_threshold_;
 
+  // True if |this| is allowed to preconnect to same origin hosts.
+  const bool same_origin_preconnecting_allowed_;
+
   // Timing of document loaded and last click.
   base::TimeTicks document_loaded_timing_;
   base::TimeTicks last_click_timing_;

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -20,6 +20,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/embedded_test_server_connection_listener.h"
 #include "third_party/blink/public/common/features.h"
 #include "url/gurl.h"
 
@@ -44,6 +45,44 @@ void RetryForHistogramUntilCountReached(base::HistogramTester* histogram_tester,
   }
 }
 
+// Gets notified by the EmbeddedTestServer on incoming connections being
+// accepted or read from.
+class ConnectionListener
+    : public net::test_server::EmbeddedTestServerConnectionListener {
+ public:
+  ConnectionListener() = default;
+
+  ~ConnectionListener() override = default;
+
+  void ResetAcceptedSocketCount() { accepted_socket_count_ = 0; }
+
+  size_t accepted_socket_count() const { return accepted_socket_count_; }
+
+  void SetOnAcceptedSocketCallback(
+      base::OnceCallback<void()> on_accepted_socket_callback) {
+    on_accepted_socket_callback_ = std::move(on_accepted_socket_callback);
+  }
+
+ private:
+  // Gets called from the EmbeddedTestServer thread to be notified that
+  // a connection was accepted.
+  void AcceptedSocket(const net::StreamSocket& connection) override {
+    ++accepted_socket_count_;
+    if (on_accepted_socket_callback_)
+      std::move(on_accepted_socket_callback_).Run();
+  }
+
+  // Gets called from the EmbeddedTestServer thread to be notified that
+  // a connection was read from.
+  void ReadFromSocket(const net::StreamSocket& connection, int rv) override {}
+
+  size_t accepted_socket_count_ = 0;
+
+  base::OnceCallback<void()> on_accepted_socket_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectionListener);
+};
+
 }  // namespace
 
 class NavigationPredictorBrowserTest
@@ -61,6 +100,7 @@ class NavigationPredictorBrowserTest
   void SetUp() override {
     https_server_.reset(
         new net::EmbeddedTestServer(net::EmbeddedTestServer::TYPE_HTTPS));
+    https_server_->SetConnectionListener(&https_connection_listener_);
     https_server_->ServeFilesFromSourceDirectory(
         "chrome/test/data/navigation_predictor");
     ASSERT_TRUE(https_server_->Start());
@@ -79,6 +119,11 @@ class NavigationPredictorBrowserTest
     host_resolver()->ClearRules();
   }
 
+  void FlushAllSocketsAndConnections() {
+    http_server_->FlushAllSocketsAndConnectionsOnUIThread();
+    https_server_->FlushAllSocketsAndConnectionsOnUIThread();
+  }
+
   const GURL GetTestURL(const char* file) const {
     return https_server_->GetURL(file);
   }
@@ -87,11 +132,35 @@ class NavigationPredictorBrowserTest
     return http_server_->GetURL(file);
   }
 
+  void ResetAcceptedHttpsSocketCount() {
+    https_connection_listener_.ResetAcceptedSocketCount();
+  }
+
+  void WaitUntilHttpsSocketAccepted() {
+    if (accepted_socket_count() > 0)
+      return;
+
+    base::RunLoop open_loop;
+    auto open_callback = base::BindOnce(
+        [](base::OnceClosure quit_closure) { std::move(quit_closure).Run(); },
+        open_loop.QuitClosure());
+
+    https_connection_listener_.SetOnAcceptedSocketCallback(
+        std::move(open_callback));
+    open_loop.Run();
+  }
+
+  size_t accepted_socket_count() const {
+    return https_connection_listener_.accepted_socket_count();
+  }
+
  private:
   std::unique_ptr<net::EmbeddedTestServer> http_server_;
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
   base::test::ScopedFeatureList feature_list_;
 
+  ConnectionListener https_connection_listener_;
+
   DISALLOW_COPY_AND_ASSIGN(NavigationPredictorBrowserTest);
 };
 
@@ -310,6 +379,9 @@ IN_PROC_BROWSER_TEST_F(
           kPrefetchActionClickToDifferentOrigin,
       1);
 
+  FlushAllSocketsAndConnections();
+  ResetAcceptedHttpsSocketCount();
+
   // Change to visibile.
   browser()->tab_strip_model()->GetActiveWebContents()->WasShown();
   histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
@@ -326,6 +398,69 @@ IN_PROC_BROWSER_TEST_F(
       "NavigationPredictor.OnNonDSE.ActionTaken",
       NavigationPredictor::Action::kPreconnectOnVisibilityChange, 1);
 
+  // Hiding and showing the tab again should not cause change in histograms
+  // since Pre* on tab foreground is done at most once per page.
+  browser()->tab_strip_model()->GetActiveWebContents()->WasHidden();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    2);
+  browser()->tab_strip_model()->GetActiveWebContents()->WasShown();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    2);
+
+  EXPECT_EQ(0u, accepted_socket_count());
+}
+
+// Test that the action accuracy is properly recorded and when same origin
+// preconnections are enabled, then navigation predictor initiates the
+// preconnection.
+IN_PROC_BROWSER_TEST_F(
+    NavigationPredictorBrowserTest,
+    DISABLE_ON_CHROMEOS(
+        ActionAccuracy_DifferentOrigin_VisibilityChangedPreconnectEnabled)) {
+  std::map<std::string, std::string> parameters;
+  base::test::ScopedFeatureList feature_list;
+  parameters["same_origin_preconnecting_allowed"] = "true";
+  feature_list.InitAndEnableFeatureWithParameters(
+      blink::features::kRecordAnchorMetricsVisible, parameters);
+
+  base::HistogramTester histogram_tester;
+
+  const GURL& url = GetTestURL("/page_with_same_host_anchor_element.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  histogram_tester.ExpectUniqueSample(
+      "AnchorElementMetrics.Visible.NumberOfAnchorElements", 2, 1);
+  // Same document anchor element should be removed after merge.
+  histogram_tester.ExpectUniqueSample(
+      "AnchorElementMetrics.Visible.NumberOfAnchorElementsAfterMerge", 2, 1);
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.ActionTaken",
+      NavigationPredictor::Action::kPrefetch, 1);
+
+  // Change to visible.
+  browser()->tab_strip_model()->GetActiveWebContents()->WasShown();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    1);
+
+  browser()->tab_strip_model()->GetActiveWebContents()->WasHidden();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    1);
+
+  FlushAllSocketsAndConnections();
+  ResetAcceptedHttpsSocketCount();
+
+  browser()->tab_strip_model()->GetActiveWebContents()->WasShown();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    2);
+  histogram_tester.ExpectBucketCount(
+      "NavigationPredictor.OnNonDSE.ActionTaken",
+      NavigationPredictor::Action::kPreconnectOnVisibilityChange, 1);
+
+  WaitUntilHttpsSocketAccepted();
+
+  ResetAcceptedHttpsSocketCount();
+
   // Hiding and showing the tab again should not cause change in histograms
   // since Pre* on tab foreground is done at most once per page.
   browser()->tab_strip_model()->GetActiveWebContents()->WasHidden();

chrome/browser/navigation_predictor/navigation_predictor_unittest.cc

@@ -69,14 +69,6 @@ class NavigationPredictorTest : public ChromeRenderViewHostTestHarness {
   NavigationPredictorTest() = default;
   ~NavigationPredictorTest() override = default;
 
-  void SetUp() override {
-    SetupFieldTrial(base::nullopt /* preconnect_origin_score_threshold */,
-                    base::nullopt /* prefetch_url_score_threshold */);
-    ChromeRenderViewHostTestHarness::SetUp();
-    predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
-        mojo::MakeRequest(&predictor_service_), main_rfh());
-  }
-
   // Helper function to generate mojom metrics.
   blink::mojom::AnchorElementMetricsPtr CreateMetricsPtr(
       const std::string& source_url,
@@ -106,8 +98,18 @@ class NavigationPredictorTest : public ChromeRenderViewHostTestHarness {
   }
 
  protected:
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
+        mojo::MakeRequest(&predictor_service_), main_rfh());
+  }
+
   void SetupFieldTrial(base::Optional<int> preconnect_origin_score_threshold,
                        base::Optional<int> prefetch_url_score_threshold) {
+    if (field_trial_initiated_)
+      return;
+
+    field_trial_initiated_ = true;
     const std::string kTrialName = "TrialFoo2";
     const std::string kGroupName = "GroupFoo2";  // Value not used
 
@@ -129,6 +131,8 @@ class NavigationPredictorTest : public ChromeRenderViewHostTestHarness {
 
  private:
   base::test::ScopedFeatureList scoped_feature_list;
+
+  bool field_trial_initiated_ = false;
 };
 
 }  // namespace
@@ -387,10 +391,12 @@ TEST_F(NavigationPredictorTest,
 // disabled by setting |prefetch_url_score_threshold| to too high.
 class NavigationPredictorPrefetchDisabledTest : public NavigationPredictorTest {
  public:
-  void SetUp() override {
+  NavigationPredictorPrefetchDisabledTest() {
     SetupFieldTrial(0 /* preconnect_origin_score_threshold */,
                     101 /* prefetch_url_score_threshold */);
+  }
 
+  void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
     predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
         mojo::MakeRequest(&predictor_service_), main_rfh());
@@ -481,10 +487,12 @@ TEST_F(NavigationPredictorPrefetchDisabledTest,
 class NavigationPredictorPreconnectPrefetchDisabledTest
     : public NavigationPredictorTest {
  public:
-  void SetUp() override {
+  NavigationPredictorPreconnectPrefetchDisabledTest() {
     SetupFieldTrial(101 /* preconnect_origin_score_threshold */,
                     101 /* prefetch_url_score_threshold */);
+  }
 
+  void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
     predictor_service_helper_ = std::make_unique<TestNavigationPredictor>(
         mojo::MakeRequest(&predictor_service_), main_rfh());

chrome/browser/predictors/loading_predictor_config.h

@@ -30,7 +30,7 @@ bool IsLoadingPredictorEnabled(Profile* profile);
 bool IsPreconnectAllowed(Profile* profile);
 
 // Indicates what caused the page load hint.
-enum class HintOrigin { NAVIGATION, EXTERNAL, OMNIBOX };
+enum class HintOrigin { NAVIGATION, EXTERNAL, OMNIBOX, NAVIGATION_PREDICTOR };
 
 // Represents the config for the Loading predictor.
 struct LoadingPredictorConfig {