Remove ExitFunnel instrumentation and clean up leaked data.

BUG=442526

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

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

12 files changed, 166 insertions, 320 deletions

diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index ba4ce89..3632038 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -188,7 +188,6 @@
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/shell_util.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util_win.h"
 #include "ui/gfx/win/dpi.h"
@@ -490,15 +489,8 @@ bool ProcessSingletonNotificationCallback(
     const base::CommandLine& command_line,
     const base::FilePath& current_directory) {
   // Drop the request if the browser process is already in shutdown path.
-  if (!g_browser_process || g_browser_process->IsShuttingDown()) {
-#if defined(OS_WIN)
-    browser_watcher::ExitFunnel::RecordSingleEvent(
-        chrome::kBrowserExitCodesRegistryPath,
-        L"ProcessSingletonIsShuttingDown");
-#endif  // defined(OS_WIN)
-
+  if (!g_browser_process || g_browser_process->IsShuttingDown())
     return false;
-  }
 
   if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
     std::string start_time_string =
diff --git a/chrome/browser/lifetime/application_lifetime.cc b/chrome/browser/lifetime/application_lifetime.cc
index 1986d1b..06a9008 100644
--- a/chrome/browser/lifetime/application_lifetime.cc
+++ b/chrome/browser/lifetime/application_lifetime.cc
@@ -48,7 +48,6 @@
 
 #if defined(OS_WIN)
 #include "base/win/win_util.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #endif
 
 #if defined(USE_ASH)
@@ -257,12 +256,6 @@ void ExitCleanly() {
 #endif
 
 void SessionEnding() {
-#if defined(OS_WIN)
-  browser_watcher::ExitFunnel funnel;
-
-  funnel.Init(kBrowserExitCodesRegistryPath, base::GetCurrentProcessHandle());
-  funnel.RecordEvent(L"SessionEnding");
-#endif
   // This is a time-limited shutdown where we need to write as much to
   // disk as we can as soon as we can, and where we must kill the
   // process within a hang timeout to avoid user prompts.
@@ -288,22 +281,12 @@ void SessionEnding() {
       content::NotificationService::AllSources(),
       content::NotificationService::NoDetails());
 
-#if defined(OS_WIN)
-  funnel.RecordEvent(L"EndSession");
-#endif
   // Write important data first.
   g_browser_process->EndSession();
 
 #if defined(OS_WIN)
   base::win::SetShouldCrashOnProcessDetach(false);
 #endif
-
-#if defined(OS_WIN)
-  // KillProcess ought to terminate the process without further ado, so if
-  // execution gets to this point, presumably this is normal exit.
-  funnel.RecordEvent(L"KillProcess");
-#endif
-
   // On Windows 7 and later, the system will consider the process ripe for
   // termination as soon as it hides or destroys its windows. Since any
   // execution past that point will be non-deterministically cut short, we
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index a8cf6ef..0ac839f 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -348,25 +348,11 @@ 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::GetChannel()) {
-    case version_info::Channel::CANARY:
-    case version_info::Channel::DEV:
-      report_exit_funnels = true;
-      break;
-    case version_info::Channel::UNKNOWN:
-    case version_info::Channel::BETA:
-    case version_info::Channel::STABLE:
-      // report_exit_funnels was initialized to the right value above.
-      DCHECK(!report_exit_funnels);
-      break;
-  }
-
   metrics_service_->RegisterMetricsProvider(
       scoped_ptr<metrics::MetricsProvider>(
           new browser_watcher::WatcherMetricsProviderWin(
-              chrome::kBrowserExitCodesRegistryPath, report_exit_funnels)));
+              chrome::kBrowserExitCodesRegistryPath,
+              content::BrowserThread::GetBlockingPool())));
 #endif  // defined(OS_WIN)
 
 #if defined(ENABLE_PLUGINS)
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index d0c5166..623aa7f 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -32,7 +32,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/installer/util/wmi.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "content/public/common/result_codes.h"
 #include "net/base/escape.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -268,9 +267,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
       remote_window_ = NULL;
       return PROCESS_NONE;
     case chrome::NOTIFY_WINDOW_HUNG:
-      // Record a hung rendezvous event in this process' exit funnel.
-      browser_watcher::ExitFunnel::RecordSingleEvent(
-          chrome::kBrowserExitCodesRegistryPath, L"RendezvousToHungBrowser");
+      // Fall through and potentially terminate the hung browser.
       break;
   }
 
@@ -294,11 +291,6 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
     return PROCESS_NOTIFIED;
   }
 
-  // Record the termination event in the hung process' exit funnel.
-  browser_watcher::ExitFunnel funnel;
-  if (funnel.Init(chrome::kBrowserExitCodesRegistryPath, process.Handle()))
-    funnel.RecordEvent(L"HungBrowserTerminated");
-
   // Time to take action. Kill the browser process.
   process.Terminate(content::RESULT_CODE_HUNG, true);
   remote_window_ = NULL;
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 3cc7dac..24e66d5 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -209,7 +209,6 @@
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "components/autofill/core/browser/autofill_ie_toolbar_import_win.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/win/shell.h"
 #endif  // OS_WIN
@@ -714,13 +713,8 @@ void Browser::OnWindowClosing() {
   bool should_quit_if_last_browser =
       browser_shutdown::IsTryingToQuit() || !chrome::WillKeepAlive();
 
-  if (should_quit_if_last_browser && chrome::ShouldStartShutdown(this)) {
-#if defined(OS_WIN)
-    browser_watcher::ExitFunnel::RecordSingleEvent(
-          chrome::kBrowserExitCodesRegistryPath, L"LastWindowClose");
-#endif
+  if (should_quit_if_last_browser && chrome::ShouldStartShutdown(this))
     browser_shutdown::OnShutdownStarting(browser_shutdown::WINDOW_CLOSE);
-  }
 
   // Don't use GetForProfileIfExisting here, we want to force creation of the
   // session service so that user can restore what was open.
diff --git a/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc b/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
index ac7d9c2..0fcedee 100644
--- a/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
+++ b/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
@@ -19,7 +19,6 @@
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/theme_image_mapper.h"
 #include "chrome/common/chrome_constants.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/win/dpi.h"
 #include "ui/views/controls/menu/native_menu_win.h"
@@ -74,31 +73,6 @@ class DesktopThemeProvider : public ui::ThemeProvider {
   DISALLOW_COPY_AND_ASSIGN(DesktopThemeProvider);
 };
 
-// See http://crbug.com/412384.
-void TraceSessionEnding(LPARAM lparam) {
-  browser_watcher::ExitFunnel funnel;
-  if (!funnel.Init(chrome::kBrowserExitCodesRegistryPath,
-                   base::GetCurrentProcessHandle())) {
-    return;
-  }
-
-  // This exit path is the prime suspect for most our unclean shutdowns.
-  // Trace all the possible options to WM_ENDSESSION. This may result in
-  // multiple events for a single shutdown, but that's fine.
-  funnel.RecordEvent(L"WM_ENDSESSION");
-
-  if (lparam & ENDSESSION_CLOSEAPP)
-    funnel.RecordEvent(L"ES_CloseApp");
-  if (lparam & ENDSESSION_CRITICAL)
-    funnel.RecordEvent(L"ES_Critical");
-  if (lparam & ENDSESSION_LOGOFF)
-    funnel.RecordEvent(L"ES_Logoff");
-  const LPARAM kKnownBits =
-      ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
-  if (lparam & ~kKnownBits)
-    funnel.RecordEvent(L"ES_Other");
-}
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -211,7 +185,6 @@ bool BrowserDesktopWindowTreeHostWin::PreHandleMSG(UINT message,
         minimize_button_metrics_.OnHWNDActivated();
       return false;
     case WM_ENDSESSION:
-      TraceSessionEnding(l_param);
       chrome::SessionEnding();
       return true;
     case WM_INITMENUPOPUP:
diff --git a/chrome/browser/ui/views/status_icons/status_tray_win.cc b/chrome/browser/ui/views/status_icons/status_tray_win.cc
index a87eec1..c32481c 100644
--- a/chrome/browser/ui/views/status_icons/status_tray_win.cc
+++ b/chrome/browser/ui/views/status_icons/status_tray_win.cc
@@ -16,7 +16,6 @@
 #include "chrome/browser/ui/views/status_icons/status_icon_win.h"
 #include "chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h"
 #include "chrome/common/chrome_constants.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/win/hwnd_util.h"
 
@@ -30,31 +29,6 @@ UINT ReservedIconId(StatusTray::StatusIconType type) {
   return kBaseIconId + static_cast<UINT>(type);
 }
 
-// See http://crbug.com/412384.
-void TraceSessionEnding(LPARAM lparam) {
-  browser_watcher::ExitFunnel funnel;
-  if (!funnel.Init(chrome::kBrowserExitCodesRegistryPath,
-                   base::GetCurrentProcessHandle())) {
-    return;
-  }
-
-  // This exit path is the prime suspect for most our unclean shutdowns.
-  // Trace all the possible options to WM_ENDSESSION. This may result in
-  // multiple events for a single shutdown, but that's fine.
-  funnel.RecordEvent(L"TraybarEndSession");
-
-  if (lparam & ENDSESSION_CLOSEAPP)
-    funnel.RecordEvent(L"ES_CloseApp");
-  if (lparam & ENDSESSION_CRITICAL)
-    funnel.RecordEvent(L"ES_Critical");
-  if (lparam & ENDSESSION_LOGOFF)
-    funnel.RecordEvent(L"ES_Logoff");
-  const LPARAM kKnownBits =
-      ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
-  if (lparam & ~kKnownBits)
-    funnel.RecordEvent(L"ES_Other");
-}
-
 }  // namespace
 
 // Default implementation for StatusTrayStateChanger that communicates to
@@ -226,7 +200,6 @@ LRESULT CALLBACK StatusTrayWin::WndProc(HWND hwnd,
     // If Chrome is in background-only mode, this is the only notification
     // it gets that Windows is exiting. Make sure we shutdown in an orderly
     // fashion.
-    TraceSessionEnding(lparam);
     chrome::SessionEnding();
   }
   return ::DefWindowProc(hwnd, message, wparam, lparam);
diff --git a/chrome/chrome_watcher/chrome_watcher_main.cc b/chrome/chrome_watcher/chrome_watcher_main.cc
index 6219bec..db578d6 100644
--- a/chrome/chrome_watcher/chrome_watcher_main.cc
+++ b/chrome/chrome_watcher/chrome_watcher_main.cc
@@ -35,7 +35,6 @@
 #include "chrome/installer/util/util_constants.h"
 #include "components/browser_watcher/endsession_watcher_window_win.h"
 #include "components/browser_watcher/exit_code_watcher_win.h"
-#include "components/browser_watcher/exit_funnel_win.h"
 #include "components/browser_watcher/window_hang_monitor_win.h"
 
 #ifdef KASKO
@@ -79,9 +78,6 @@ class BrowserMonitor {
   // Posted to main thread from Watch when browser exits.
   void BrowserExited();
 
-  // The funnel used to record events for this browser.
-  browser_watcher::ExitFunnel exit_funnel_;
-
   browser_watcher::ExitCodeWatcher exit_code_watcher_;
   browser_watcher::EndSessionWatcherWindow end_session_watcher_window_;
 
@@ -121,11 +117,6 @@ bool BrowserMonitor::StartWatching(
   if (!exit_code_watcher_.Initialize(process.Pass()))
     return false;
 
-  if (!exit_funnel_.Init(registry_path,
-                         exit_code_watcher_.process().Handle())) {
-    return false;
-  }
-
   if (!background_thread_.StartWithOptions(
         base::Thread::Options(base::MessageLoop::TYPE_IO, 0))) {
     return false;
@@ -144,22 +135,6 @@ bool BrowserMonitor::StartWatching(
 void BrowserMonitor::OnEndSessionMessage(UINT message, LPARAM lparam) {
   DCHECK_EQ(main_thread_, base::ThreadTaskRunnerHandle::Get());
 
-  if (message == WM_QUERYENDSESSION) {
-    exit_funnel_.RecordEvent(L"WatcherQueryEndSession");
-  } else if (message == WM_ENDSESSION) {
-    exit_funnel_.RecordEvent(L"WatcherEndSession");
-  }
-  if (lparam & ENDSESSION_CLOSEAPP)
-    exit_funnel_.RecordEvent(L"ES_CloseApp");
-  if (lparam & ENDSESSION_CRITICAL)
-    exit_funnel_.RecordEvent(L"ES_Critical");
-  if (lparam & ENDSESSION_LOGOFF)
-    exit_funnel_.RecordEvent(L"ES_Logoff");
-  const LPARAM kKnownBits =
-      ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
-  if (lparam & ~kKnownBits)
-    exit_funnel_.RecordEvent(L"ES_Other");
-
   // If the browser hasn't exited yet, dally for a bit to try and stretch this
   // process' lifetime to give it some more time to capture the browser exit.
   browser_exited_.TimedWait(base::TimeDelta::FromSeconds(kDelayTimeSeconds));
@@ -177,7 +152,6 @@ void BrowserMonitor::Watch(base::win::ScopedHandle on_initialized_event) {
   on_initialized_event.Close();
 
   exit_code_watcher_.WaitForExit();
-  exit_funnel_.RecordEvent(L"BrowserExit");
 
   // Note that the browser has exited.
   browser_exited_.Signal();
@@ -213,24 +187,9 @@ void OnWindowEvent(
     base::Process process,
     const base::Callback<void(const base::Process&)>& on_hung_callback,
     browser_watcher::WindowHangMonitor::WindowEvent window_event) {
-  browser_watcher::ExitFunnel exit_funnel;
-  if (exit_funnel.Init(registry_path.c_str(), process.Handle())) {
-    switch (window_event) {
-      case browser_watcher::WindowHangMonitor::WINDOW_NOT_FOUND:
-        exit_funnel.RecordEvent(L"MessageWindowNotFound");
-        break;
-      case browser_watcher::WindowHangMonitor::WINDOW_HUNG:
-        exit_funnel.RecordEvent(L"MessageWindowHung");
-        if (!on_hung_callback.is_null())
-          on_hung_callback.Run(process);
-        break;
-      case browser_watcher::WindowHangMonitor::WINDOW_VANISHED:
-        exit_funnel.RecordEvent(L"MessageWindowVanished");
-        break;
-      default:
-        NOTREACHED();
-        break;
-    }
+  if (window_event == browser_watcher::WindowHangMonitor::WINDOW_HUNG &&
+      !on_hung_callback.is_null()) {
+    on_hung_callback.Run(process);
   }
 }
 
diff --git a/components/browser_watcher/exit_funnel_win.h b/components/browser_watcher/exit_funnel_win.h
index 9af2b388..efd35ab 100644
--- a/components/browser_watcher/exit_funnel_win.h
+++ b/components/browser_watcher/exit_funnel_win.h
@@ -19,6 +19,9 @@ namespace browser_watcher {
 // The event trace is stored in registry under a new key named
 // <pid>-<uniquifier>, where each event is a value named after the event, with
 // an associated QWORD value noting the event time.
+// Note: this is deprecated, and is only kept around for testing the cleanup
+// in the WatcherMetricsProvider.
+// TODO(siggi): Kill this class - see http://crbug.com/442526.
 class ExitFunnel {
  public:
   ExitFunnel();
diff --git a/components/browser_watcher/watcher_metrics_provider_win.cc b/components/browser_watcher/watcher_metrics_provider_win.cc
index 76352b8..5484956 100644
--- a/components/browser_watcher/watcher_metrics_provider_win.cc
+++ b/components/browser_watcher/watcher_metrics_provider_win.cc
@@ -7,6 +7,7 @@
 #include <limits>
 #include <vector>
 
+#include "base/bind.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/process/process.h"
 #include "base/strings/string_number_conversions.h"
@@ -98,82 +99,26 @@ void RecordExitCodes(const base::string16& registry_path) {
     regkey.DeleteValue(to_delete[i].c_str());
 }
 
-void ReadSingleExitFunnel(
-    base::win::RegKey* parent_key, const base::char16* name,
-    std::vector<std::pair<base::string16, int64>>* events_out) {
-  DCHECK(parent_key);
-  DCHECK(name);
-  DCHECK(events_out);
+void DeleteAllValues(base::win::RegKey* key) {
+  DCHECK(key);
 
-  base::win::RegKey regkey(parent_key->Handle(), name, KEY_READ | KEY_WRITE);
-  if (!regkey.Valid())
-    return;
-
-  // Exit early if no work to do.
-  size_t num = regkey.GetValueCount();
-  if (num == 0)
-    return;
-
-  // Enumerate the recorded events for this process for processing.
-  std::vector<std::pair<base::string16, int64>> events;
-  for (size_t i = 0; i < num; ++i) {
-    base::string16 event_name;
-    LONG res = regkey.GetValueNameAt(static_cast<int>(i), &event_name);
-    if (res == ERROR_SUCCESS) {
-      int64 event_time = 0;
-      res = regkey.ReadInt64(event_name.c_str(), &event_time);
-      if (res == ERROR_SUCCESS)
-        events.push_back(std::make_pair(event_name, event_time));
+  while (key->GetValueCount() != 0) {
+    base::string16 value_name;
+    LONG res = key->GetValueNameAt(0, &value_name);
+    if (res != ERROR_SUCCESS) {
+      DVLOG(1) << "Failed to get value name " << res;
+      return;
     }
-  }
 
-  // Attempt to delete the values before reporting anything.
-  // Exit if this fails to make sure there is no double-reporting on e.g.
-  // permission problems or other corruption.
-  for (size_t i = 0; i < events.size(); ++i) {
-    const base::string16& event_name = events[i].first;
-    LONG res = regkey.DeleteValue(event_name.c_str());
+    res = key->DeleteValue(value_name.c_str());
     if (res != ERROR_SUCCESS) {
-      LOG(ERROR) << "Failed to delete value " << event_name;
+      DVLOG(1) << "Failed to delete value " << value_name;
       return;
     }
   }
-
-  events_out->swap(events);
 }
 
-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();
-  for (size_t i = 0; i < events.size(); ++i)
-    min_time = std::min(min_time, events[i].second);
-
-  // Record the exit funnel event times in a sparse stability histogram.
-  for (size_t i = 0; i < events.size(); ++i) {
-    std::string histogram_name(
-        WatcherMetricsProviderWin::kExitFunnelHistogramPrefix);
-    histogram_name.append(base::WideToUTF8(events[i].first));
-    base::TimeDelta event_time =
-        base::Time::FromInternalValue(events[i].second) -
-            base::Time::FromInternalValue(min_time);
-    base::HistogramBase* histogram =
-        base::SparseHistogram::FactoryGet(
-            histogram_name.c_str(),
-            base::HistogramBase::kUmaStabilityHistogramFlag);
-
-    // Record the time rounded up to the nearest millisecond.
-    histogram->Add(event_time.InMillisecondsRoundedUp());
-  }
-}
-
-void MaybeRecordExitFunnels(const base::string16& registry_path, bool report) {
+void DeleteExitFunnels(const base::string16& registry_path) {
   base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, registry_path.c_str());
   if (!it.Valid())
     return;
@@ -188,42 +133,90 @@ void MaybeRecordExitFunnels(const base::string16& registry_path, bool report) {
                         registry_path.c_str(),
                         KEY_QUERY_VALUE);
   if (!key.Valid()) {
-    LOG(ERROR) << "Failed to open " << registry_path << " for writing.";
+    DVLOG(1) << "Failed to open " << registry_path << " for writing.";
     return;
   }
 
-  std::vector<base::string16> to_delete;
-  for (; it.Valid(); ++it) {
-    // Defer reporting on still-live processes.
-    if (IsDeadProcess(it.Name())) {
-      MaybeRecordSingleExitFunnel(&key, it.Name(), report);
-      to_delete.push_back(it.Name());
+  // Key names to delete.
+  std::vector<base::string16> keys_to_delete;
+  // Constrain the cleanup to 100 exit funnels at a time, as otherwise this may
+  // take a long time to finish where a lot of data has accrued. This will be
+  // the case in particular for non-UMA users, as the exit funnel data will
+  // accrue without bounds for those users.
+  const size_t kMaxCleanup = 100;
+  for (; it.Valid() && keys_to_delete.size() < kMaxCleanup; ++it) {
+    base::win::RegKey sub_key;
+    LONG res =
+        sub_key.Open(key.Handle(), it.Name(), KEY_QUERY_VALUE | KEY_SET_VALUE);
+    if (res != ERROR_SUCCESS) {
+      DVLOG(1) << "Failed to open subkey " << it.Name();
+      return;
     }
+    DeleteAllValues(&sub_key);
+
+    // Schedule the subkey for deletion.
+    keys_to_delete.push_back(it.Name());
   }
 
-  for (size_t i = 0; i < to_delete.size(); ++i) {
-    LONG res = key.DeleteEmptyKey(to_delete[i].c_str());
+  for (const base::string16& key_name : keys_to_delete) {
+    LONG res = key.DeleteEmptyKey(key_name.c_str());
     if (res != ERROR_SUCCESS)
-      LOG(ERROR) << "Failed to delete key " << to_delete[i];
+      DVLOG(1) << "Failed to delete key " << key_name;
+  }
+}
+
+// Called from the blocking pool when metrics reporting is disabled, as there
+// may be a sizable stash of data to delete.
+void DeleteExitCodeRegistryKey(const base::string16& registry_path) {
+  CHECK_NE(L"", registry_path);
+
+  DeleteExitFunnels(registry_path);
+
+  base::win::RegKey key;
+  LONG res = key.Open(HKEY_CURRENT_USER, registry_path.c_str(),
+                      KEY_QUERY_VALUE | KEY_SET_VALUE);
+  if (res == ERROR_SUCCESS) {
+    DeleteAllValues(&key);
+    res = key.DeleteEmptyKey(L"");
   }
+  if (res != ERROR_FILE_NOT_FOUND && res != ERROR_SUCCESS)
+    DVLOG(1) << "Failed to delete exit code key " << registry_path;
 }
 
 }  // namespace
 
 const char WatcherMetricsProviderWin::kBrowserExitCodeHistogramName[] =
     "Stability.BrowserExitCodes";
-const char WatcherMetricsProviderWin::kExitFunnelHistogramPrefix[] =
-    "Stability.ExitFunnel.";
 
 WatcherMetricsProviderWin::WatcherMetricsProviderWin(
-    const base::char16* registry_path, bool report_exit_funnels) :
-        registry_path_(registry_path),
-        report_exit_funnels_(report_exit_funnels) {
+    const base::char16* registry_path,
+    base::TaskRunner* cleanup_io_task_runner)
+    : recording_enabled_(false),
+      cleanup_scheduled_(false),
+      registry_path_(registry_path),
+      cleanup_io_task_runner_(cleanup_io_task_runner) {
+  DCHECK(cleanup_io_task_runner_);
 }
 
 WatcherMetricsProviderWin::~WatcherMetricsProviderWin() {
 }
 
+void WatcherMetricsProviderWin::OnRecordingEnabled() {
+  recording_enabled_ = true;
+}
+
+void WatcherMetricsProviderWin::OnRecordingDisabled() {
+  if (!recording_enabled_ && !cleanup_scheduled_) {
+    // When metrics reporting is disabled, the providers get an
+    // OnRecordingDisabled notification at startup. Use that first notification
+    // to issue the cleanup task.
+    cleanup_io_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&DeleteExitCodeRegistryKey, registry_path_));
+
+    cleanup_scheduled_ = true;
+  }
+}
+
 void WatcherMetricsProviderWin::ProvideStabilityMetrics(
     metrics::SystemProfileProto* /* system_profile_proto */) {
   // Note that if there are multiple instances of Chrome running in the same
@@ -233,7 +226,7 @@ void WatcherMetricsProviderWin::ProvideStabilityMetrics(
   // necessary to implement some form of global locking, which is not worth it
   // here.
   RecordExitCodes(registry_path_);
-  MaybeRecordExitFunnels(registry_path_, report_exit_funnels_);
+  DeleteExitFunnels(registry_path_);
 }
 
 }  // namespace browser_watcher
diff --git a/components/browser_watcher/watcher_metrics_provider_win.h b/components/browser_watcher/watcher_metrics_provider_win.h
index e198c52..d2f800e 100644
--- a/components/browser_watcher/watcher_metrics_provider_win.h
+++ b/components/browser_watcher/watcher_metrics_provider_win.h
@@ -7,39 +7,34 @@
 
 #include "base/macros.h"
 #include "base/strings/string16.h"
+#include "base/task_runner.h"
 #include "components/metrics/metrics_provider.h"
 
 namespace browser_watcher {
 
 // Provides stability data captured by the Chrome Watcher, namely the browser
-// process exit codes, as well as exit funnel metrics.
-// The exit funnel records a trace of named, timed events in registry per
-// process. For reporting, the trace is recorded as a sequence of events
-// named Stability.ExitFunnel.<eventname>, associated to the time
-// (in milliseconds) from first event in a trace. For a normal process exit,
-// the sequence might look like this:
-// - Stability.ExitFunnel.Logoff: 0
-// - Stability.ExitFunnel.NotifyShutdown: 10
-// - Stability.ExitFunnel.EndSession: 20
-// - Stability.ExitFunnel.KillProcess: 30
+// process exit codes.
 class WatcherMetricsProviderWin : public metrics::MetricsProvider {
  public:
   static const char kBrowserExitCodeHistogramName[];
-  static const char kExitFunnelHistogramPrefix[];
 
-  // Initializes the reporter. If |report_exit_funnels| is false, the provider
-  // will clear the registry data, but not report it.
+  // Initializes the reporter. |cleanup_io_task_runner| is used to clear
+  // leftover data in registry if metrics reporting is disabled.
   WatcherMetricsProviderWin(const base::char16* registry_path,
-                            bool report_exit_funnels);
+                            base::TaskRunner* cleanup_io_task_runner);
   ~WatcherMetricsProviderWin() override;
 
   // metrics::MetricsProvider implementation.
+  void OnRecordingEnabled() override;
+  void OnRecordingDisabled() override;
   void ProvideStabilityMetrics(
       metrics::SystemProfileProto* system_profile_proto) override;
 
  private:
+  bool recording_enabled_;
+  bool cleanup_scheduled_;
   base::string16 registry_path_;
-  bool report_exit_funnels_;
+  scoped_refptr<base::TaskRunner> cleanup_io_task_runner_;
 
   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 b4c3922..f77f54f 100644
--- a/components/browser_watcher/watcher_metrics_provider_win_unittest.cc
+++ b/components/browser_watcher/watcher_metrics_provider_win_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/test_reg_util_win.h"
+#include "base/test/test_simple_task_runner.h"
 #include "base/win/registry.h"
 #include "components/browser_watcher/exit_funnel_win.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -29,6 +30,7 @@ class WatcherMetricsProviderWinTest : public testing::Test {
     Super::SetUp();
 
     override_manager_.OverrideRegistry(HKEY_CURRENT_USER);
+    test_task_runner_ = new base::TestSimpleTaskRunner();
   }
 
   void AddProcessExitCode(bool use_own_pid, int exit_code) {
@@ -69,6 +71,7 @@ class WatcherMetricsProviderWinTest : public testing::Test {
  protected:
   registry_util::RegistryOverrideManager override_manager_;
   base::HistogramTester histogram_tester_;
+  scoped_refptr<base::TestSimpleTaskRunner> test_task_runner_;
 };
 
 }  // namespace
@@ -81,7 +84,7 @@ TEST_F(WatcherMetricsProviderWinTest, RecordsStabilityHistogram) {
   // Record a single failure.
   AddProcessExitCode(false, 100);
 
-  WatcherMetricsProviderWin provider(kRegistryPath, true);
+  WatcherMetricsProviderWin provider(kRegistryPath, test_task_runner_.get());
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectBucketCount(
@@ -103,7 +106,7 @@ TEST_F(WatcherMetricsProviderWinTest, DoesNotReportOwnProcessId) {
   // Record own process as STILL_ACTIVE.
   AddProcessExitCode(true, STILL_ACTIVE);
 
-  WatcherMetricsProviderWin provider(kRegistryPath, true);
+  WatcherMetricsProviderWin provider(kRegistryPath, test_task_runner_.get());
 
   provider.ProvideStabilityMetrics(NULL);
   histogram_tester_.ExpectUniqueSample(
@@ -113,89 +116,89 @@ TEST_F(WatcherMetricsProviderWinTest, DoesNotReportOwnProcessId) {
   EXPECT_EQ(ExitCodeRegistryPathValueCount(), 1);
 }
 
-TEST_F(WatcherMetricsProviderWinTest, RecordsOrderedExitFunnelEvents) {
-  // Record an exit funnel with a given set of timings and check that the
-  // ordering is correct on the reported histograms.
-  // Note the recorded times are in microseconds, but the reporting is in
-  // milliseconds, hence the times 1000.
+TEST_F(WatcherMetricsProviderWinTest, DeletesRecordedExitFunnelEvents) {
+  // Record an exit funnel and make sure the registry is cleaned up on
+  // reporting, without recording any events.
   AddExitFunnelEvent(100, L"One", 1000 * 1000);
-  AddExitFunnelEvent(100, L"Two", 1010 * 1000);
-  AddExitFunnelEvent(100, L"Three", 990 * 1000);
+  AddExitFunnelEvent(101, L"Two", 1010 * 1000);
+  AddExitFunnelEvent(102, L"Three", 990 * 1000);
 
-  WatcherMetricsProviderWin provider(kRegistryPath, true);
+  base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, kRegistryPath);
+  EXPECT_EQ(it.SubkeyCount(), 3);
 
-  provider.ProvideStabilityMetrics(NULL);
-  histogram_tester_.ExpectUniqueSample("Stability.ExitFunnel.Three", 0, 1);
-  histogram_tester_.ExpectUniqueSample("Stability.ExitFunnel.One", 10, 1);
-  histogram_tester_.ExpectUniqueSample("Stability.ExitFunnel.Two", 20, 1);
+  WatcherMetricsProviderWin provider(kRegistryPath, test_task_runner_.get());
 
-  // Make sure the subkey is deleted on reporting.
-  base::win::RegistryKeyIterator it(HKEY_CURRENT_USER, kRegistryPath);
+  provider.ProvideStabilityMetrics(NULL);
+  // Make sure the exit funnel events are no longer recorded in histograms.
+  EXPECT_TRUE(
+      histogram_tester_.GetAllSamples("Stability.ExitFunnel.One").empty());
+  EXPECT_TRUE(
+      histogram_tester_.GetAllSamples("Stability.ExitFunnel.Two").empty());
+  EXPECT_TRUE(
+      histogram_tester_.GetAllSamples("Stability.ExitFunnel.Three").empty());
+
+  // Make sure the subkeys are deleted on reporting.
   ASSERT_EQ(it.SubkeyCount(), 0);
 }
 
-TEST_F(WatcherMetricsProviderWinTest, ReadsExitFunnelWrites) {
-  // Test that the metrics provider picks up the writes from the funnel.
+TEST_F(WatcherMetricsProviderWinTest, DeletesExitcodeKeyWhenNotReporting) {
+  // Test that the registry at kRegistryPath is deleted when reporting is
+  // disabled.
   ExitFunnel funnel;
 
-  // Events against our own process should not get reported.
-  ASSERT_TRUE(funnel.Init(kRegistryPath, base::GetCurrentProcessHandle()));
-  ASSERT_TRUE(funnel.RecordEvent(L"Forgetaboutit"));
+  // Record multiple success exits.
+  for (size_t i = 0; i < 11; ++i)
+    AddProcessExitCode(false, 0);
+  // Record a single failure.
+  AddProcessExitCode(false, 100);
 
-  // Reset the funnel to a pseudo process. The PID 4 is the system process,
-  // which tests can hopefully never open.
+  // Record an exit funnel.
   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"));
-
-  WatcherMetricsProviderWin provider(kRegistryPath, true);
+  AddExitFunnelEvent(100, L"One", 1000 * 1000);
+  AddExitFunnelEvent(101, L"Two", 1010 * 1000);
+  AddExitFunnelEvent(102, L"Three", 990 * 1000);
+
+  // Make like the user is opted out of reporting.
+  WatcherMetricsProviderWin provider(kRegistryPath, test_task_runner_.get());
+  provider.OnRecordingDisabled();
+
+  base::win::RegKey key;
+  {
+    // The deletion should be scheduled to the test_task_runner, and not happen
+    // immediately.
+    ASSERT_EQ(ERROR_SUCCESS,
+              key.Open(HKEY_CURRENT_USER, kRegistryPath, KEY_READ));
+  }
 
-  provider.ProvideStabilityMetrics(NULL);
-  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.One", 1);
-  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.Two", 1);
-  histogram_tester_.ExpectTotalCount("Stability.ExitFunnel.Three", 1);
+  // Flush the task(s).
+  test_task_runner_->RunPendingTasks();
 
   // 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);
+  ASSERT_EQ(ERROR_FILE_NOT_FOUND,
+            key.Open(HKEY_CURRENT_USER, kRegistryPath, KEY_READ));
 }
 
-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);
+TEST_F(WatcherMetricsProviderWinTest, DeletesOnly100FunnelsAtATime) {
+  // Record 200 distinct exit funnels.
+  for (size_t i = 0; i < 200; ++i) {
+    AddExitFunnelEvent(i, L"One", 10);
+    AddExitFunnelEvent(i, L"Two", 10);
+  }
 
-  // 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);
+  EXPECT_EQ(it.SubkeyCount(), 200);
+
+  {
+    // Make like the user is opted out of reporting.
+    WatcherMetricsProviderWin provider(kRegistryPath, test_task_runner_.get());
+    provider.OnRecordingDisabled();
+    // Flush the task(s).
+    test_task_runner_->RunPendingTasks();
+  }
+
+  // We expect only 100 of the funnels have been scrubbed.
+  EXPECT_EQ(it.SubkeyCount(), 100);
 }
 
 }  // namespace browser_watcher