Add FromGWS variants to the AbortTiming metrics

This patch was split off from the larger CL authored here:
https://codereview.chromium.org/1901303004/

BUG=605259

Review URL: https://codereview.chromium.org/1919193003

Cr-Commit-Position: refs/heads/master@{#389903}

chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer.h

@@ -12,6 +12,13 @@
 namespace internal {
 // Exposed for tests.
 extern const char kHistogramFromGWSFirstTextPaint[];
+extern const char kHistogramFromGWSAbortStopBeforePaint[];
+extern const char kHistogramFromGWSAbortStopBeforeCommit[];
+extern const char kHistogramFromGWSAbortCloseBeforePaint[];
+extern const char kHistogramFromGWSAbortCloseBeforeCommit[];
+extern const char kHistogramFromGWSAbortNewNavigationBeforePaint[];
+extern const char kHistogramFromGWSAbortUnknownNavigationBeforeCommit[];
+
 }  // namespace internal
 
 // FromGWSPageLoadMetricsLogger is a peer class to
@@ -25,9 +32,8 @@ class FromGWSPageLoadMetricsLogger {
  public:
   FromGWSPageLoadMetricsLogger() {}
 
-  void set_previously_committed_url(const GURL& url) {
-    previously_committed_url_ = url;
-  }
+  void SetPreviouslyCommittedUrl(const GURL& url);
+  void SetProvisionalUrl(const GURL& url);
 
   void set_navigation_initiated_via_link(bool navigation_initiated_via_link) {
     navigation_initiated_via_link_ = navigation_initiated_via_link;
@@ -42,6 +48,7 @@ class FromGWSPageLoadMetricsLogger {
   static bool IsGoogleSearchResultUrl(const GURL& url);
   static bool IsGoogleRedirectorUrl(const GURL& url);
   static bool IsGoogleSearchRedirectorUrl(const GURL& url);
+  bool ShouldLogMetrics(const GURL& url);
 
   // Whether the given query string contains the given component. The query
   // parameter should contain the query string of a URL (the portion following
@@ -58,21 +65,25 @@ class FromGWSPageLoadMetricsLogger {
   static bool QueryContainsComponentPrefix(const base::StringPiece query,
                                            const base::StringPiece component);
 
-  // Whether metrics should be logged based on state provided via setters and
-  // the given committed_url.
-  bool ShouldLogMetrics(const GURL& committed_url) const;
-
  private:
-  GURL previously_committed_url_;
+  bool previously_committed_url_is_search_results_ = false;
+  bool previously_committed_url_is_search_redirector_ = false;
   bool navigation_initiated_via_link_ = false;
+  bool provisional_url_is_search_results_or_google_redirector_ = false;
+
+  static bool IsUrlFromGWS(const GURL& url);
 
   // Common helper for QueryContainsComponent and QueryContainsComponentPrefix.
   static bool QueryContainsComponentHelper(const base::StringPiece query,
                                            const base::StringPiece component,
                                            bool component_is_prefix);
 
-  void LogMetrics(const page_load_metrics::PageLoadTiming& timing,
-                  const page_load_metrics::PageLoadExtraInfo& extra_info);
+  void LogCommittedMetrics(
+      const page_load_metrics::PageLoadTiming& timing,
+      const page_load_metrics::PageLoadExtraInfo& extra_info);
+  void LogProvisionalMetrics(
+      const page_load_metrics::PageLoadTiming& timing,
+      const page_load_metrics::PageLoadExtraInfo& extra_info);
 
   DISALLOW_COPY_AND_ASSIGN(FromGWSPageLoadMetricsLogger);
 };

chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc

@@ -10,6 +10,7 @@
 
 namespace {
 const char kExampleUrl[] = "http://www.example.com/";
+const char kGoogleSearchResultsUrl[] = "https://www.google.com/webhp?q=d";
 }  // namespace
 
 class FromGWSPageLoadMetricsObserverTest
@@ -19,6 +20,11 @@ class FromGWSPageLoadMetricsObserverTest
     tracker->AddObserver(
         base::WrapUnique(new FromGWSPageLoadMetricsObserver()));
   }
+  void SimulateTimingWithoutPaint() {
+    page_load_metrics::PageLoadTiming timing;
+    timing.navigation_start = base::Time::FromDoubleT(1);
+    SimulateTimingUpdate(timing);
+  }
 };
 
 class FromGWSPageLoadMetricsLoggerTest : public testing::Test {};
@@ -350,6 +356,171 @@ TEST_F(FromGWSPageLoadMetricsObserverTest,
       timing.first_text_paint.InMilliseconds(), 1);
 }
 
+TEST_F(FromGWSPageLoadMetricsObserverTest, UnknownNavigationBeforeCommit) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  StartNavigation(GURL("http://example.test"));
+
+  // Simulate the user performing another navigation before commit.
+  StartNavigation(GURL("https://www.example.com"));
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortUnknownNavigationBeforeCommit, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, NewNavigationBeforePaint) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("http://example.test"));
+  SimulateTimingWithoutPaint();
+  // Simulate the user performing another navigation before paint.
+  NavigateAndCommit(GURL("https://www.example.com"));
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortNewNavigationBeforePaint, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, StopBeforeCommit) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  StartNavigation(GURL("http://example.test"));
+  // Simulate the user pressing the stop button.
+  web_contents()->Stop();
+  // Now close the tab. This will trigger logging for the prior navigation which
+  // was stopped above.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforeCommit, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, StopBeforeCommitNonSearch) {
+  NavigateAndCommit(GURL("http://google.com"));
+  StartNavigation(GURL("http://example.test"));
+  // Simulate the user pressing the stop button.
+  web_contents()->Stop();
+  // Now close the tab. This will trigger logging for the prior navigation which
+  // was stopped above.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforeCommit, 0);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, StopBeforeCommitSearchToSearch) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  StartNavigation(GURL("http://www.google.com/webhp?q=5"));
+  // Simulate the user pressing the stop button.
+  web_contents()->Stop();
+  // Now close the tab. This will trigger logging for the prior navigation which
+  // was stopped above.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforeCommit, 0);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, StopBeforePaint) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("http://example.test"));
+  SimulateTimingWithoutPaint();
+  // Simulate the user pressing the stop button.
+  web_contents()->Stop();
+  // Now close the tab. This will trigger logging for the prior navigation which
+  // was stopped above.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforePaint, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, StopBeforeCommitAndBeforePaint) {
+  // Commit the first navigation.
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("https://example.test"));
+  SimulateTimingWithoutPaint();
+  // Now start a second navigation, but don't commit it.
+  StartNavigation(GURL("https://www.google.com"));
+  // Simulate the user pressing the stop button. This should cause us to record
+  // stop metrics for the FromGWS committed load, too.
+  web_contents()->Stop();
+  // Simulate closing the tab.
+  DeleteContents();
+  // The second navigation was not from GWS.
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforeCommit, 0);
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforePaint, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, CloseBeforeCommit) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  StartNavigation(GURL("https://example.test"));
+  // Simulate closing the tab.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforeCommit, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, CloseBeforePaint) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("https://example.test"));
+  SimulateTimingWithoutPaint();
+  // Simulate closing the tab.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforePaint, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest,
+       AbortCloseBeforeCommitAndBeforePaint) {
+  // Commit the first navigation.
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("https://example.test"));
+  SimulateTimingWithoutPaint();
+  // Now start a second navigation, but don't commit it.
+  StartNavigation(GURL("https://example.test2"));
+  // Simulate closing the tab.
+  DeleteContents();
+  // The second navigation was not from GWS.
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforeCommit, 0);
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforePaint, 1);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest,
+       AbortStopBeforeCommitAndCloseBeforePaint) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  StartNavigation(GURL("https://example.test"));
+  // Simulate the user pressing the stop button.
+  web_contents()->Stop();
+  NavigateAndCommit(GURL("https://example.test2"));
+  SimulateTimingWithoutPaint();
+  // Simulate closing the tab.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortStopBeforeCommit, 1);
+  // The second navigation was from GWS, as GWS was the last committed URL.
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforePaint, 1);
+}
+
+// TODO(bmcquade, csharrison): add tests for reload, back/forward, and other
+// aborts.
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, NoAbortNewNavigationFromAboutURL) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("about:blank"));
+  NavigateAndCommit(GURL("https://www.example.com"));
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortNewNavigationBeforePaint, 0);
+}
+
+TEST_F(FromGWSPageLoadMetricsObserverTest, NoAbortNewNavigationAfterPaint) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_paint = base::TimeDelta::FromMicroseconds(1);
+  PopulateRequiredTimingFields(&timing);
+  NavigateAndCommit(GURL("https://example.test"));
+  SimulateTimingUpdate(timing);
+  NavigateAndCommit(GURL("https://example.test2"));
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortNewNavigationBeforePaint, 0);
+}
+
 TEST_F(FromGWSPageLoadMetricsLoggerTest, IsGoogleSearchHostname) {
   struct {
     bool expected_result;
@@ -527,31 +698,39 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, Basic) {
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NoPreviousPage) {
   FromGWSPageLoadMetricsLogger logger;
-  logger.set_previously_committed_url(GURL());
+  logger.SetPreviouslyCommittedUrl(GURL());
   logger.set_navigation_initiated_via_link(true);
   ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationNotInitiatedViaLink) {
   FromGWSPageLoadMetricsLogger logger;
-  logger.set_previously_committed_url(
+  logger.SetPreviouslyCommittedUrl(
       GURL("https://www.google.com/search?q=test"));
   logger.set_navigation_initiated_via_link(false);
   ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
 }
 
-TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearchToSearch) {
+TEST_F(FromGWSPageLoadMetricsLoggerTest,
+       ProvisionalFromGWSNotInitiatedViaLink) {
   FromGWSPageLoadMetricsLogger logger;
-  logger.set_previously_committed_url(
+  logger.SetProvisionalUrl(
       GURL("https://www.google.com/search?q=test"));
-  logger.set_navigation_initiated_via_link(true);
-  ASSERT_FALSE(
-      logger.ShouldLogMetrics(GURL("https://www.google.com/search?q=test")));
+  logger.set_navigation_initiated_via_link(false);
+  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+}
+
+TEST_F(FromGWSPageLoadMetricsLoggerTest,
+       ProvisionalNotFromGWSNotInitiatedViaLink) {
+  FromGWSPageLoadMetricsLogger logger;
+  logger.SetProvisionalUrl(GURL("https://example.test/"));
+  logger.set_navigation_initiated_via_link(false);
+  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearch) {
   FromGWSPageLoadMetricsLogger logger;
-  logger.set_previously_committed_url(
+  logger.SetPreviouslyCommittedUrl(
       GURL("https://www.google.com/search?q=test"));
   logger.set_navigation_initiated_via_link(true);
   ASSERT_TRUE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
@@ -559,7 +738,7 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearch) {
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearchRedirector) {
   FromGWSPageLoadMetricsLogger logger;
-  logger.set_previously_committed_url(
+  logger.SetPreviouslyCommittedUrl(
       GURL("https://www.google.com/url?source=web"));
   logger.set_navigation_initiated_via_link(true);
   ASSERT_TRUE(logger.ShouldLogMetrics(GURL(kExampleUrl)));

tools/metrics/histograms/histograms.xml

@@ -91815,6 +91815,27 @@ To add a new entry, add it with any value and run test to compute valid value.
   <affected-histogram name="PageLoad.Timing2.NavigationToFirstBackground"/>
 </histogram_suffixes>
 
+<histogram_suffixes name="PageLoadMetricsAbortClients" separator="."
+    ordering="prefix">
+  <suffix name="Clients.FromGWS2"
+      label="PageLoadMetrics that are a result of a navigation from a Google
+             web search"/>
+  <affected-histogram
+      name="PageLoad.AbortTiming.Close.AfterCommit.BeforePaint"/>
+  <affected-histogram name="PageLoad.AbortTiming.Close.BeforeCommit"/>
+  <affected-histogram
+      name="PageLoad.AbortTiming.ForwardBackNavigation.AfterCommit.BeforePaint"/>
+  <affected-histogram
+      name="PageLoad.AbortTiming.NewNavigation.AfterCommit.BeforePaint"/>
+  <affected-histogram name="PageLoad.AbortTiming.Other.BeforeCommit"/>
+  <affected-histogram
+      name="PageLoad.AbortTiming.Reload.AfterCommit.BeforePaint"/>
+  <affected-histogram name="PageLoad.AbortTiming.Stop.AfterCommit.BeforePaint"/>
+  <affected-histogram name="PageLoad.AbortTiming.Stop.BeforeCommit"/>
+  <affected-histogram
+      name="PageLoad.AbortTiming.UnknownNavigation.BeforeCommit"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="PageLoadMetricsClients" separator="."
     ordering="prefix">
   <suffix name="Clients.FromGWS"