Fix bug in ObserverListThreadsafe::RemoveObserver

base/observer_list_unittest.cc

 // Copyright (c) 2011 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 "base/observer_list.h"
 #include "base/observer_list_threadsafe.h"
 
 #include <vector>
 
 #include "base/memory/ref_counted.h"
@@ skipping 157 matching lines @@
 
   int count_observes_;  // Number of times we observed.
   int count_addtask_;   // Number of times thread AddTask was called
   bool do_notifies_;    // Whether these threads should do notifications.
 
   ScopedRunnableMethodFactory<AddRemoveThread>* factory_;
 };
 
 TEST(ObserverListTest, BasicTest) {
   ObserverList<Foo> observer_list;
-  Adder a(1), b(-1), c(1), d(-1);
+  Adder a(1), b(-1), c(1), d(-1), e(-1);
   Disrupter evil(&observer_list, &c);
 
   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);
 
+  // Removing an observer not in the list should do nothing.
+  observer_list.RemoveObserver(&e);
+
   FOR_EACH_OBSERVER(Foo, observer_list, Observe(10));
 
   EXPECT_EQ(a.total, 20);
   EXPECT_EQ(b.total, -20);
   EXPECT_EQ(c.total, 0);
   EXPECT_EQ(d.total, -10);
+  EXPECT_EQ(e.total, 0);
 }
 
 TEST(ObserverListThreadSafeTest, BasicTest) {
   MessageLoop loop;
 
   scoped_refptr<ObserverListThreadSafe<Foo> > observer_list(
       new ObserverListThreadSafe<Foo>);
   Adder a(1);
   Adder b(-1);
   Adder c(1);
@@ skipping 12 matching lines @@
 
   observer_list->Notify(&Foo::Observe, 10);
   loop.RunAllPending();
 
   EXPECT_EQ(a.total, 20);
   EXPECT_EQ(b.total, -20);
   EXPECT_EQ(c.total, 0);
   EXPECT_EQ(d.total, -10);
 }
 
+TEST(ObserverListThreadSafeTest, RemoveObserver) {
+  MessageLoop loop;
+
+  scoped_refptr<ObserverListThreadSafe<Foo> > observer_list(
+      new ObserverListThreadSafe<Foo>);
+  Adder a(1), b(1);
+
+  // Should do nothing.
+  observer_list->RemoveObserver(&a);
+  observer_list->RemoveObserver(&b);
+
+  observer_list->Notify(&Foo::Observe, 10);
+  loop.RunAllPending();
+
+  EXPECT_EQ(a.total, 0);
+  EXPECT_EQ(b.total, 0);
+
+  observer_list->AddObserver(&a);
+
+  // Should also do nothing.
+  observer_list->RemoveObserver(&b);
+
+  observer_list->Notify(&Foo::Observe, 10);
+  loop.RunAllPending();
+
+  EXPECT_EQ(a.total, 10);
+  EXPECT_EQ(b.total, 0);
+}
+
 class FooRemover : public Foo {
  public:
   explicit FooRemover(ObserverListThreadSafe<Foo>* list) : list_(list) {}
   virtual ~FooRemover() {}
 
   void AddFooToRemove(Foo* foo) {
     foos_.push_back(foo);
   }
 
   virtual void Observe(int x) {
@@ skipping 145 matching lines @@
 
   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