Navigation Predictor: Preconnect on tab foreground

When the tab is brought to the foreground, preconnect to the
origin of the main frame request. This preconnection is done
at most once, and is helpful in case user clicks on a link.

Bug: 908725
Change-Id: Ic4c35c881e6b1a99bdccaac1bbfd585cfab8cb48
Reviewed-on: https://chromium-review.googlesource.com/c/1352490Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611389}

chrome/browser/navigation_predictor/navigation_predictor.cc

@@ -18,12 +18,27 @@
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/site_instance.h"
+#include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "third_party/blink/public/common/features.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
+namespace {
+
+content::RenderFrameHost* GetMainFrame(content::RenderFrameHost* rfh) {
+  // Don't use rfh->GetRenderViewHost()->GetMainFrame() here because
+  // RenderViewHost is being deprecated and because in OOPIF,
+  // RenderViewHost::GetMainFrame() returns nullptr for child frames hosted in a
+  // different process from the main frame.
+  while (rfh->GetParent() != nullptr)
+    rfh = rfh->GetParent();
+  return rfh;
+}
+
+}  // namespace
+
 struct NavigationPredictor::NavigationScore {
   NavigationScore(const GURL& url,
                   size_t area_rank,
@@ -96,10 +111,19 @@ NavigationPredictor::NavigationPredictor(
       is_low_end_device_(base::SysInfo::IsLowEndDevice()) {
   DCHECK(browser_context_);
   DETACH_FROM_SEQUENCE(sequence_checker_);
+
+  if (render_frame_host != GetMainFrame(render_frame_host))
+    return;
+
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  current_visibility_ = web_contents->GetVisibility();
+  Observe(web_contents);
 }
 
 NavigationPredictor::~NavigationPredictor() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  Observe(nullptr);
 }
 
 void NavigationPredictor::Create(
@@ -185,6 +209,43 @@ void NavigationPredictor::RecordActionAccuracyOnClick(
   return;
 }
 
+void NavigationPredictor::OnVisibilityChanged(content::Visibility visibility) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Check if the visibility changed from HIDDEN to VISIBLE. Since navigation
+  // predictor is currently restriced to Android, it is okay to disregard the
+  // occluded state.
+  if (current_visibility_ != content::Visibility::HIDDEN ||
+      visibility != content::Visibility::VISIBLE) {
+    current_visibility_ = visibility;
+    return;
+  }
+
+  current_visibility_ = visibility;
+
+  // Previously, the visibility was HIDDEN, and now it is VISIBLE implying that
+  // the web contents that was fully hidden is now fully visible.
+  base::Optional<url::Origin> preconnect_origin;
+  if (prefetch_url_) {
+    // Preconnect to the origin of the prefetch URL.
+    preconnect_origin = url::Origin::Create(prefetch_url_.value());
+  }
+
+  if (!preconnect_origin)
+    return;
+
+  std::string action_histogram_name =
+      source_is_default_search_engine_page_
+          ? "NavigationPredictor.OnDSE.ActionTaken"
+          : "NavigationPredictor.OnNonDSE.ActionTaken";
+  base::UmaHistogramEnumeration(action_histogram_name,
+                                Action::kPreconnectOnVisibilityChange);
+
+  // To keep the overhead as low, Pre* action on tab foreground is taken at most
+  // once per page.
+  Observe(nullptr);
+}
+
 SiteEngagementService* NavigationPredictor::GetEngagementService() const {
   Profile* profile = Profile::FromBrowserContext(browser_context_);
   SiteEngagementService* service = SiteEngagementService::Get(profile);

chrome/browser/navigation_predictor/navigation_predictor.h

@@ -13,6 +13,8 @@
 #include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
+#include "content/public/browser/visibility.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "third_party/blink/public/mojom/loader/navigation_predictor.mojom.h"
 
@@ -31,7 +33,8 @@ class TemplateURLService;
 // This class gathers metrics of anchor elements from both renderer process
 // and browser process. Then it uses these metrics to make predictions on what
 // are the most likely anchor elements that the user will click.
-class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost {
+class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost,
+                            public content::WebContentsObserver {
  public:
   // |render_frame_host| is the host associated with the render frame. It is
   // used to retrieve metrics at the browser side.
@@ -52,7 +55,8 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost {
     kPreresolve = 2,
     kPreconnect = 3,
     kPrefetch = 4,
-    kMaxValue = kPrefetch,
+    kPreconnectOnVisibilityChange = 5,
+    kMaxValue = kPreconnectOnVisibilityChange,
   };
 
   // Enum describing the accuracy of actions taken by the navigation predictor.
@@ -146,6 +150,9 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost {
   // |target_url| is the URL navigated to by the user.
   void RecordActionAccuracyOnClick(const GURL& target_url) const;
 
+  // content::WebContentsObserver:
+  void OnVisibilityChanged(content::Visibility visibility) override;
+
   // Used to get keyed services.
   content::BrowserContext* const browser_context_;
 
@@ -184,6 +191,9 @@ class NavigationPredictor : public blink::mojom::AnchorElementMetricsHost {
   // predict the next navigation) is a page from user's default search engine.
   bool source_is_default_search_engine_page_ = false;
 
+  // Current visibility state of the web contents.
+  content::Visibility current_visibility_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(NavigationPredictor);

chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

@@ -265,6 +265,80 @@ IN_PROC_BROWSER_TEST_F(NavigationPredictorBrowserTest,
       1);
 }
 
+// Disabled because it fails when SingleProcessMash feature is enabled. Since
+// Navigation Predictor is not going to be enabled on Chrome OS, disabling the
+// browser test on that platform is fine.
+#if defined(OS_CHROMEOS)
+#define DISABLE_ON_CHROMEOS(x) DISABLED_##x
+#else
+#define DISABLE_ON_CHROMEOS(x) x
+#endif
+
+// Simulate a click at the anchor element.
+// Test that the action accuracy is properly recorded.
+// User clicks on an anchor element that points to a origin different than the
+// origin of the URL prefetched.
+IN_PROC_BROWSER_TEST_F(
+    NavigationPredictorBrowserTest,
+    DISABLE_ON_CHROMEOS(ActionAccuracy_DifferentOrigin_VisibilityChanged)) {
+  base::HistogramTester histogram_tester;
+
+  const GURL& url = GetTestURL("/page_with_same_host_anchor_element.html");
+  ui_test_utils::NavigateToURL(browser(), url);
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(content::ExecuteScript(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "document.getElementById('google').click();"));
+  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);
+
+  histogram_tester.ExpectTotalCount(
+      "AnchorElementMetrics.Clicked.HrefEngagementScore2", 1);
+
+  histogram_tester.ExpectUniqueSample(
+      "NavigationPredictor.OnNonDSE.AccuracyActionTaken",
+      NavigationPredictor::ActionAccuracy::
+          kPrefetchActionClickToDifferentOrigin,
+      1);
+
+  // Change to visibile.
+  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);
+
+  browser()->tab_strip_model()->GetActiveWebContents()->WasShown();
+  histogram_tester.ExpectTotalCount("NavigationPredictor.OnNonDSE.ActionTaken",
+                                    2);
+  histogram_tester.ExpectBucketCount(
+      "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);
+  histogram_tester.ExpectBucketCount(
+      "NavigationPredictor.OnNonDSE.ActionTaken",
+      NavigationPredictor::Action::kPreconnectOnVisibilityChange, 1);
+}
+
 // Simulate a click at the anchor element.
 // Test that the action accuracy is properly recorded.
 // User clicks on an anchor element that points to same URL as the URL
@@ -506,7 +580,7 @@ IN_PROC_BROWSER_TEST_F(NavigationPredictorBrowserTest,
 // Tests that the browser only receives anchor elements that are in the
 // viewport, and from anchor elements whose target differ from document URL
 // by one digit.
-IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
+IN_PROC_BROWSER_TEST_F(NavigationPredictorBrowserTest,
                        ViewportOnlyAndUrlIncrementByOne) {
   base::HistogramTester histogram_tester;
 
@@ -514,15 +588,9 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
   ui_test_utils::NavigateToURL(browser(), url);
   base::RunLoop().RunUntilIdle();
 
-  if (base::FeatureList::IsEnabled(
-          blink::features::kRecordAnchorMetricsVisible)) {
-    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);
-  } else {
-    histogram_tester.ExpectTotalCount(
-        "AnchorElementMetrics.Visible.NumberOfAnchorElements", 0);
-  }
+  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);
 }

tools/metrics/histograms/enums.xml

@@ -34607,8 +34607,10 @@ Called by update_use_counter_css.py.-->
   <int value="0" label="Unknown"/>
   <int value="1" label="None"/>
   <int value="2" label="A host was preresolved"/>
-  <int value="3" label="A host was preconnected to"/>
+  <int value="3" label="An origin was preconnected to at onLoad"/>
   <int value="4" label="A URL was prefetched"/>
+  <int value="5"
+      label="An origin was preconnected to when tab was foregrounded"/>
 </enum>
 
 <enum name="NavigationScheme">