Add a CallbackRegistry class to base/ to manage callbacks

base/callback_list_internal.cc

Index: base/callback_list_internal.cc
diff --git a/base/callback_list_internal.cc b/base/callback_list_internal.cc
new file mode 100644
index 0000000000000000000000000000000000000000..cbb341310bda207a13c05490d54ba47b02d8a39d
--- /dev/null
+++ b/base/callback_list_internal.cc
@@ -0,0 +1,97 @@
+// Copyright 2013 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/callback_list_internal.h"
+
+#include <algorithm>
+
+#include "base/bind.h"

[erikwright (departed), 2013/09/04 19:35:55]

The only purpose of bind in this file is for building the 'removal closure'.

If you follow my suggestion elsewhere about using a Handle object instead of a
Closure you can get rid of the Bind and then I think concerns about template
bloat should be pretty minor.

In that case, I would propose folding CallbackListImpl into CallbackListBase and
making it use base::Callback<void(Detail)> rather than CallbackBase. That would
eliminate the encapsulation violation, eliminate a cast or two, etc.

+#include "base/bind_helpers.h"
+
+namespace base {
+namespace internal {
+
+CallbackListImpl::Iterator::Iterator(
+    const base::WeakPtr<CallbackListImpl>& list)
+    : list_(list),
+      index_(0),
+      max_index_(list->type_ == CALLBACKS_NOTIFY_ALL ?
+                 std::numeric_limits<size_t>::max() :
+                 list->callbacks_.size()) {
+  ++list_->active_iterator_count_;
+}
+
+CallbackListImpl::Iterator::~Iterator() {
+  if (list_ && --list_->active_iterator_count_ == 0) {
+    list_->Compact();
+  }
+}
+
+CallbackBase* CallbackListImpl::Iterator::GetNext() {
+  if (!list_)
+    return NULL;
+  size_t max_index = std::min(max_index_, list_->callbacks_.size());
+  while (index_ < max_index && !list_->callbacks_[index_])
+    ++index_;
+  return index_ < max_index ? list_->callbacks_[index_++] : NULL;
+}
+
+CallbackListImpl::CallbackListImpl(CallbackNotificationType type)
+    : active_iterator_count_(0),
+      type_(type),
+      weak_factory_(this) {}
+
+CallbackListImpl::~CallbackListImpl() {
+  Clear();
+}
+
+void CallbackListImpl::Clear() {
+  if (active_iterator_count_) {
+    for (size_t i = 0; i < callbacks_.size(); ++i) {
+      CallbackBase* tmp = callbacks_[i];
+      tmp->Reset();

[erikwright (departed), 2013/09/04 19:35:55]

Why is the Reset necessary?

[Cait (Slow), 2013/09/04 22:09:25]

Done.

+      delete tmp;
+      callbacks_[i] = NULL;
+    }
+  } else {
+    for (size_t i = 0; i < callbacks_.size(); ++i)
+      delete callbacks_[i];
+    callbacks_.clear();
+  }
+}
+
+void CallbackListImpl::RemoveCallback(base::internal::CallbackBase* cb) {
+  for (size_t i = 0; i < callbacks_.size(); i++) {
+    if (callbacks_[i] == cb) {
+      CallbackBase* tmp = callbacks_[i];
+      tmp->Reset();

[erikwright (departed), 2013/09/04 19:35:55]

Again, why is the Reset necessary?

[Cait (Slow), 2013/09/04 22:09:25]

Done.

+      delete tmp;
+      if (active_iterator_count_) {
+        callbacks_[i] = NULL;
+      } else {
+        callbacks_.erase(callbacks_.begin() + i);
+      }
+      return;
+    }
+  }
+}
+
+base::Closure CallbackListImpl::AddCallback(base::internal::CallbackBase* cb) {
+  callbacks_.push_back(cb);
+  return base::Bind(&CallbackListImpl::RemoveCallback, Unretained(this), cb);
+}
+
+base::WeakPtr<CallbackListImpl> CallbackListImpl::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
+void CallbackListImpl::Compact() {
+  callbacks_.erase(
+      std::remove(callbacks_.begin(), callbacks_.end(),
+                  static_cast<base::internal::CallbackBase*>(NULL)),
+                  callbacks_.end());
+}
+
+}  // namespace internal
+}  // namespace base