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

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

2 files changed, 132 insertions, 54 deletions

diff --git a/base/observer_list.h b/base/observer_list.h
index f67df14..0b4c583 100644
--- a/base/observer_list.h
+++ b/base/observer_list.h
@@ -56,8 +56,11 @@
 //
 ///////////////////////////////////////////////////////////////////////////////
 
-template <class ObserverType, bool check_empty = false>
-class ObserverList {
+template <typename ObserverType>
+class ObserverListThreadSafe;
+
+template <class ObserverType>
+class ObserverListBase {
  public:
   // Enumeration of which observers are notified.
   enum NotificationType {
@@ -70,49 +73,11 @@ class ObserverList {
     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 FOREACH_OBSERVER macro defined below.
+  // also the FOR_EACH_OBSERVER macro defined below.
   class Iterator {
    public:
-    Iterator(const ObserverList<ObserverType>& list)
+    Iterator(ObserverListBase<ObserverType>& list)
         : list_(list),
           index_(0),
           max_index_(list.type_ == NOTIFY_ALL ?
@@ -136,15 +101,60 @@ class ObserverList {
     }
 
    private:
-    const ObserverList<ObserverType>& list_;
+    ObserverListBase<ObserverType>& list_;
     size_t index_;
     size_t max_index_;
   };
 
- private:
-  typedef std::vector<ObserverType*> ListType;
+  ObserverListBase() : notify_depth_(0), type_(NOTIFY_ALL) {}
+  explicit ObserverListBase(NotificationType type)
+      : notify_depth_(0), type_(type) {}
 
-  void Compact() const {
+  // 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();
+  }
+
+  void Compact() {
     typename ListType::iterator it = observers_.begin();
     while (it != observers_.end()) {
       if (*it) {
@@ -155,19 +165,42 @@ class ObserverList {
     }
   }
 
-  // These are marked mutable to facilitate having NotifyAll be const.
-  mutable ListType observers_;
-  mutable int notify_depth_;
+ private:
+  friend class ObserverListThreadSafe<ObserverType>;
+
+  typedef std::vector<ObserverType*> ListType;
+
+  ListType observers_;
+  int notify_depth_;
   NotificationType type_;
 
-  friend class ObserverList::Iterator;
+  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) {}
 
-  DISALLOW_EVIL_CONSTRUCTORS(ObserverList);
+  ~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);
+    }
+  }
 };
 
 #define FOR_EACH_OBSERVER(ObserverType, observer_list, func)  \
   do {                                                        \
-    ObserverList<ObserverType>::Iterator it(observer_list);   \
+    ObserverListBase<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 79af96a..6982d3d 100644
--- a/base/observer_list_unittest.cc
+++ b/base/observer_list_unittest.cc
@@ -172,8 +172,6 @@ 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);
@@ -304,3 +302,50 @@ 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