Use MessageLoopProxy instead of MessageLoop to dispatch notifications in ObserverListThreadsafe.

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>
 
 #include "base/basictypes.h"
 #include "base/callback_old.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/observer_list.h"
 #include "base/task.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 //
 // OVERVIEW:
 //
 //   A thread-safe container for a list of observers.
 //   This is similar to the observer_list (see observer_list.h), but it
 //   is more robust for multi-threaded situations.
@@ skipping 62 matching lines @@
     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];
+        observer_lists_[loop] = new ObserverListContext(type_);

[darin (slow to review), 2011/08/09 19:32:31]

it is very surprising that insert did not work.  i'm pretty sure we use that
elsewhere.  what was the failure?

[adamk, 2011/08/09 20:33:40]

The failure was due to NULL not being "typed" enough.  I originally ran into
this when using std::make_pair(), and fixed it for all known configurations by
using std::map::value_type, but apparently that wasn't good enough for VS2010. 
If you'd like, I can _probably_ get it to compile with the addition of a
static_cast, but it makes the code even harder to read. Up to you.

+      list = &(observer_lists_[loop]->list);
     }
     list->AddObserver(obs);
   }
 
   // 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.
   // 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) {
+    ObserverListContext* context = NULL;
     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_);
       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
+        // This will happen if we try to remove an observer on a thread
         // we never added an observer for.
         return;
       }
-      list = it->second;
+      context = it->second;
+      list = &context->list;
 
       // If we're about to remove the last observer from the list,
       // then we can remove this observer_list entirely.
       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;
+      delete context;
   }
 
   // Notify methods.
   // Make a thread-safe callback to each Observer in the list.
   // Note, these calls are effectively asynchronous.  You cannot assume
   // that at the completion of the Notify call that all Observers have
   // been Notified.  The notification may still be pending delivery.
   template <class Method>
   void Notify(Method m) {
     UnboundMethod<ObserverType, Method, Tuple0> method(m, MakeTuple());
@@ skipping 26 matching lines @@
         m, MakeTuple(a, b, c, d));
     Notify<Method, Tuple4<A, B, C, D> >(method);
   }
 
   // TODO(mbelshe):  Add more wrappers for Notify() with more arguments.
 
  private:
   // See comment above ObserverListThreadSafeTraits' definition.
   friend struct ObserverListThreadSafeTraits<ObserverType>;
 
+  struct ObserverListContext {
+    explicit ObserverListContext(NotificationType type)
+        : loop(base::MessageLoopProxy::CreateForCurrentThread()),
+          list(type) {
+    }
+
+    scoped_refptr<base::MessageLoopProxy> loop;
+    ObserverList<ObserverType> list;
+
+    DISALLOW_COPY_AND_ASSIGN(ObserverListContext);
+  };
+
   ~ObserverListThreadSafe() {
     typename ObserversListMap::const_iterator it;
     for (it = observer_lists_.begin(); it != observer_lists_.end(); ++it)
       delete (*it).second;
     observer_lists_.clear();
   }
 
   template <class Method, class Params>
   void Notify(const UnboundMethod<ObserverType, Method, Params>& method) {
     base::AutoLock lock(list_lock_);
     typename ObserversListMap::iterator it;
     for (it = observer_lists_.begin(); it != observer_lists_.end(); ++it) {
-      MessageLoop* loop = (*it).first;
-      ObserverList<ObserverType>* list = (*it).second;
-      loop->PostTask(
+      ObserverListContext* context = (*it).second;
+      context->loop->PostTask(
           FROM_HERE,
           NewRunnableMethod(this,
               &ObserverListThreadSafe<ObserverType>::
-                 template NotifyWrapper<Method, Params>, list, method));
+                 template NotifyWrapper<Method, Params>, context, method));
     }
   }
 
   // Wrapper which is called to fire the notifications for each thread's
   // ObserverList.  This function MUST be called on the thread which owns
   // the unsafe ObserverList.
   template <class Method, class Params>
-  void NotifyWrapper(ObserverList<ObserverType>* list,
+  void NotifyWrapper(ObserverListContext* context,
       const UnboundMethod<ObserverType, Method, Params>& method) {
 
     // Check that this list still needs notifications.
     {
       base::AutoLock lock(list_lock_);
       typename ObserversListMap::iterator it =
           observer_lists_.find(MessageLoop::current());
 
       // The ObserverList could have been removed already.  In fact, it could
       // have been removed and then re-added!  If the master list's loop
       // does not match this one, then we do not need to finish this
       // notification.
-      if (it == observer_lists_.end() || it->second != list)
+      if (it == observer_lists_.end() || it->second != context)
         return;
     }
 
     {
-      typename ObserverList<ObserverType>::Iterator it(*list);
+      typename ObserverList<ObserverType>::Iterator it(context->list);
       ObserverType* obs;
       while ((obs = it.GetNext()) != NULL)
         method.Run(obs);
     }
 
     // If there are no more observers on the list, we can now delete it.
-    if (list->size() == 0) {
+    if (context->list.size() == 0) {
       {
         base::AutoLock lock(list_lock_);
         // Remove |list| if it's not already removed.
         // This can happen if multiple observers got removed in a notification.
         // See http://crbug.com/55725.
         typename ObserversListMap::iterator it =
             observer_lists_.find(MessageLoop::current());
-        if (it != observer_lists_.end() && it->second == list)
+        if (it != observer_lists_.end() && it->second == context)
           observer_lists_.erase(it);
       }
-      delete list;
+      delete context;
     }
   }
 
-  typedef std::map<MessageLoop*, ObserverList<ObserverType>*> ObserversListMap;
+  typedef std::map<MessageLoop*, ObserverListContext*> ObserversListMap;
 
-  // 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_