Revert 39942 - Switch NetworkChangeNotifier implementations to use ObserverList.

Fix up observer list so we can use FOR_EACH_OBSERVER when check_empty is set.
Clean up the ObserverList API a bit, replacing GetElementAt() with HasObserver() and Clear().
BUG=36590

Review URL: http://codereview.chromium.org/652205

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/661029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39945 0039d316-1c4b-4281-b951-d872f2087c98

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) {