Add a CallbackRegistry class to base/ to manage callbacks

base/callback_list_unittest.cc

Index: base/callback_list_unittest.cc
diff --git a/base/callback_list_unittest.cc b/base/callback_list_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..66a73171103fe6982f29a93c8e6dc1651c506708
--- /dev/null
+++ b/base/callback_list_unittest.cc
@@ -0,0 +1,250 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Avi (use Gerrit), 2013/08/23 16:51:12]

New files get "Copyright 2013".

[Cait (Slow), 2013/08/23 18:21:10]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/callback_list.h"
+
+#include <vector>
+
+#include "base/compiler_specific.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/threading/platform_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace {
+
+typedef CallbackList<base::Closure> TestCallbackList;
+
+class Listener {
+ public:
+  explicit Listener() : total(0) {}
+  virtual ~Listener() {}
+  virtual void OnEvent() {
+    total ++;

[Avi (use Gerrit), 2013/08/23 16:51:12]

No space.

[Cait (Slow), 2013/08/23 18:21:10]

Done.

+  }
+  int total;
+};
+
+class Remover : public Listener {
+ public:
+  explicit Remover() : removal_cb_(base::Closure()) {}
+  virtual ~Remover() {}
+  virtual void OnEvent() OVERRIDE {
+    Listener::OnEvent();
+    removal_cb_.Run();
+  }
+  virtual void SetCallback(const base::Closure& cb) {
+    removal_cb_ = cb;
+  }
+
+ private:
+  base::Closure removal_cb_;
+};
+
+class Adder : public Listener {
+ public:
+  explicit Adder(TestCallbackList* cb_list)
+      : added(false),
+        cb_list_(cb_list) {}
+  virtual ~Adder() {}
+  virtual void OnAddedEvent() {
+    if (!added) {
+      added = true;
+      cb_list_->RegisterCallback(
+          base::Bind(&Adder::OnEvent, base::Unretained(this)));
+    }
+  }
+  bool added;
+
+ private:
+  TestCallbackList* cb_list_;
+};
+
+class Clearer : public Listener{

[Avi (use Gerrit), 2013/08/23 16:51:12]

Space before {

[Cait (Slow), 2013/08/23 18:21:10]

Done.

+ public:
+  explicit Clearer(TestCallbackList* cb_list)
+      : added(false),
+        cb_list_(cb_list) {}
+  virtual ~Clearer() {}
+  virtual void OnClearAndAddEvent() {
+    cb_list_->Clear();
+    cb_list_->RegisterCallback(
+        base::Bind(&Clearer::OnEvent, base::Unretained(this)));
+    added = true;
+  }
+  bool added;
+
+ private:
+  TestCallbackList* cb_list_;
+};
+
+class ListDestructor {
+ public:
+  explicit ListDestructor(TestCallbackList* cb_list)
+      : cb_list_(cb_list) {}
+  virtual ~ListDestructor() {}
+  virtual void OnEvent() {
+    delete cb_list_;
+  }
+
+ private:
+  TestCallbackList* cb_list_;
+};
+
+TEST(CallbackListTest, BasicTest) {
+  TestCallbackList cb_list;
+  Listener a, b, c;
+  base::Closure a_cb = base::Bind(&Listener::OnEvent, base::Unretained(&a));
+  base::Closure b_cb = base::Bind(&Listener::OnEvent, base::Unretained(&b));
+
+  base::Closure remove_a = cb_list.RegisterCallback(a_cb);
+
+  EXPECT_TRUE(cb_list.HasCallback(a_cb));
+  EXPECT_FALSE(cb_list.HasCallback(b_cb));
+
+  base::Closure remove_b = cb_list.RegisterCallback(b_cb);
+  EXPECT_TRUE(cb_list.HasCallback(b_cb));
+
+  EXPECT_FALSE(remove_a.is_null());
+  EXPECT_FALSE(remove_b.is_null());
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  EXPECT_EQ(1, a.total);
+  EXPECT_EQ(1, b.total);
+
+  remove_b.Run();
+
+  base::Closure remove_c = cb_list.RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&c)));
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  EXPECT_EQ(2, a.total);
+  EXPECT_EQ(1, b.total);
+  EXPECT_EQ(1, c.total);
+
+  remove_a.Run();
+  remove_b.Run();
+  remove_c.Run();
+
+  EXPECT_FALSE(cb_list.might_have_callbacks());
+}
+
+TEST(CallbackListTest, RemoveCallbacksDuringIteration) {
+  TestCallbackList cb_list;
+  Listener a, b;
+  Remover remover_1, remover_2;
+
+  base::Closure remover_1_cb = cb_list.RegisterCallback(
+      base::Bind(&Remover::OnEvent, base::Unretained(&remover_1)));
+  base::Closure remover_2_cb = cb_list.RegisterCallback(
+      base::Bind(&Remover::OnEvent, base::Unretained(&remover_2)));
+  base::Closure a_cb = cb_list.RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&a)));
+  base::Closure b_cb = cb_list.RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&b)));
+
+  // |remover_1| will remove itself.
+  remover_1.SetCallback(remover_1_cb);
+  // |remover_2| will remove a.
+  remover_2.SetCallback(a_cb);
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  // |remover_1| runs once (and rms itself), |remover_2| runs once (and removes

[Avi (use Gerrit), 2013/08/23 16:51:12]

s/rms/removes/

[Cait (Slow), 2013/08/23 18:21:10]

Done.

+  // a), |a| never runs, and |b| runs once.
+  EXPECT_EQ(1, remover_1.total);
+  EXPECT_EQ(1, remover_2.total);
+  EXPECT_EQ(0, a.total);
+  EXPECT_EQ(1, b.total);
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  // Only |remover_2| and |b| run this time.
+  EXPECT_EQ(1, remover_1.total);
+  EXPECT_EQ(2, remover_2.total);
+  EXPECT_EQ(0, a.total);
+  EXPECT_EQ(2, b.total);
+
+  EXPECT_TRUE(cb_list.might_have_callbacks());
+
+  remover_2_cb.Run();
+  b_cb.Run();
+
+  EXPECT_FALSE(cb_list.might_have_callbacks());
+}
+
+TEST(CallbackListTest, AddCallbacksDuringIteration) {

[Avi (use Gerrit), 2013/08/23 16:51:12]

So we're only supporting the equivalent of NOTIFY_ALL model of ObserverList.
Probably worth documenting.

[Cait (Slow), 2013/08/23 18:21:10]

Actually, adding support for EXISTING_ONLY was pretty straight forward, and
seemed worthwhile for feature-parity with ObserverList.

+  TestCallbackList cb_list;
+  Adder a(&cb_list);
+  Listener b;
+  cb_list.RegisterCallback(
+      base::Bind(&Adder::OnAddedEvent, base::Unretained(&a)));
+  cb_list.RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&b)));
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  EXPECT_EQ(1, a.total);
+  EXPECT_EQ(1, b.total);
+  EXPECT_TRUE(a.added);
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  EXPECT_EQ(2, a.total);
+  EXPECT_EQ(2, b.total);
+}
+
+TEST(CallbackListTest, ClearCallbacksDuringIteration) {
+  TestCallbackList cb_list;
+  Clearer a(&cb_list);
+  Listener b;
+  cb_list.RegisterCallback(
+      base::Bind(&Clearer::OnClearAndAddEvent, base::Unretained(&a)));
+  cb_list.RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&b)));
+
+  FOR_EACH_CALLBACK(base::Closure, cb_list);
+
+  EXPECT_EQ(1, a.total);
+  EXPECT_EQ(0, b.total);
+  EXPECT_TRUE(a.added);
+}
+
+TEST(CallbackListTest, AddNullCallbacks) {
+  TestCallbackList cb_list;
+
+  base::Closure noop = cb_list.RegisterCallback(base::Closure());
+  base::Closure noop2 = cb_list.RegisterCallback(base::Closure());
+
+  // NULL callbacks should not get added.
+  EXPECT_FALSE(cb_list.might_have_callbacks());
+
+  noop.Run();
+  noop2.Run();
+
+  // |noop| and |noop2| should be no-ops.
+  EXPECT_FALSE(cb_list.might_have_callbacks());
+}
+
+TEST(CallbackListTest, IteratorOutlivesList) {
+  TestCallbackList* cb_list = new TestCallbackList();
+  ListDestructor destructor(cb_list);
+  cb_list->RegisterCallback(
+      base::Bind(&ListDestructor::OnEvent, base::Unretained(&destructor)));
+  Listener a;
+  cb_list->RegisterCallback(
+      base::Bind(&Listener::OnEvent, base::Unretained(&a)));
+
+  FOR_EACH_CALLBACK(base::Closure, *cb_list);
+
+  // |a| never gets called, as |cb_list| got deleted first.
+  EXPECT_EQ(0, a.total);
+}
+
+}  // namespace

[Avi (use Gerrit), 2013/08/23 16:51:12]

What happened to tests of callbacks with parameters? It would be nice to have at
least one.

[Cait (Slow), 2013/08/23 18:21:10]

Done.

+}  // namespace base