Base ObserverList: Add basic support for standard C++ iterators.

base/observer_list_unittest.cc

Index: base/observer_list_unittest.cc
diff --git a/base/observer_list_unittest.cc b/base/observer_list_unittest.cc
index 097a2ed28b15490b3f1f36daca349aa199645858..757b3c9b21013601ab1a0258778ce27afc614175 100644
--- a/base/observer_list_unittest.cc
+++ b/base/observer_list_unittest.cc
@@ -22,13 +22,17 @@ class Foo {
  public:
   virtual void Observe(int x) = 0;
   virtual ~Foo() {}
+  virtual int GetValue() const { return 0; }
 };
 
 class Adder : public Foo {
  public:
   explicit Adder(int scaler) : total(0), scaler_(scaler) {}
-  void Observe(int x) override { total += x * scaler_; }
   ~Adder() override {}
+
+  void Observe(int x) override { total += x * scaler_; }
+  int GetValue() const override { return total; }
+
   int total;
 
  private:
@@ -49,6 +53,16 @@ class Disrupter : public Foo {
   Foo* doomed_;
 };
 
+class DisrupterSelf : public Foo {
+ public:
+  DisrupterSelf(ObserverList<Foo>* list) : list_(list) {}
+  ~DisrupterSelf() override {}
+  void Observe(int x) override { list_->RemoveObserver(this); }
+
+ private:
+  ObserverList<Foo>* list_;
+};
+
 class ThreadSafeDisrupter : public Foo {
  public:
   ThreadSafeDisrupter(ObserverListThreadSafe<Foo>* list, Foo* doomed)
@@ -176,6 +190,8 @@ class AddRemoveThread : public PlatformThread::Delegate,
   base::WeakPtrFactory<AddRemoveThread> weak_factory_;
 };
 
+}  // namespace
+
 TEST(ObserverListTest, BasicTest) {
   ObserverList<Foo> observer_list;
   Adder a(1), b(-1), c(1), d(-1), e(-1);
@@ -205,6 +221,48 @@ TEST(ObserverListTest, BasicTest) {
   EXPECT_EQ(0, e.total);
 }
 
+TEST(ObserverListTest, DesruptSelf) {

[dcheng, 2016/10/05 07:08:24]

Nit: Desrupt -> Disrupt

[loyso (OOO), 2016/10/07 03:45:08]

Done.

+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  DisrupterSelf evil(&observer_list);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(10));
+
+  observer_list.AddObserver(&evil);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(10));
+
+  EXPECT_EQ(20, a.total);
+  EXPECT_EQ(-20, b.total);
+  EXPECT_EQ(10, c.total);
+  EXPECT_EQ(-10, d.total);
+}
+
+TEST(ObserverListTest, DesruptBefore) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  Disrupter evil(&observer_list, &b);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&evil);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(10));
+  FOR_EACH_OBSERVER(Foo, observer_list, Observe(10));
+
+  EXPECT_EQ(20, a.total);
+  EXPECT_EQ(-10, b.total);
+  EXPECT_EQ(20, c.total);
+  EXPECT_EQ(-20, d.total);
+}
+
 TEST(ObserverListThreadSafeTest, BasicTest) {
   MessageLoop loop;
 
@@ -541,5 +599,212 @@ TEST(ObserverListTest, IteratorOutlivesList) {
   // of scope.
 }
 
-}  // namespace
+TEST(ObserverListTest, BasicStdIterator) {
+  using FooList = ObserverList<Foo>;
+  FooList observer_list;
+
+  // An optimization: begin() and end() do not involve weak pointers on
+  // empty list.
+  EXPECT_FALSE(observer_list.begin().list_);
+  EXPECT_FALSE(observer_list.end().list_);
+
+  // Iterate over empty list: no effect, no crash.
+  for (auto& i : observer_list)
+    i.Observe(10);
+
+  Adder a(1), b(-1), c(1), d(-1);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (FooList::iterator i = observer_list.begin(), e = observer_list.end();
+       i != e; ++i)
+    i->Observe(1);
+
+  EXPECT_EQ(1, a.total);
+  EXPECT_EQ(-1, b.total);
+  EXPECT_EQ(1, c.total);
+  EXPECT_EQ(-1, d.total);
+
+  // Check an iteration over a 'const view' for a given container.
+  const FooList& const_list = observer_list;
+  for (FooList::const_iterator i = const_list.begin(), e = const_list.end();
+       i != e; ++i) {
+    EXPECT_EQ(1, std::abs(i->GetValue()));
+  }
+
+  for (const auto& o : const_list)
+    EXPECT_EQ(1, std::abs(o.GetValue()));
+}
+
+TEST(ObserverListTest, StdIteratorRemoveItself) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  DisrupterSelf disrupter(&observer_list);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveBefore) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  Disrupter disrupter(&observer_list, &b);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-1, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveAfter) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  Disrupter disrupter(&observer_list, &c);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(0, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveAfterFront) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  Disrupter disrupter(&observer_list, &a);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(1, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveBeforeBack) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  Disrupter disrupter(&observer_list, &d);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&d);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(0, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveFront) {
+  using FooList = ObserverList<Foo>;
+  FooList observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  DisrupterSelf disrupter(&observer_list);
+
+  observer_list.AddObserver(&disrupter);
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+
+  for (FooList::iterator i = observer_list.begin(), e = observer_list.end();
+       i != e; ++i) {
+    const bool has_disruptor = observer_list.HasObserver(&disrupter);
+    i->Observe(1);
+    // Check that second call to i->Observe() would crash here.
+    if (has_disruptor)
+      EXPECT_FALSE(i.GetCurrent());
+  }
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
+TEST(ObserverListTest, StdIteratorRemoveBack) {
+  ObserverList<Foo> observer_list;
+  Adder a(1), b(-1), c(1), d(-1);
+  DisrupterSelf disrupter(&observer_list);
+
+  observer_list.AddObserver(&a);
+  observer_list.AddObserver(&b);
+  observer_list.AddObserver(&c);
+  observer_list.AddObserver(&d);
+  observer_list.AddObserver(&disrupter);
+
+  for (auto& o : observer_list)
+    o.Observe(1);
+
+  for (auto& o : observer_list)
+    o.Observe(10);
+
+  EXPECT_EQ(11, a.total);
+  EXPECT_EQ(-11, b.total);
+  EXPECT_EQ(11, c.total);
+  EXPECT_EQ(-11, d.total);
+}
+
 }  // namespace base