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

Side by Side Diff: base/observer_list_threadsafe.h

Issue 8635002: Make ObserverListThreadSafe key its observers by PlatformThreadId instead of MessageLoop. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add comment Created 9 years, 1 month 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 unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « no previous file | base/observer_list_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #ifndef BASE_OBSERVER_LIST_THREADSAFE_H_ 5 #ifndef BASE_OBSERVER_LIST_THREADSAFE_H_
6 #define BASE_OBSERVER_LIST_THREADSAFE_H_ 6 #define BASE_OBSERVER_LIST_THREADSAFE_H_
7 #pragma once 7 #pragma once
8 8
9 #include <algorithm> 9 #include <algorithm>
10 #include <map> 10 #include <map>
11 11
12 #include "base/basictypes.h" 12 #include "base/basictypes.h"
13 #include "base/bind.h" 13 #include "base/bind.h"
14 #include "base/callback_old.h" 14 #include "base/callback_old.h"
15 #include "base/location.h" 15 #include "base/location.h"
16 #include "base/logging.h" 16 #include "base/logging.h"
17 #include "base/memory/ref_counted.h" 17 #include "base/memory/ref_counted.h"
18 #include "base/message_loop.h" 18 #include "base/message_loop.h"
19 #include "base/message_loop_proxy.h" 19 #include "base/message_loop_proxy.h"
20 #include "base/observer_list.h" 20 #include "base/observer_list.h"
21 #include "base/task.h" 21 #include "base/task.h"
22 #include "base/threading/platform_thread.h"
22 23
23 /////////////////////////////////////////////////////////////////////////////// 24 ///////////////////////////////////////////////////////////////////////////////
24 // 25 //
25 // OVERVIEW: 26 // OVERVIEW:
26 // 27 //
27 // A thread-safe container for a list of observers. 28 // A thread-safe container for a list of observers.
28 // This is similar to the observer_list (see observer_list.h), but it 29 // This is similar to the observer_list (see observer_list.h), but it
29 // is more robust for multi-threaded situations. 30 // is more robust for multi-threaded situations.
30 // 31 //
31 // The following use cases are supported: 32 // The following use cases are supported:
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
82 typedef typename ObserverList<ObserverType>::NotificationType 83 typedef typename ObserverList<ObserverType>::NotificationType
83 NotificationType; 84 NotificationType;
84 85
85 ObserverListThreadSafe() 86 ObserverListThreadSafe()
86 : type_(ObserverListBase<ObserverType>::NOTIFY_ALL) {} 87 : type_(ObserverListBase<ObserverType>::NOTIFY_ALL) {}
87 explicit ObserverListThreadSafe(NotificationType type) : type_(type) {} 88 explicit ObserverListThreadSafe(NotificationType type) : type_(type) {}
88 89
89 // Add an observer to the list. An observer should not be added to 90 // Add an observer to the list. An observer should not be added to
90 // the same list more than once. 91 // the same list more than once.
91 void AddObserver(ObserverType* obs) { 92 void AddObserver(ObserverType* obs) {
93 // If there is not a current MessageLoop, it is impossible to notify on it,
94 // so do not add the observer.
95 if (!MessageLoop::current())
96 return;
97
92 ObserverList<ObserverType>* list = NULL; 98 ObserverList<ObserverType>* list = NULL;
93 MessageLoop* loop = MessageLoop::current(); 99 base::PlatformThreadId thread_id = base::PlatformThread::CurrentId();
94 // TODO(mbelshe): Get rid of this check. Its needed right now because
95 // Time currently triggers usage of the ObserverList.
96 // And unittests use time without a MessageLoop.
97 if (!loop)
98 return; // Some unittests may access this without a message loop.
99 { 100 {
100 base::AutoLock lock(list_lock_); 101 base::AutoLock lock(list_lock_);
101 if (observer_lists_.find(loop) == observer_lists_.end()) 102 if (observer_lists_.find(thread_id) == observer_lists_.end())
102 observer_lists_[loop] = new ObserverListContext(type_); 103 observer_lists_[thread_id] = new ObserverListContext(type_);
103 list = &(observer_lists_[loop]->list); 104 list = &(observer_lists_[thread_id]->list);
104 } 105 }
105 list->AddObserver(obs); 106 list->AddObserver(obs);
106 } 107 }
107 108
108 // Remove an observer from the list if it is in the list. 109 // Remove an observer from the list if it is in the list.
109 // If there are pending notifications in-transit to the observer, they will 110 // If there are pending notifications in-transit to the observer, they will
110 // be aborted. 111 // be aborted.
111 // If the observer to be removed is in the list, RemoveObserver MUST 112 // If the observer to be removed is in the list, RemoveObserver MUST
112 // be called from the same thread which called AddObserver. 113 // be called from the same thread which called AddObserver.
113 void RemoveObserver(ObserverType* obs) { 114 void RemoveObserver(ObserverType* obs) {
114 ObserverListContext* context = NULL; 115 ObserverListContext* context = NULL;
115 ObserverList<ObserverType>* list = NULL; 116 ObserverList<ObserverType>* list = NULL;
116 MessageLoop* loop = MessageLoop::current(); 117 base::PlatformThreadId thread_id = base::PlatformThread::CurrentId();
117 if (!loop)
118 return; // On shutdown, it is possible that current() is already null.
119 { 118 {
120 base::AutoLock lock(list_lock_); 119 base::AutoLock lock(list_lock_);
121 typename ObserversListMap::iterator it = observer_lists_.find(loop); 120 typename ObserversListMap::iterator it = observer_lists_.find(thread_id);
122 if (it == observer_lists_.end()) { 121 if (it == observer_lists_.end()) {
123 // This will happen if we try to remove an observer on a thread 122 // This will happen if we try to remove an observer on a thread
124 // we never added an observer for. 123 // we never added an observer for.
125 return; 124 return;
126 } 125 }
127 context = it->second; 126 context = it->second;
128 list = &context->list; 127 list = &context->list;
129 128
130 // If we're about to remove the last observer from the list, 129 // If we're about to remove the last observer from the list,
131 // then we can remove this observer_list entirely. 130 // then we can remove this observer_list entirely.
(...skipping 89 matching lines...) Expand 10 before | Expand all | Expand 10 after
221 // ObserverList. This function MUST be called on the thread which owns 220 // ObserverList. This function MUST be called on the thread which owns
222 // the unsafe ObserverList. 221 // the unsafe ObserverList.
223 template <class Method, class Params> 222 template <class Method, class Params>
224 void NotifyWrapper(ObserverListContext* context, 223 void NotifyWrapper(ObserverListContext* context,
225 const UnboundMethod<ObserverType, Method, Params>& method) { 224 const UnboundMethod<ObserverType, Method, Params>& method) {
226 225
227 // Check that this list still needs notifications. 226 // Check that this list still needs notifications.
228 { 227 {
229 base::AutoLock lock(list_lock_); 228 base::AutoLock lock(list_lock_);
230 typename ObserversListMap::iterator it = 229 typename ObserversListMap::iterator it =
231 observer_lists_.find(MessageLoop::current()); 230 observer_lists_.find(base::PlatformThread::CurrentId());
232 231
233 // The ObserverList could have been removed already. In fact, it could 232 // The ObserverList could have been removed already. In fact, it could
234 // have been removed and then re-added! If the master list's loop 233 // have been removed and then re-added! If the master list's loop
235 // does not match this one, then we do not need to finish this 234 // does not match this one, then we do not need to finish this
236 // notification. 235 // notification.
237 if (it == observer_lists_.end() || it->second != context) 236 if (it == observer_lists_.end() || it->second != context)
238 return; 237 return;
239 } 238 }
240 239
241 { 240 {
242 typename ObserverList<ObserverType>::Iterator it(context->list); 241 typename ObserverList<ObserverType>::Iterator it(context->list);
243 ObserverType* obs; 242 ObserverType* obs;
244 while ((obs = it.GetNext()) != NULL) 243 while ((obs = it.GetNext()) != NULL)
245 method.Run(obs); 244 method.Run(obs);
246 } 245 }
247 246
248 // If there are no more observers on the list, we can now delete it. 247 // If there are no more observers on the list, we can now delete it.
249 if (context->list.size() == 0) { 248 if (context->list.size() == 0) {
250 { 249 {
251 base::AutoLock lock(list_lock_); 250 base::AutoLock lock(list_lock_);
252 // Remove |list| if it's not already removed. 251 // Remove |list| if it's not already removed.
253 // This can happen if multiple observers got removed in a notification. 252 // This can happen if multiple observers got removed in a notification.
254 // See http://crbug.com/55725. 253 // See http://crbug.com/55725.
255 typename ObserversListMap::iterator it = 254 typename ObserversListMap::iterator it =
256 observer_lists_.find(MessageLoop::current()); 255 observer_lists_.find(base::PlatformThread::CurrentId());
257 if (it != observer_lists_.end() && it->second == context) 256 if (it != observer_lists_.end() && it->second == context)
258 observer_lists_.erase(it); 257 observer_lists_.erase(it);
259 } 258 }
260 delete context; 259 delete context;
261 } 260 }
262 } 261 }
263 262
264 typedef std::map<MessageLoop*, ObserverListContext*> ObserversListMap; 263 // Key by PlatformThreadId because in tests, clients can attempt to remove
264 // observers without a MessageLoop. If this were keyed by MessageLoop, that
265 // operation would be silently ignored, leaving garbage in the ObserverList.
266 typedef std::map<base::PlatformThreadId, ObserverListContext*>
267 ObserversListMap;
265 268
266 base::Lock list_lock_; // Protects the observer_lists_. 269 base::Lock list_lock_; // Protects the observer_lists_.
267 ObserversListMap observer_lists_; 270 ObserversListMap observer_lists_;
268 const NotificationType type_; 271 const NotificationType type_;
269 272
270 DISALLOW_COPY_AND_ASSIGN(ObserverListThreadSafe); 273 DISALLOW_COPY_AND_ASSIGN(ObserverListThreadSafe);
271 }; 274 };
272 275
273 #endif // BASE_OBSERVER_LIST_THREADSAFE_H_ 276 #endif // BASE_OBSERVER_LIST_THREADSAFE_H_
OLDNEW
« no previous file with comments | « no previous file | base/observer_list_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698