Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(32)

Unified Diff: chrome/browser/metrics/thread_watcher.cc

Issue 2300133002: Fix memory leak in ThreadWatcher. (Closed)
Patch Set: Use unique_ptr; return ptr from Register Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/metrics/thread_watcher.cc
diff --git a/chrome/browser/metrics/thread_watcher.cc b/chrome/browser/metrics/thread_watcher.cc
index b7215ae52e2862521a4affb475f9481770df1fb5..7aebfceb668d0ea5edb0e8fed81bef6aaa1bfc73 100644
--- a/chrome/browser/metrics/thread_watcher.cc
+++ b/chrome/browser/metrics/thread_watcher.cc
@@ -79,14 +79,15 @@ 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))
- return;
- watcher->ActivateThreadWatching();
+ // so don't activate thread watching.
+ ThreadWatcher* registered_watcher =
+ ThreadWatcherList::Register(std::move(watcher));
+ if (registered_watcher != nullptr) {
+ registered_watcher->ActivateThreadWatching();
+ }
Ilya Sherman 2016/09/01 20:30:25 nit: No need for curlies here.
Joshua LeVasseur 2016/09/01 21:28:22 Done.
}
void ThreadWatcher::ActivateThreadWatching() {
@@ -224,7 +225,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 +382,14 @@ void ThreadWatcherList::StopWatchingAll() {
}
// static
-void ThreadWatcherList::Register(ThreadWatcher* watcher) {
+ThreadWatcher* 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 nullptr;
+ content::BrowserThread::ID thread_id = watcher->thread_id();
+ DCHECK(g_thread_watcher_list_->registered_.count(thread_id) == 0);
+ return g_thread_watcher_list_->registered_[thread_id] = watcher.release();
}
// static

Powered by Google App Engine
This is Rietveld 408576698