[CPM Update] Add UMA counters to track both ways of counting page loads.

Bug: 786551
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I621a26b36fd3777345dd2b77890dc4aee596105b
Reviewed-on: https://chromium-review.googlesource.com/777770
Commit-Queue: Danyao Wang <danyao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518398}

ios/chrome/browser/metrics/BUILD.gn

@@ -66,7 +66,7 @@ source_set("unit_tests") {
   testonly = true
   sources = [
     "ios_chrome_metrics_service_accessor_unittest.cc",
-    "ios_chrome_stability_metrics_provider_unittest.cc",
+    "ios_chrome_stability_metrics_provider_unittest.mm",
     "mobile_session_shutdown_metrics_provider_unittest.mm",
     "previous_session_info_unittest.mm",
   ]
@@ -80,7 +80,7 @@ source_set("unit_tests") {
     "//components/prefs:test_support",
     "//components/version_info",
     "//ios/chrome/test:test_support",
-    "//ios/web/public/test",
+    "//ios/web/public/test/fakes",
     "//testing/gtest",
   ]
 }

ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider.h

@@ -12,11 +12,33 @@
 
 class PrefService;
 
+namespace web {
+class NavigationContext;
+}  // namespace web
+
 // IOSChromeStabilityMetricsProvider gathers and logs Chrome-specific stability-
 // related metrics.
 class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider,
                                           public web::GlobalWebStateObserver {
  public:
+  // Buckets for the histogram that counts events relevant for counting page
+  // loads. The *_NAVIGATION events are mutually exclusive.
+  enum class StabilityMetricEventType {
+    // A chrome:// URL navigation. This is not counted for page load.
+    CHROME_URL_NAVIGATION = 0,
+    // A same-document web (i.e. not chrome:// URL) navigation. This is not
+    // counted for page load.
+    SAME_DOCUMENT_WEB_NAVIGATION = 1,
+    // A navigation that is not SAME_DOCUMENT_WEB or CHROME_URL. It is counted
+    // as a page load.
+    PAGE_LOAD_NAVIGATION = 2,
+    // A loading start event. This is the legacy page load count.
+    // TODO(crbug.com/786547): Deprecate this counter after page load count cuts
+    // over to be based on DidStartNavigation.
+    LOADING_STARTED = 3,
+    COUNT
+  };
+
   explicit IOSChromeStabilityMetricsProvider(PrefService* local_state);
   ~IOSChromeStabilityMetricsProvider() override;
 
@@ -29,11 +51,16 @@ class IOSChromeStabilityMetricsProvider : public metrics::MetricsProvider,
 
   // web::GlobalWebStateObserver:
   void WebStateDidStartLoading(web::WebState* web_state) override;
+  void WebStateDidStartNavigation(
+      web::WebState* web_state,
+      web::NavigationContext* navigation_context) override;
   void RenderProcessGone(web::WebState* web_state) override;
 
   // Records a renderer process crash.
   void LogRendererCrash();
 
+  static const char kPageLoadCountMigrationEventKey[];
+
  private:
   metrics::StabilityMetricsHelper helper_;
 

ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider.mm

@@ -4,12 +4,19 @@
 
 #include "ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider.h"
 
+#include "base/metrics/histogram_macros.h"
+#include "ios/chrome/browser/chrome_url_constants.h"
+#import "ios/web/public/web_state/navigation_context.h"
 #import "ios/web/public/web_state/web_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
+const char
+    IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey[] =
+        "IOS.PageLoadCountMigration.Counts";
+
 IOSChromeStabilityMetricsProvider::IOSChromeStabilityMetricsProvider(
     PrefService* local_state)
     : helper_(local_state), recording_enabled_(false) {}
@@ -51,9 +58,31 @@ void IOSChromeStabilityMetricsProvider::WebStateDidStartLoading(
   if (!recording_enabled_)
     return;
 
+  UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMigrationEventKey,
+                            StabilityMetricEventType::LOADING_STARTED,
+                            StabilityMetricEventType::COUNT);
   helper_.LogLoadStarted();
 }
 
+void IOSChromeStabilityMetricsProvider::WebStateDidStartNavigation(
+    web::WebState* web_state,
+    web::NavigationContext* navigation_context) {
+  if (!recording_enabled_)
+    return;
+
+  StabilityMetricEventType type =
+      StabilityMetricEventType::PAGE_LOAD_NAVIGATION;
+  if (navigation_context->GetUrl().SchemeIs(kChromeUIScheme)) {
+    type = StabilityMetricEventType::CHROME_URL_NAVIGATION;
+  } else if (navigation_context->IsSameDocument()) {
+    type = StabilityMetricEventType::SAME_DOCUMENT_WEB_NAVIGATION;
+  } else {
+    // TODO(crbug.com/786547): Move helper_.LogLoadStarted() here.
+  }
+  UMA_HISTOGRAM_ENUMERATION(kPageLoadCountMigrationEventKey, type,
+                            StabilityMetricEventType::COUNT);
+}
+
 void IOSChromeStabilityMetricsProvider::RenderProcessGone(
     web::WebState* web_state) {
   if (!recording_enabled_)

ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider_unittest.cc → ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider_unittest.mm

@@ -5,34 +5,34 @@
 #include "ios/chrome/browser/metrics/ios_chrome_stability_metrics_provider.h"
 
 #include "base/macros.h"
+#include "base/test/histogram_tester.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/prefs/testing_pref_service.h"
+#import "ios/web/public/test/fakes/fake_navigation_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 #include "third_party/metrics_proto/system_profile.pb.h"
 
 namespace {
 
+web::WebState* const kNullWebState = nullptr;
+
 class IOSChromeStabilityMetricsProviderTest : public PlatformTest {
  protected:
-  IOSChromeStabilityMetricsProviderTest()
-      : prefs_(new TestingPrefServiceSimple) {
-    metrics::StabilityMetricsHelper::RegisterPrefs(prefs()->registry());
+  IOSChromeStabilityMetricsProviderTest() {
+    metrics::StabilityMetricsHelper::RegisterPrefs(prefs_.registry());
   }
 
-  TestingPrefServiceSimple* prefs() { return prefs_.get(); }
-
- private:
-  std::unique_ptr<TestingPrefServiceSimple> prefs_;
-
-  DISALLOW_COPY_AND_ASSIGN(IOSChromeStabilityMetricsProviderTest);
+  base::HistogramTester histogram_tester_;
+  TestingPrefServiceSimple prefs_;
 };
 
 }  // namespace
 
-TEST_F(IOSChromeStabilityMetricsProviderTest, LogLoadStart) {
-  IOSChromeStabilityMetricsProvider provider(prefs());
+TEST_F(IOSChromeStabilityMetricsProviderTest,
+       DidStartLoadingEventShouldIncrementPageLoadCount) {
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
 
   // A load should not increment metrics if recording is disabled.
   provider.WebStateDidStartLoading(nullptr);
@@ -44,6 +44,10 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, LogLoadStart) {
   provider.ProvideStabilityMetrics(&system_profile);
 
   EXPECT_EQ(0, system_profile.stability().page_load_count());
+  EXPECT_TRUE(histogram_tester_
+                  .GetTotalCountsForPrefix(IOSChromeStabilityMetricsProvider::
+                                               kPageLoadCountMigrationEventKey)
+                  .empty());
 
   // A load should increment metrics if recording is enabled.
   provider.OnRecordingEnabled();
@@ -53,10 +57,102 @@ TEST_F(IOSChromeStabilityMetricsProviderTest, LogLoadStart) {
   provider.ProvideStabilityMetrics(&system_profile);
 
   EXPECT_EQ(1, system_profile.stability().page_load_count());
+  histogram_tester_.ExpectUniqueSample(
+      IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey,
+      static_cast<base::HistogramBase::Sample>(
+          IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
+              LOADING_STARTED),
+      1);
+}
+
+TEST_F(IOSChromeStabilityMetricsProviderTest,
+       SameDocumentNavigationShouldNotLogPageLoad) {
+  web::FakeNavigationContext context;
+  context.SetIsSameDocument(true);
+
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
+  provider.OnRecordingEnabled();
+  provider.WebStateDidStartNavigation(kNullWebState, &context);
+
+  histogram_tester_.ExpectUniqueSample(
+      IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey,
+      static_cast<base::HistogramBase::Sample>(
+          IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
+              SAME_DOCUMENT_WEB_NAVIGATION),
+      1);
+
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideStabilityMetrics(&system_profile);
+  EXPECT_EQ(0, system_profile.stability().page_load_count());
+}
+
+TEST_F(IOSChromeStabilityMetricsProviderTest,
+       ChromeUrlNavigationShouldNotLogPageLoad) {
+  web::FakeNavigationContext context;
+  context.SetUrl(GURL("chrome://newtab"));
+  context.SetIsSameDocument(false);
+
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
+  provider.OnRecordingEnabled();
+  provider.WebStateDidStartNavigation(kNullWebState, &context);
+
+  histogram_tester_.ExpectUniqueSample(
+      IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey,
+      static_cast<base::HistogramBase::Sample>(
+          IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
+              CHROME_URL_NAVIGATION),
+      1);
+
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideStabilityMetrics(&system_profile);
+  EXPECT_EQ(0, system_profile.stability().page_load_count());
+}
+
+TEST_F(IOSChromeStabilityMetricsProviderTest,
+       SameDocumentChromeUrlNavigationShouldNotLogPageLoad) {
+  web::FakeNavigationContext context;
+  context.SetUrl(GURL("chrome://newtab"));
+  context.SetIsSameDocument(true);
+
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
+  provider.OnRecordingEnabled();
+  provider.WebStateDidStartNavigation(kNullWebState, &context);
+
+  histogram_tester_.ExpectUniqueSample(
+      IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey,
+      static_cast<base::HistogramBase::Sample>(
+          IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
+              CHROME_URL_NAVIGATION),
+      1);
+
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideStabilityMetrics(&system_profile);
+  EXPECT_EQ(0, system_profile.stability().page_load_count());
+}
+
+TEST_F(IOSChromeStabilityMetricsProviderTest, WebNavigationShouldLogPageLoad) {
+  web::FakeNavigationContext context;
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
+  provider.OnRecordingEnabled();
+  provider.WebStateDidStartNavigation(kNullWebState, &context);
+
+  histogram_tester_.ExpectUniqueSample(
+      IOSChromeStabilityMetricsProvider::kPageLoadCountMigrationEventKey,
+      static_cast<base::HistogramBase::Sample>(
+          IOSChromeStabilityMetricsProvider::StabilityMetricEventType::
+              PAGE_LOAD_NAVIGATION),
+      1);
+
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideStabilityMetrics(&system_profile);
+  // TODO(crbug.com/786547): change to 1 once page load count cuts over to be
+  // based on DidStartNavigation.
+  EXPECT_EQ(0, system_profile.stability().page_load_count());
 }
 
-TEST_F(IOSChromeStabilityMetricsProviderTest, LogRendererCrash) {
-  IOSChromeStabilityMetricsProvider provider(prefs());
+TEST_F(IOSChromeStabilityMetricsProviderTest,
+       LogRendererCrashShouldIncrementCrashCount) {
+  IOSChromeStabilityMetricsProvider provider(&prefs_);
 
   // A crash should not increment the renderer crash count if recording is
   // disabled.

ios/web/public/web_state/global_web_state_observer.h

@@ -13,6 +13,7 @@
 namespace web {
 
 struct LoadCommittedDetails;
+class NavigationContext;
 class WebState;
 
 // An observer API implemented by classes that are interested in various
@@ -55,6 +56,13 @@ class GlobalWebStateObserver {
   // TODO(crbug.com/782269): Remove this method.
   virtual void WebStateDidStopLoading(WebState* web_state) {}
 
+  // Called when a navigation started in |web_state| for the main frame.
+  // DEPRECATED. Use WebStateObserver's |DidStartNavigation| instead.
+  // TODO(crbug.com/782269): Remove this method.
+  virtual void WebStateDidStartNavigation(
+      WebState* web_state,
+      NavigationContext* navigation_context) {}
+
   // Called when the current page is loaded in |web_state|.
   // DEPRECATED. Use WebStateObserver's |PageLoaded| instead.
   // TODO(crbug.com/782269): Remove this method.