Refactor PageLoadMetricsObserver completion callback policy

chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc

Index: chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
diff --git a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
index 752bab5ad5fd096ac139572fdce313a47429eddf..17d44631c4c20fc0235bd02d9bc23cdc346e9503 100644
--- a/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
+++ b/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
@@ -674,6 +674,16 @@ TEST_F(FromGWSPageLoadMetricsObserverTest, CloseAfterInteraction) {
       internal::kHistogramFromGWSAbortCloseBeforeInteraction, 0);
 }
 
+TEST_F(FromGWSPageLoadMetricsObserverTest, Intent) {
+  NavigateAndCommit(GURL(kGoogleSearchResultsUrl));
+  NavigateAndCommit(GURL("intent://en.m.wikipedia.org/wiki/Test"));
+  SimulateTimingWithFirstPaint();
+  // Simulate closing the tab.
+  DeleteContents();
+  histogram_tester().ExpectTotalCount(
+      internal::kHistogramFromGWSAbortCloseBeforeInteraction, 0);
+}
+
 TEST_F(FromGWSPageLoadMetricsLoggerTest, IsGoogleSearchHostname) {
   struct {
     bool expected_result;
@@ -830,49 +840,35 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, QueryContainsComponentPrefix) {
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, Basic) {
   FromGWSPageLoadMetricsLogger logger;
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+  ASSERT_FALSE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NoPreviousPage) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL());
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+  ASSERT_FALSE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationNotInitiatedViaLink) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.set_navigation_initiated_via_link(false);
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
-}
-
-TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalNonHttpOrHttpsScheme) {

[Charlie Harrison, 2016/07/18 20:02:18]

Looks like the test you added doesn't cover the same cases as these deleted
tests.

[Bryan McQuade, 2016/07/19 12:55:38]

Ah, you mean it not being a provisional load? I added a provisional version of
the new test as well. I think the 2 intent:-based tests I removed were redundant
- they were both just testing different intent: URLs. So I didn't see a need to
replicate both of those provisional tests. I agree it's be good to have a
provisional version of the test so I added that. I also verified that if I
change the intent: URL to an https: URL, both tests fail, so these tests are
verifying that we ignore intent-based navigations.

-  FromGWSPageLoadMetricsLogger logger;
-  logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
-  logger.SetProvisionalUrl(GURL("intent://foo"));
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL::EmptyGURL()));
+  ASSERT_FALSE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalFromGWS) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.SetProvisionalUrl(GURL(kGoogleSearchResultsUrl));
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL::EmptyGURL()));
+  ASSERT_FALSE(logger.ShouldLogFailedProvisionalLoadMetrics());
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalNotFromGWS) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.SetProvisionalUrl(GURL(kExampleUrl));
-  ASSERT_TRUE(logger.ShouldLogMetrics(GURL::EmptyGURL()));
-}
-
-TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalIntent) {
-  FromGWSPageLoadMetricsLogger logger;
-  logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
-  logger.SetProvisionalUrl(GURL("intent://en.m.wikipedia.org/wiki/Test"));
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL::EmptyGURL()));
+  ASSERT_TRUE(logger.ShouldLogFailedProvisionalLoadMetrics());
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalIgnoredAfterCommit1) {
@@ -880,7 +876,8 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalIgnoredAfterCommit1) {
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.SetProvisionalUrl(GURL(kExampleUrl));
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL(kGoogleSearchResultsUrl)));
+  ASSERT_FALSE(
+      logger.ShouldLogPostCommitMetrics(GURL(kGoogleSearchResultsUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalIgnoredAfterCommit2) {
@@ -888,21 +885,22 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, ProvisionalIgnoredAfterCommit2) {
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.SetProvisionalUrl(GURL(kGoogleSearchResultsUrl));
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_TRUE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+  ASSERT_TRUE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearch) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_TRUE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+  ASSERT_TRUE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationToSearchHostname) {
   FromGWSPageLoadMetricsLogger logger;
   logger.SetPreviouslyCommittedUrl(GURL(kGoogleSearchResultsUrl));
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_FALSE(logger.ShouldLogMetrics(GURL("https://www.google.com/about/")));
+  ASSERT_FALSE(
+      logger.ShouldLogPostCommitMetrics(GURL("https://www.google.com/about/")));
 }
 
 TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearchRedirector) {
@@ -910,5 +908,5 @@ TEST_F(FromGWSPageLoadMetricsLoggerTest, NavigationFromSearchRedirector) {
   logger.SetPreviouslyCommittedUrl(
       GURL("https://www.google.com/url?source=web"));
   logger.set_navigation_initiated_via_link(true);
-  ASSERT_TRUE(logger.ShouldLogMetrics(GURL(kExampleUrl)));
+  ASSERT_TRUE(logger.ShouldLogPostCommitMetrics(GURL(kExampleUrl)));
 }