diff options
-rw-r--r-- | base/observer_list.h | 137 | ||||
-rw-r--r-- | base/observer_list_unittest.cc | 49 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_shelf_model.cc | 3 | ||||
-rw-r--r-- | chrome/browser/tabs/tab_strip_model.cc | 6 | ||||
-rw-r--r-- | net/base/network_change_notifier_helper.cc | 45 | ||||
-rw-r--r-- | net/base/network_change_notifier_helper.h | 44 | ||||
-rw-r--r-- | net/base/network_change_notifier_linux.cc | 2 | ||||
-rw-r--r-- | net/base/network_change_notifier_linux.h | 9 | ||||
-rw-r--r-- | net/base/network_change_notifier_mac.h | 14 | ||||
-rw-r--r-- | net/base/network_change_notifier_win.cc | 2 | ||||
-rw-r--r-- | net/base/network_change_notifier_win.h | 16 | ||||
-rw-r--r-- | net/net.gyp | 2 | ||||
-rw-r--r-- | webkit/appcache/appcache_group.cc | 8 |
13 files changed, 174 insertions, 163 deletions
diff --git a/base/observer_list.h b/base/observer_list.h index 0b4c583..f67df14 100644 --- a/base/observer_list.h +++ b/base/observer_list.h @@ -56,11 +56,8 @@ // /////////////////////////////////////////////////////////////////////////////// -template <typename ObserverType> -class ObserverListThreadSafe; - -template <class ObserverType> -class ObserverListBase { +template <class ObserverType, bool check_empty = false> +class ObserverList { public: // Enumeration of which observers are notified. enum NotificationType { @@ -73,11 +70,49 @@ class ObserverListBase { NOTIFY_EXISTING_ONLY }; + ObserverList() : notify_depth_(0), type_(NOTIFY_ALL) {} + ObserverList(NotificationType type) : notify_depth_(0), type_(type) {} + ~ObserverList() { + // When check_empty is true, assert that the list is empty on destruction. + if (check_empty) { + Compact(); + DCHECK_EQ(observers_.size(), 0U); + } + } + + // Add an observer to the list. + void AddObserver(ObserverType* obs) { + DCHECK(find(observers_.begin(), observers_.end(), obs) == observers_.end()) + << "Observers can only be added once!"; + observers_.push_back(obs); + } + + // Remove an observer from the list. + void RemoveObserver(ObserverType* obs) { + typename ListType::iterator it = + std::find(observers_.begin(), observers_.end(), obs); + if (it != observers_.end()) { + if (notify_depth_) { + *it = 0; + } else { + observers_.erase(it); + } + } + } + + size_t size() const { + return observers_.size(); + } + + ObserverType* GetElementAt(int index) const { + return observers_[index]; + } + // An iterator class that can be used to access the list of observers. See - // also the FOR_EACH_OBSERVER macro defined below. + // also the FOREACH_OBSERVER macro defined below. class Iterator { public: - Iterator(ObserverListBase<ObserverType>& list) + Iterator(const ObserverList<ObserverType>& list) : list_(list), index_(0), max_index_(list.type_ == NOTIFY_ALL ? @@ -101,60 +136,15 @@ class ObserverListBase { } private: - ObserverListBase<ObserverType>& list_; + const ObserverList<ObserverType>& list_; size_t index_; size_t max_index_; }; - ObserverListBase() : notify_depth_(0), type_(NOTIFY_ALL) {} - explicit ObserverListBase(NotificationType type) - : notify_depth_(0), type_(type) {} - - // Add an observer to the list. - void AddObserver(ObserverType* obs) { - DCHECK(find(observers_.begin(), observers_.end(), obs) == observers_.end()) - << "Observers can only be added once!"; - observers_.push_back(obs); - } - - // Remove an observer from the list. - void RemoveObserver(ObserverType* obs) { - typename ListType::iterator it = - std::find(observers_.begin(), observers_.end(), obs); - if (it != observers_.end()) { - if (notify_depth_) { - *it = 0; - } else { - observers_.erase(it); - } - } - } - - bool HasObserver(ObserverType* observer) const { - for (size_t i = 0; i < observers_.size(); ++i) { - if (observers_[i] == observer) - return true; - } - return false; - } - - void Clear() { - if (notify_depth_) { - for (typename ListType::iterator it = observers_.begin(); - it != observers_.end(); ++it) { - *it = 0; - } - } else { - observers_.clear(); - } - } - - protected: - size_t size() const { - return observers_.size(); - } + private: + typedef std::vector<ObserverType*> ListType; - void Compact() { + void Compact() const { typename ListType::iterator it = observers_.begin(); while (it != observers_.end()) { if (*it) { @@ -165,42 +155,19 @@ class ObserverListBase { } } - private: - friend class ObserverListThreadSafe<ObserverType>; - - typedef std::vector<ObserverType*> ListType; - - ListType observers_; - int notify_depth_; + // These are marked mutable to facilitate having NotifyAll be const. + mutable ListType observers_; + mutable int notify_depth_; NotificationType type_; - friend class ObserverListBase::Iterator; - - DISALLOW_COPY_AND_ASSIGN(ObserverListBase); -}; - -template <class ObserverType, bool check_empty = false> -class ObserverList : public ObserverListBase<ObserverType> { - public: - typedef typename ObserverListBase<ObserverType>::NotificationType - NotificationType; - - ObserverList() {} - explicit ObserverList(NotificationType type) - : ObserverListBase<ObserverType>(type) {} + friend class ObserverList::Iterator; - ~ObserverList() { - // When check_empty is true, assert that the list is empty on destruction. - if (check_empty) { - ObserverListBase<ObserverType>::Compact(); - DCHECK_EQ(ObserverListBase<ObserverType>::size(), 0U); - } - } + DISALLOW_EVIL_CONSTRUCTORS(ObserverList); }; #define FOR_EACH_OBSERVER(ObserverType, observer_list, func) \ do { \ - ObserverListBase<ObserverType>::Iterator it(observer_list); \ + ObserverList<ObserverType>::Iterator it(observer_list); \ ObserverType* obs; \ while ((obs = it.GetNext()) != NULL) \ obs->func; \ diff --git a/base/observer_list_unittest.cc b/base/observer_list_unittest.cc index 6982d3d..79af96a 100644 --- a/base/observer_list_unittest.cc +++ b/base/observer_list_unittest.cc @@ -172,6 +172,8 @@ class AddRemoveThread : public PlatformThread::Delegate, ScopedRunnableMethodFactory<AddRemoveThread>* factory_; }; +} // namespace + TEST(ObserverListTest, BasicTest) { ObserverList<Foo> observer_list; Adder a(1), b(-1), c(1), d(-1); @@ -302,50 +304,3 @@ TEST(ObserverListTest, Existing) { FOR_EACH_OBSERVER(Foo, observer_list, Observe(1)); EXPECT_EQ(1, b.adder.total); } - -class AddInClearObserve : public Foo { - public: - explicit AddInClearObserve(ObserverList<Foo>* list) - : list_(list), added_(false), adder_(1) {} - - virtual void Observe(int /* x */) { - list_->Clear(); - list_->AddObserver(&adder_); - added_ = true; - } - - bool added() const { return added_; } - const Adder& adder() const { return adder_; } - - private: - ObserverList<Foo>* const list_; - - bool added_; - Adder adder_; -}; - -TEST(ObserverListTest, ClearNotifyAll) { - ObserverList<Foo> observer_list; - AddInClearObserve a(&observer_list); - - observer_list.AddObserver(&a); - - FOR_EACH_OBSERVER(Foo, observer_list, Observe(1)); - EXPECT_TRUE(a.added()); - EXPECT_EQ(1, a.adder().total) - << "Adder should observe once and have sum of 1."; -} - -TEST(ObserverListTest, ClearNotifyExistingOnly) { - ObserverList<Foo> observer_list(ObserverList<Foo>::NOTIFY_EXISTING_ONLY); - AddInClearObserve a(&observer_list); - - observer_list.AddObserver(&a); - - FOR_EACH_OBSERVER(Foo, observer_list, Observe(1)); - EXPECT_TRUE(a.added()); - EXPECT_EQ(0, a.adder().total) - << "Adder should not observe, so sum should still be 0."; -} - -} // namespace diff --git a/chrome/browser/extensions/extension_shelf_model.cc b/chrome/browser/extensions/extension_shelf_model.cc index 51a8ce1..dc584d8 100644 --- a/chrome/browser/extensions/extension_shelf_model.cc +++ b/chrome/browser/extensions/extension_shelf_model.cc @@ -44,7 +44,8 @@ ExtensionShelfModel::~ExtensionShelfModel() { FOR_EACH_OBSERVER(ExtensionShelfModelObserver, observers_, ShelfModelDeleting()); - observers_.Clear(); + while (observers_.size()) + observers_.RemoveObserver(observers_.GetElementAt(0)); for (iterator t = toolstrips_.begin(); t != toolstrips_.end(); ++t) delete t->host; diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc index 2fba4b5..3b216d4 100644 --- a/chrome/browser/tabs/tab_strip_model.cc +++ b/chrome/browser/tabs/tab_strip_model.cc @@ -86,7 +86,11 @@ bool TabStripModel::HasNonPhantomTabs() const { } bool TabStripModel::HasObserver(TabStripModelObserver* observer) { - return observers_.HasObserver(observer); + for (size_t i = 0; i < observers_.size(); ++i) { + if (observers_.GetElementAt(i) == observer) + return true; + } + return false; } bool TabStripModel::ContainsIndex(int index) const { diff --git a/net/base/network_change_notifier_helper.cc b/net/base/network_change_notifier_helper.cc new file mode 100644 index 0000000..052e3ef8 --- /dev/null +++ b/net/base/network_change_notifier_helper.cc @@ -0,0 +1,45 @@ +// Copyright (c) 2009 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. + +#include "net/base/network_change_notifier_helper.h" +#include <algorithm> +#include "base/logging.h" + +namespace net { + +namespace internal { + +NetworkChangeNotifierHelper::NetworkChangeNotifierHelper() + : is_notifying_observers_(false) {} + +NetworkChangeNotifierHelper::~NetworkChangeNotifierHelper() { + // TODO(willchan): Re-enable this DCHECK after fixing http://crbug.com/34391 + // since we're leaking URLRequestContextGetters that cause this DCHECK to + // fire. + // DCHECK(observers_.empty()); +} + +void NetworkChangeNotifierHelper::AddObserver(Observer* observer) { + DCHECK(!is_notifying_observers_); + observers_.push_back(observer); +} + +void NetworkChangeNotifierHelper::RemoveObserver(Observer* observer) { + DCHECK(!is_notifying_observers_); + observers_.erase(std::remove(observers_.begin(), observers_.end(), observer)); +} + +void NetworkChangeNotifierHelper::OnIPAddressChanged() { + DCHECK(!is_notifying_observers_); + is_notifying_observers_ = true; + for (std::vector<Observer*>::iterator it = observers_.begin(); + it != observers_.end(); ++it) { + (*it)->OnIPAddressChanged(); + } + is_notifying_observers_ = false; +} + +} // namespace internal + +} // namespace net diff --git a/net/base/network_change_notifier_helper.h b/net/base/network_change_notifier_helper.h new file mode 100644 index 0000000..f860c45 --- /dev/null +++ b/net/base/network_change_notifier_helper.h @@ -0,0 +1,44 @@ +// Copyright (c) 2009 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. + +// NetworkChangeNotifierHelper is a helper class that assists in implementing +// base functionality for a NetworkChangeNotifier implementation. In +// particular, it manages adding/removing observers and sending them +// notifications of event changes. + +#ifndef NET_BASE_NETWORK_CHANGE_NOTIFIER_HELPER_H_ +#define NET_BASE_NETWORK_CHANGE_NOTIFIER_HELPER_H_ + +#include <vector> +#include "base/basictypes.h" +#include "net/base/network_change_notifier.h" + +namespace net { + +namespace internal { + +class NetworkChangeNotifierHelper { + public: + typedef NetworkChangeNotifier::Observer Observer; + + NetworkChangeNotifierHelper(); + ~NetworkChangeNotifierHelper(); + + void AddObserver(Observer* observer); + void RemoveObserver(Observer* observer); + + void OnIPAddressChanged(); + + private: + bool is_notifying_observers_; + std::vector<Observer*> observers_; + + DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierHelper); +}; + +} // namespace internal + +} // namespace net + +#endif // NET_BASE_NETWORK_CHANGE_NOTIFIER_HELPER_H_ diff --git a/net/base/network_change_notifier_linux.cc b/net/base/network_change_notifier_linux.cc index e6f5453..2a85c15 100644 --- a/net/base/network_change_notifier_linux.cc +++ b/net/base/network_change_notifier_linux.cc @@ -57,7 +57,7 @@ void NetworkChangeNotifierLinux::ListenForNotifications() { int rv = ReadNotificationMessage(buf, arraysize(buf)); while (rv > 0 ) { if (HandleNetlinkMessage(buf, rv)) - FOR_EACH_OBSERVER(Observer, observers_, OnIPAddressChanged()); + helper_.OnIPAddressChanged(); rv = ReadNotificationMessage(buf, arraysize(buf)); } diff --git a/net/base/network_change_notifier_linux.h b/net/base/network_change_notifier_linux.h index 91d73e8..323ea55 100644 --- a/net/base/network_change_notifier_linux.h +++ b/net/base/network_change_notifier_linux.h @@ -7,8 +7,7 @@ #include "base/basictypes.h" #include "base/message_loop.h" -#include "base/observer_list.h" -#include "net/base/network_change_notifier.h" +#include "net/base/network_change_notifier_helper.h" namespace net { @@ -20,11 +19,11 @@ class NetworkChangeNotifierLinux // NetworkChangeNotifier methods: virtual void AddObserver(Observer* observer) { - observers_.AddObserver(observer); + helper_.AddObserver(observer); } virtual void RemoveObserver(Observer* observer) { - observers_.RemoveObserver(observer); + helper_.RemoveObserver(observer); } // MessageLoopForIO::Watcher methods: @@ -47,7 +46,7 @@ class NetworkChangeNotifierLinux // Handles the netlink message and notifies the observers. void HandleNotifications(const char* buf, size_t len); - ObserverList<Observer, true> observers_; + internal::NetworkChangeNotifierHelper helper_; int netlink_fd_; // This is the netlink socket descriptor. MessageLoopForIO* const loop_; diff --git a/net/base/network_change_notifier_mac.h b/net/base/network_change_notifier_mac.h index 44d4806..5e777f4 100644 --- a/net/base/network_change_notifier_mac.h +++ b/net/base/network_change_notifier_mac.h @@ -6,11 +6,11 @@ #define NET_BASE_NETWORK_CHANGE_NOTIFIER_MAC_H_ #include "base/basictypes.h" -#include "base/observer_list.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/task.h" #include "net/base/network_change_notifier.h" +#include "net/base/network_change_notifier_helper.h" class MessageLoop; namespace base { @@ -23,18 +23,16 @@ class NetworkChangeNotifierMac : public NetworkChangeNotifier { public: NetworkChangeNotifierMac(); - void OnIPAddressChanged() { - FOR_EACH_OBSERVER(Observer, observers_, OnIPAddressChanged()); - } + void OnIPAddressChanged() { helper_.OnIPAddressChanged(); } // NetworkChangeNotifier methods: virtual void AddObserver(Observer* observer) { - observers_.AddObserver(observer); + helper_.AddObserver(observer); } virtual void RemoveObserver(Observer* observer) { - observers_.RemoveObserver(observer); + helper_.RemoveObserver(observer); } private: @@ -51,9 +49,7 @@ class NetworkChangeNotifierMac : public NetworkChangeNotifier { // Receives the OS X network change notifications on this thread. scoped_ptr<base::Thread> notifier_thread_; - // TODO(willchan): Fix the URLRequestContextGetter leaks and flip the false to - // true so we assert that all observers have been removed. - ObserverList<Observer, false> observers_; + internal::NetworkChangeNotifierHelper helper_; // Used to initialize the notifier thread. ScopedRunnableMethodFactory<NetworkChangeNotifierMac> method_factory_; diff --git a/net/base/network_change_notifier_win.cc b/net/base/network_change_notifier_win.cc index d139eb0f..e52bd46 100644 --- a/net/base/network_change_notifier_win.cc +++ b/net/base/network_change_notifier_win.cc @@ -3,11 +3,9 @@ // found in the LICENSE file. #include "net/base/network_change_notifier_win.h" - #include <iphlpapi.h> #include <windows.h> #include <winsock2.h> - #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/logging.h" diff --git a/net/base/network_change_notifier_win.h b/net/base/network_change_notifier_win.h index 0f1f8d4..b11ec0d 100644 --- a/net/base/network_change_notifier_win.h +++ b/net/base/network_change_notifier_win.h @@ -7,8 +7,7 @@ #include "base/basictypes.h" #include "base/object_watcher.h" -#include "base/observer_list.h" -#include "net/base/network_change_notifier.h" +#include "net/base/network_change_notifier_helper.h" namespace net { @@ -17,19 +16,16 @@ class NetworkChangeNotifierWin : public NetworkChangeNotifier { NetworkChangeNotifierWin(); // Called by NetworkChangeNotifierWin::Impl. - void OnIPAddressChanged() { - FOR_EACH_OBSERVER(Observer, observers_, OnIPAddressChanged()); - } - + void OnIPAddressChanged() { helper_.OnIPAddressChanged(); } // NetworkChangeNotifier methods: virtual void AddObserver(Observer* observer) { - observers_.AddObserver(observer); + helper_.AddObserver(observer); } virtual void RemoveObserver(Observer* observer) { - observers_.RemoveObserver(observer); + helper_.RemoveObserver(observer); } private: @@ -37,9 +33,7 @@ class NetworkChangeNotifierWin : public NetworkChangeNotifier { virtual ~NetworkChangeNotifierWin(); - // TODO(willchan): Fix the URLRequestContextGetter leaks and flip the false to - // true so we assert that all observers have been removed. - ObserverList<Observer, false> observers_; + internal::NetworkChangeNotifierHelper helper_; scoped_ptr<Impl> impl_; DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierWin); diff --git a/net/net.gyp b/net/net.gyp index 7cabfc7..3fc9653 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -107,6 +107,8 @@ 'base/net_util_win.cc', 'base/network_change_notifier.cc', 'base/network_change_notifier.h', + 'base/network_change_notifier_helper.cc', + 'base/network_change_notifier_helper.h', 'base/network_change_notifier_linux.cc', 'base/network_change_notifier_linux.h', 'base/network_change_notifier_mac.cc', diff --git a/webkit/appcache/appcache_group.cc b/webkit/appcache/appcache_group.cc index 8063b911..fd22563 100644 --- a/webkit/appcache/appcache_group.cc +++ b/webkit/appcache/appcache_group.cc @@ -215,7 +215,13 @@ void AppCacheGroup::RunQueuedUpdates() { bool AppCacheGroup::FindObserver(UpdateObserver* find_me, const ObserverList<UpdateObserver>& observer_list) { - return observer_list.HasObserver(find_me); + ObserverList<UpdateObserver>::Iterator it(observer_list); + UpdateObserver* obs; + while ((obs = it.GetNext()) != NULL) { + if (obs == find_me) + return true; + } + return false; } void AppCacheGroup::ScheduleUpdateRestart(int delay_ms) { |