[Metrics] Move Windows-specific CountBrowserCrashDumpAttempts() out of MetricsService

BUG=374216
TEST=none
R=asvitkine@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272590 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -9,6 +9,9 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
@@ -17,6 +20,7 @@
 #include "chrome/browser/memory_details.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/ui/browser_otr_state.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/crash_keys.h"
@@ -29,6 +33,12 @@
 #include "chrome/browser/service_process/service_process_control.h"
 #endif
 
+#if defined(OS_WIN)
+#include <windows.h>
+#include "base/win/registry.h"
+#include "chrome/browser/metrics/google_update_metrics_provider_win.h"
+#endif
+
 namespace {
 
 // This specifies the amount of time to wait for all renderers to send their
@@ -82,6 +92,10 @@ ChromeMetricsServiceClient::ChromeMetricsServiceClient()
   DCHECK(thread_checker_.CalledOnValidThread());
   RecordCommandLineMetrics();
   RegisterForNotifications();
+
+#if defined(OS_WIN)
+  CountBrowserCrashDumpAttempts();
+#endif  // defined(OS_WIN)
 }
 
 ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
@@ -289,3 +303,55 @@ void ChromeMetricsServiceClient::StartGatheringMetrics(
   // TODO(blundell): Move metrics gathering tasks from MetricsService to here.
   done_callback.Run();
 }
+
+#if defined(OS_WIN)
+void ChromeMetricsServiceClient::CountBrowserCrashDumpAttempts() {
+  // Open the registry key for iteration.
+  base::win::RegKey regkey;
+  if (regkey.Open(HKEY_CURRENT_USER,
+                  chrome::kBrowserCrashDumpAttemptsRegistryPath,
+                  KEY_ALL_ACCESS) != ERROR_SUCCESS) {
+    return;
+  }
+
+  // The values we're interested in counting are all prefixed with the version.
+  base::string16 chrome_version(base::ASCIIToUTF16(chrome::kChromeVersion));
+
+  // Track a list of values to delete. We don't modify the registry key while
+  // we're iterating over its values.
+  typedef std::vector<base::string16> StringVector;
+  StringVector to_delete;
+
+  // Iterate over the values in the key counting dumps with and without crashes.
+  // We directly walk the values instead of using RegistryValueIterator in order
+  // to read all of the values as DWORDS instead of strings.
+  base::string16 name;
+  DWORD value = 0;
+  int dumps_with_crash = 0;
+  int dumps_with_no_crash = 0;
+  for (int i = regkey.GetValueCount() - 1; i >= 0; --i) {
+    if (regkey.GetValueNameAt(i, &name) == ERROR_SUCCESS &&
+        StartsWith(name, chrome_version, false) &&
+        regkey.ReadValueDW(name.c_str(), &value) == ERROR_SUCCESS) {
+      to_delete.push_back(name);
+      if (value == 0)
+        ++dumps_with_no_crash;
+      else
+        ++dumps_with_crash;
+    }
+  }
+
+  // Delete the registry keys we've just counted.
+  for (StringVector::iterator i = to_delete.begin(); i != to_delete.end(); ++i)
+    regkey.DeleteValue(i->c_str());
+
+  // Capture the histogram samples.
+  if (dumps_with_crash != 0)
+    UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithCrash", dumps_with_crash);
+  if (dumps_with_no_crash != 0)
+    UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithNoCrash", dumps_with_no_crash);
+  int total_dumps = dumps_with_crash + dumps_with_no_crash;
+  if (total_dumps != 0)
+    UMA_HISTOGRAM_COUNTS("Chrome.BrowserCrashDumpAttempts", total_dumps);
+}
+#endif  // defined(OS_WIN)

chrome/browser/metrics/chrome_metrics_service_client.h

@@ -64,6 +64,12 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+#if defined(OS_WIN)
+  // Counts (and removes) the browser crash dump attempt signals left behind by
+  // any previous browser processes which generated a crash dump.
+  void CountBrowserCrashDumpAttempts();
+#endif  // OS_WIN
+
   base::ThreadChecker thread_checker_;
 
   // The MetricsService that |this| is a client of. Weak pointer.

chrome/browser/metrics/metrics_service.cc

@@ -192,7 +192,6 @@
 #include "chrome/browser/metrics/network_metrics_provider.h"
 #include "chrome/browser/metrics/omnibox_metrics_provider.h"
 #include "chrome/browser/metrics/tracking_synchronizer.h"
-#include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/variations/variations_util.h"
 #include "components/metrics/metrics_log_base.h"
@@ -218,8 +217,6 @@
 #endif
 
 #if defined(OS_WIN)
-#include <windows.h>  // Needed for STATUS_* codes
-#include "base/win/registry.h"
 #include "chrome/browser/metrics/google_update_metrics_provider_win.h"
 #endif
 
@@ -658,58 +655,6 @@ void MetricsService::RecordBreakpadHasDebugger(bool has_debugger) {
     IncrementPrefValue(prefs::kStabilityDebuggerPresent);
 }
 
-#if defined(OS_WIN)
-void MetricsService::CountBrowserCrashDumpAttempts() {
-  // Open the registry key for iteration.
-  base::win::RegKey regkey;
-  if (regkey.Open(HKEY_CURRENT_USER,
-                  chrome::kBrowserCrashDumpAttemptsRegistryPath,
-                  KEY_ALL_ACCESS) != ERROR_SUCCESS) {
-    return;
-  }
-
-  // The values we're interested in counting are all prefixed with the version.
-  base::string16 chrome_version(base::ASCIIToUTF16(chrome::kChromeVersion));
-
-  // Track a list of values to delete. We don't modify the registry key while
-  // we're iterating over its values.
-  typedef std::vector<base::string16> StringVector;
-  StringVector to_delete;
-
-  // Iterate over the values in the key counting dumps with and without crashes.
-  // We directly walk the values instead of using RegistryValueIterator in order
-  // to read all of the values as DWORDS instead of strings.
-  base::string16 name;
-  DWORD value = 0;
-  int dumps_with_crash = 0;
-  int dumps_with_no_crash = 0;
-  for (int i = regkey.GetValueCount() - 1; i >= 0; --i) {
-    if (regkey.GetValueNameAt(i, &name) == ERROR_SUCCESS &&
-        StartsWith(name, chrome_version, false) &&
-        regkey.ReadValueDW(name.c_str(), &value) == ERROR_SUCCESS) {
-      to_delete.push_back(name);
-      if (value == 0)
-        ++dumps_with_no_crash;
-      else
-        ++dumps_with_crash;
-    }
-  }
-
-  // Delete the registry keys we've just counted.
-  for (StringVector::iterator i = to_delete.begin(); i != to_delete.end(); ++i)
-    regkey.DeleteValue(i->c_str());
-
-  // Capture the histogram samples.
-  if (dumps_with_crash != 0)
-    UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithCrash", dumps_with_crash);
-  if (dumps_with_no_crash != 0)
-    UMA_HISTOGRAM_COUNTS("Chrome.BrowserDumpsWithNoCrash", dumps_with_no_crash);
-  int total_dumps = dumps_with_crash + dumps_with_no_crash;
-  if (total_dumps != 0)
-    UMA_HISTOGRAM_COUNTS("Chrome.BrowserCrashDumpAttempts", total_dumps);
-}
-#endif  // defined(OS_WIN)
-
 //------------------------------------------------------------------------------
 // private methods
 //------------------------------------------------------------------------------
@@ -755,10 +700,6 @@ void MetricsService::InitializeMetricsState() {
   DCHECK_EQ(UNINITIALIZED_PHASE, execution_phase_);
   SetExecutionPhase(START_METRICS_RECORDING);
 
-#if defined(OS_WIN)
-  CountBrowserCrashDumpAttempts();
-#endif  // defined(OS_WIN)
-
   if (!pref->GetBoolean(prefs::kStabilitySessionEndCompleted)) {
     IncrementPrefValue(prefs::kStabilityIncompleteSessionEndCount);
     // This is marked false when we get a WM_ENDSESSION.

chrome/browser/metrics/metrics_service.h

@@ -23,6 +23,7 @@
 #include "base/metrics/histogram_snapshot_manager.h"
 #include "base/metrics/user_metrics.h"
 #include "base/observer_list.h"
+#include "base/strings/string16.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "chrome/browser/metrics/metrics_log.h"
@@ -207,12 +208,6 @@ class MetricsService
   // This count is eventually send via UMA logs.
   void RecordBreakpadHasDebugger(bool has_debugger);
 
-#if defined(OS_WIN)
-  // Counts (and removes) the browser crash dump attempt signals left behind by
-  // any previous browser processes which generated a crash dump.
-  void CountBrowserCrashDumpAttempts();
-#endif  // OS_WIN
-
 #if defined(OS_CHROMEOS)
   // Records a Chrome OS crash.
   void LogChromeOSCrash(const std::string &crash_type);