Only report exit funnels for canary and dev channels.

BUG=442526

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

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

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

Cr-Commit-Position: refs/branch-heads/2272@{#144}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}

4 files changed, 64 insertions, 13 deletions

diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index ef588c1..83234c0 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -319,10 +319,19 @@ void ChromeMetricsServiceClient::Initialize() {
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(google_update_metrics_provider_));
 
+  // Report exit funnels for canary and dev only.
+  bool report_exit_funnels = false;
+  switch (chrome::VersionInfo::GetChannel()) {
+    case chrome::VersionInfo::CHANNEL_CANARY:
+    case chrome::VersionInfo::CHANNEL_DEV:
+      report_exit_funnels = true;
+      break;
+  }
+
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(
           new browser_watcher::WatcherMetricsProviderWin(
-              chrome::kBrowserExitCodesRegistryPath)));
+              chrome::kBrowserExitCodesRegistryPath, report_exit_funnels)));
 #endif  // defined(OS_WIN)
 
 #if defined(ENABLE_PLUGINS)
diff --git a/components/browser_watcher/watcher_metrics_provider_win.cc b/components/browser_watcher/watcher_metrics_provider_win.cc
index 0f94738..0848046 100644
--- a/components/browser_watcher/watcher_metrics_provider_win.cc
+++ b/components/browser_watcher/watcher_metrics_provider_win.cc
@@ -145,10 +145,13 @@ void ReadSingleExitFunnel(
   events_out->swap(events);
 }
 
-void RecordSingleExitFunnel(base::win::RegKey* parent_key,
-                            const base::char16* name) {
+void MaybeRecordSingleExitFunnel(base::win::RegKey* parent_key,
+                                 const base::char16* name,
+                                 bool report) {
   std::vector<std::pair<base::string16, int64>> events;
   ReadSingleExitFunnel(parent_key, name, &events);
+  if (!report)
+    return;
 
   // Find the earliest event time.
   int64 min_time = std::numeric_limits<int64>::max();
@@ -173,7 +176,7 @@ void RecordSingleExitFunnel(base::win::RegKey* parent_key,
   }
 }
 
-void RecordExitFunnels(const base::string16& registry_path) {
+void MaybeRecordExitFunnels(const base::string16& registry_path, bool report) {
   base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, registry_path.c_str());
   if (!it.Valid())
     return;
@@ -196,7 +199,7 @@ void RecordExitFunnels(const base::string16& registry_path) {
   for (; it.Valid(); ++it) {
     // Defer reporting on still-live processes.
     if (IsDeadProcess(it.Name())) {
-      RecordSingleExitFunnel(&key, it.Name());
+      MaybeRecordSingleExitFunnel(&key, it.Name(), report);
       to_delete.push_back(it.Name());
     }
   }
@@ -216,7 +219,9 @@ const char WatcherMetricsProviderWin::kExitFunnelHistogramPrefix[] =
     "Stability.ExitFunnel.";
 
 WatcherMetricsProviderWin::WatcherMetricsProviderWin(
-    const base::char16* registry_path) : registry_path_(registry_path) {
+    const base::char16* registry_path, bool report_exit_funnels) :
+        registry_path_(registry_path),
+        report_exit_funnels_(report_exit_funnels) {
 }
 
 WatcherMetricsProviderWin::~WatcherMetricsProviderWin() {
@@ -231,7 +236,7 @@ void WatcherMetricsProviderWin::ProvideStabilityMetrics(
   // necessary to implement some form of global locking, which is not worth it
   // here.
   RecordExitCodes(registry_path_);
-  RecordExitFunnels(registry_path_);
+  MaybeRecordExitFunnels(registry_path_, report_exit_funnels_);
 }
 
 }  // namespace browser_watcher
diff --git a/components/browser_watcher/watcher_metrics_provider_win.h b/components/browser_watcher/watcher_metrics_provider_win.h
index d2a47545..0004be3 100644
--- a/components/browser_watcher/watcher_metrics_provider_win.h
+++ b/components/browser_watcher/watcher_metrics_provider_win.h
@@ -27,7 +27,10 @@ class WatcherMetricsProviderWin : public metrics::MetricsProvider {
   static const char kBrowserExitCodeHistogramName[];
   static const char kExitFunnelHistogramPrefix[];
 
-  explicit WatcherMetricsProviderWin(const base::char16* registry_path);
+  // Initializes the reporter. If |report_exit_funnels| is false, the provider
+  // will clear the registry data, but not report it.
+  WatcherMetricsProviderWin(const base::char16* registry_path,
+                            bool report_exit_funnels);
   ~WatcherMetricsProviderWin();
 
   // metrics::MetricsProvider implementation.
@@ -36,6 +39,7 @@ class WatcherMetricsProviderWin : public metrics::MetricsProvider {
 
  private:
   base::string16 registry_path_;
+  bool report_exit_funnels_;
 
   DISALLOW_COPY_AND_ASSIGN(WatcherMetricsProviderWin);
 };
diff --git a/components/browser_watcher/watcher_metrics_provider_win_unittest.cc b/components/browser_watcher/watcher_metrics_provider_win_unittest.cc
index 838b12d..820ad34 100644
--- a/components/browser_watcher/watcher_metrics_provider_win_unittest.cc
+++ b/components/browser_watcher/watcher_metrics_provider_win_unittest.cc
@@ -81,7 +81,7 @@ TEST_F(WatcherMetricsProviderWinTest, RecordsStabilityHistogram) {
   // Record a single failure.
   AddProcessExitCode(false, 100);
 
-  WatcherMetricsProviderWin provider(kRegistryPath);
+  WatcherMetricsProviderWin provider(kRegistryPath, true);
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectBucketCount(
@@ -103,7 +103,7 @@ TEST_F(WatcherMetricsProviderWinTest, DoesNotReportOwnProcessId) {
   // Record own process as STILL_ACTIVE.
   AddProcessExitCode(true, STILL_ACTIVE);
 
-  WatcherMetricsProviderWin provider(kRegistryPath);
+  WatcherMetricsProviderWin provider(kRegistryPath, true);
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectUniqueSample(
@@ -122,7 +122,7 @@ TEST_F(WatcherMetricsProviderWinTest, RecordsOrderedExitFunnelEvents) {
   AddExitFunnelEvent(100, L"Two", 1010 * 1000);
   AddExitFunnelEvent(100, L"Three", 990 * 1000);
 
-  WatcherMetricsProviderWin provider(kRegistryPath);
+  WatcherMetricsProviderWin provider(kRegistryPath, true);
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectUniqueSample("Stability.ExitFunnel.Three", 0, 1);
@@ -135,7 +135,7 @@ TEST_F(WatcherMetricsProviderWinTest, RecordsOrderedExitFunnelEvents) {
 }
 
 TEST_F(WatcherMetricsProviderWinTest, ReadsExitFunnelWrites) {
-  // Test that the metrics provider picks up the writes from
+  // Test that the metrics provider picks up the writes from the funnel.
   ExitFunnel funnel;
 
   // Events against our own process should not get reported.
@@ -153,7 +153,7 @@ TEST_F(WatcherMetricsProviderWinTest, ReadsExitFunnelWrites) {
   ASSERT_TRUE(funnel.RecordEvent(L"Two"));
   ASSERT_TRUE(funnel.RecordEvent(L"Three"));
 
-  WatcherMetricsProviderWin provider(kRegistryPath);
+  WatcherMetricsProviderWin provider(kRegistryPath, true);
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.One", 1);
@@ -165,4 +165,37 @@ TEST_F(WatcherMetricsProviderWinTest, ReadsExitFunnelWrites) {
   ASSERT_EQ(it.SubkeyCount(), 1);
 }
 
+TEST_F(WatcherMetricsProviderWinTest, ClearsExitFunnelWriteWhenNotReporting) {
+  // Tests that the metrics provider cleans up, but doesn't report exit funnels
+  // when funnel reporting is quenched.
+  ExitFunnel funnel;
+
+  // Events against our own process should not get reported.
+  ASSERT_TRUE(funnel.Init(kRegistryPath, base::GetCurrentProcessHandle()));
+  ASSERT_TRUE(funnel.RecordEvent(L"Forgetaboutit"));
+
+  // Reset the funnel to a pseudo process. The PID 4 is the system process,
+  // which tests can hopefully never open.
+  ASSERT_TRUE(funnel.InitImpl(kRegistryPath, 4, base::Time::Now()));
+
+  // Each named event can only exist in a single copy.
+  ASSERT_TRUE(funnel.RecordEvent(L"One"));
+  ASSERT_TRUE(funnel.RecordEvent(L"One"));
+  ASSERT_TRUE(funnel.RecordEvent(L"One"));
+  ASSERT_TRUE(funnel.RecordEvent(L"Two"));
+  ASSERT_TRUE(funnel.RecordEvent(L"Three"));
+
+  // Turn off exit funnel reporting.
+  WatcherMetricsProviderWin provider(kRegistryPath, false);
+
+  provider.ProvideStabilityMetrics(NULL);
+  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.One", 0);
+  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.Two", 0);
+  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.Three", 0);
+
+  // Make sure the subkey for the pseudo process has been deleted on reporting.
+  base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, kRegistryPath);
+  ASSERT_EQ(it.SubkeyCount(), 1);
+}
+
 }  // namespace browser_watcher