Fix memory leak in ThreadWatcher.

chrome/browser/metrics/thread_watcher.cc

Index: chrome/browser/metrics/thread_watcher.cc
diff --git a/chrome/browser/metrics/thread_watcher.cc b/chrome/browser/metrics/thread_watcher.cc
index 84a00ae4e45f32c74fe06ff11da31ca4f7e64500..1b81d61c2e70059677937b5db3c0ada2b25dc29d 100644
--- a/chrome/browser/metrics/thread_watcher.cc
+++ b/chrome/browser/metrics/thread_watcher.cc
@@ -79,12 +79,11 @@ void ThreadWatcher::StartWatching(const WatchingParams& params) {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
 
   // Create a new thread watcher object for the given thread and activate it.
-  ThreadWatcher* watcher = new ThreadWatcher(params);
+  std::unique_ptr<ThreadWatcher> watcher(new ThreadWatcher(params));
 
-  DCHECK(watcher);
   // If we couldn't register the thread watcher object, we are shutting down,
-  // then don't activate thread watching.
-  if (!ThreadWatcherList::IsRegistered(params.thread_id))
+  // so don't activate thread watching.
+  if (!ThreadWatcherList::Register(std::move(watcher)))
     return;
   watcher->ActivateThreadWatching();

[Jakob Kummerow, 2016/09/01 13:35:17]

I believe using |watcher| after calling std::move() on it is a Bad Idea™.

[Ilya Sherman, 2016/09/01 18:15:18]

Ah, good call -- my bad for missing that!

One possible way to restructure the code: Change ThreadWatcherList::Register()
to also call ActivateThreadWatching().  Another: Have
ThreadWatcherList::Register() return a pointer to the registered watcher, or
null if registration failed.

 }
@@ -224,7 +223,6 @@ void ThreadWatcher::OnCheckResponsiveness(uint64_t ping_sequence_number) {
 
 void ThreadWatcher::Initialize() {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
-  ThreadWatcherList::Register(this);
 
   const std::string response_time_histogram_name =
       "ThreadWatcher.ResponseTime." + thread_name_;
@@ -382,18 +380,14 @@ void ThreadWatcherList::StopWatchingAll() {
 }
 
 // static
-void ThreadWatcherList::Register(ThreadWatcher* watcher) {
+bool ThreadWatcherList::Register(std::unique_ptr<ThreadWatcher> watcher) {
   DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
   if (!g_thread_watcher_list_)
-    return;
-  DCHECK(!g_thread_watcher_list_->Find(watcher->thread_id()));
-  g_thread_watcher_list_->registered_[watcher->thread_id()] = watcher;
-}
-
-// static
-bool ThreadWatcherList::IsRegistered(const BrowserThread::ID thread_id) {
-  DCHECK(WatchDogThread::CurrentlyOnWatchDogThread());
-  return nullptr != ThreadWatcherList::Find(thread_id);
+    return false;
+  content::BrowserThread::ID thread_id = watcher->thread_id();
+  DCHECK(g_thread_watcher_list_->registered_.count(thread_id) == 0);
+  g_thread_watcher_list_->registered_[thread_id] = watcher.release();
+  return true;
 }
 
 // static