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

Unified Diff: base/observer_list_threadsafe.h

Issue 7024037: Fix bug in ObserverListThreadsafe::RemoveObserver (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 7 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
« no previous file with comments | « base/observer_list.h ('k') | base/observer_list_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/observer_list_threadsafe.h
diff --git a/base/observer_list_threadsafe.h b/base/observer_list_threadsafe.h
index 31e0cae784662df364dae92fa6bd37f1b7f35a6f..883e28503e575939c54d3f1fb2aba596bd2677fc 100644
--- a/base/observer_list_threadsafe.h
+++ b/base/observer_list_threadsafe.h
@@ -83,7 +83,8 @@ class ObserverListThreadSafe
: type_(ObserverListBase<ObserverType>::NOTIFY_ALL) {}
explicit ObserverListThreadSafe(NotificationType type) : type_(type) {}
- // Add an observer to the list.
+ // Add an observer to the list. An observer should not be added to
+ // the same list more than once.
void AddObserver(ObserverType* obs) {
ObserverList<ObserverType>* list = NULL;
MessageLoop* loop = MessageLoop::current();
@@ -101,11 +102,11 @@ class ObserverListThreadSafe
list->AddObserver(obs);
}
- // Remove an observer from the list.
+ // Remove an observer from the list if it is in the list.
// If there are pending notifications in-transit to the observer, they will
// be aborted.
- // RemoveObserver MUST be called from the same thread which called
- // AddObserver.
+ // If the observer to be removed is in the list, RemoveObserver MUST
+ // be called from the same thread which called AddObserver.
void RemoveObserver(ObserverType* obs) {
ObserverList<ObserverType>* list = NULL;
MessageLoop* loop = MessageLoop::current();
@@ -113,16 +114,18 @@ class ObserverListThreadSafe
return; // On shutdown, it is possible that current() is already null.
{
base::AutoLock lock(list_lock_);
- list = observer_lists_[loop];
- if (!list) {
- NOTREACHED() << "RemoveObserver called on for unknown thread";
+ typename ObserversListMap::iterator it = observer_lists_.find(loop);
+ if (it == observer_lists_.end()) {
+ // This may happen if we try to remove an observer on a thread
+ // we never added an observer for.
return;
}
+ list = it->second;
// If we're about to remove the last observer from the list,
// then we can remove this observer_list entirely.
- if (list->size() == 1)
- observer_lists_.erase(loop);
+ if (list->HasObserver(obs) && list->size() == 1)
+ observer_lists_.erase(it);
}
list->RemoveObserver(obs);
« no previous file with comments | « base/observer_list.h ('k') | base/observer_list_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698