Adds the ability for ObserverList to not notify observers added during

notification. I need this for bookmarks. If a new observer is added
while the bookmark model is in the process of sending out notification
the newly added observer gets confused.

BUG=674
TEST=none

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

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

2 files changed, 67 insertions, 5 deletions

diff --git a/base/observer_list.h b/base/observer_list.h
index 52571df..9f8b5cf 100644
--- a/base/observer_list.h
+++ b/base/observer_list.h
@@ -5,8 +5,9 @@
 #ifndef BASE_OBSERVER_LIST_H__
 #define BASE_OBSERVER_LIST_H__
 
-#include <vector>
 #include <algorithm>
+#include <limits>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ -52,12 +53,25 @@
 //     ObserverList<Observer> observer_list_;
 //   };
 //
+//
 ///////////////////////////////////////////////////////////////////////////////
 
 template <class ObserverType, bool check_empty = false>
 class ObserverList {
  public:
-  ObserverList() : notify_depth_(0) {}
+  // Enumeration of which observers are notified.
+  enum NotificationType {
+    // Specifies that any observers added during notification are notified.
+    // This is the default type if non type is provided to the constructor.
+    NOTIFY_ALL,
+
+    // Specifies that observers added while sending out notification are not
+    // notified.
+    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) {
@@ -98,7 +112,12 @@ class ObserverList {
   // also the FOREACH_OBSERVER macro defined below.
   class Iterator {
    public:
-    Iterator(const ObserverList<ObserverType>& list) : list_(list), index_(0) {
+    Iterator(const ObserverList<ObserverType>& list)
+        : list_(list),
+          index_(0),
+          max_index_(list.type_ == NOTIFY_ALL ?
+                     std::numeric_limits<size_t>::max() :
+                     list.observers_.size()) {
       ++list_.notify_depth_;
     }
 
@@ -110,14 +129,16 @@ class ObserverList {
     ObserverType* GetNext() {
       ListType& observers = list_.observers_;
       // Advance if the current element is null
-      while (index_ < observers.size() && !observers[index_])
+      size_t max_index = std::min(max_index_, observers.size());
+      while (index_ < max_index && !observers[index_])
         ++index_;
-      return index_ < observers.size() ? observers[index_++] : NULL;
+      return index_ < max_index ? observers[index_++] : NULL;
     }
 
    private:
     const ObserverList<ObserverType>& list_;
     size_t index_;
+    size_t max_index_;
   };
 
  private:
@@ -137,6 +158,7 @@ class ObserverList {
   // These are marked mutable to facilitate having NotifyAll be const.
   mutable ListType observers_;
   mutable int notify_depth_;
+  NotificationType type_;
 
   friend class ObserverList::Iterator;
 
diff --git a/base/observer_list_unittest.cc b/base/observer_list_unittest.cc
index 8f69256..1d3668d 100644
--- a/base/observer_list_unittest.cc
+++ b/base/observer_list_unittest.cc
@@ -60,6 +60,26 @@ class ThreadSafeDisrupter : public Foo {
   Foo* doomed_;
 };
 
+class AddInObserve : public Foo {
+ public:
+  AddInObserve(ObserverList<Foo>* observer_list)
+      : added(false),
+        observer_list(observer_list),
+        adder(1) {
+  }
+  virtual void Observe(int x) {
+    if (!added) {
+      added = true;
+      observer_list->AddObserver(&adder);
+    }
+  }
+
+  bool added;
+  ObserverList<Foo>* observer_list;
+  Adder adder;
+};
+
+
 class ObserverListThreadSafeTest : public testing::Test {
 };
 
@@ -262,3 +282,23 @@ TEST(ObserverListThreadSafeTest, CrossThreadNotifications) {
   // the main thread and all 3 observer threads.
   ThreadSafeObserverHarness(3, true);
 }
+
+TEST(ObserverListTest, Existing) {
+  ObserverList<Foo> observer_list(ObserverList<Foo>::NOTIFY_EXISTING_ONLY);
+  Adder a(1);
+  AddInObserve b(&observer_list);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(1));
+
+  EXPECT_TRUE(b.added);
+  // B's adder should not have been notified because it was added during
+  // notificaiton.
+  EXPECT_EQ(0, b.adder.total);
+
+  // Notify again to make sure b's adder is notified.
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(1));
+  EXPECT_EQ(1, b.adder.total);
+}