Fix bug in ObserverListThreadsafe::RemoveObserver

base/observer_list_threadsafe.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_OBSERVER_LIST_THREADSAFE_H_
 #define BASE_OBSERVER_LIST_THREADSAFE_H_
 #pragma once
 
 #include <algorithm>
 #include <map>
@@ skipping 65 matching lines @@
         ObserverListThreadSafe<ObserverType>,
         ObserverListThreadSafeTraits<ObserverType> > {
  public:
   typedef typename ObserverList<ObserverType>::NotificationType
       NotificationType;
 
   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();
     // TODO(mbelshe): Get rid of this check.  Its needed right now because
     //                Time currently triggers usage of the ObserverList.
     //                And unittests use time without a MessageLoop.
     if (!loop)
       return;  // Some unittests may access this without a message loop.
     {
       base::AutoLock lock(list_lock_);
       if (observer_lists_.find(loop) == observer_lists_.end())
         observer_lists_[loop] = new ObserverList<ObserverType>(type_);
       list = observer_lists_[loop];
     }
     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();
     if (!loop)
       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);
 
     // If RemoveObserver is called from a notification, the size will be
     // nonzero.  Instead of deleting here, the NotifyWrapper will delete
     // when it finishes iterating.
     if (list->size() == 0)
       delete list;
   }
 
@@ skipping 90 matching lines @@
 
   // These are marked mutable to facilitate having NotifyAll be const.
   base::Lock list_lock_;  // Protects the observer_lists_.
   ObserversListMap observer_lists_;
   const NotificationType type_;
 
   DISALLOW_COPY_AND_ASSIGN(ObserverListThreadSafe);
 };
 
 #endif  // BASE_OBSERVER_LIST_THREADSAFE_H_