Add a CallbackRegistry class to base/ to manage callbacks

base/callback_registry.h

+// 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.
+
+#ifndef BASE_CALLBACK_REGISTRY_H_
+#define BASE_CALLBACK_REGISTRY_H_
+
+#include <list>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+
+// OVERVIEW:
+//
+// A container for a list of callbacks.  Unlike a normal STL vector or list,
+// this container can be modified during iteration without invalidating the
+// iterator. It safely handles the case of a callback removing itself
+// or another callback from the list while callbacks are being run.
+//
+// TYPICAL USAGE:
+//
+// class MyWidget {
+//  public:
+//   ...
+//
+//   typedef base::Callback<void(const Foo&)> OnFooCallback;
+//
+//   scoped_ptr<CallbackRegistry<Foo>::Subscription> RegisterCallback(
+//       const OnFooCallback& cb) {
+//     return callback_registry_.Add(cb);
+//   }
+//
+//  private:
+//   void NotifyFoo(const Foo& foo) {
+//      callback_registry_.Notify(foo);
+//   }
+//
+//   CallbackRegistry<Foo> callback_registry_;
+// };
+//
+//
+// class MyWidgetListener {
+//  public:
+//   MyWidgetListener::MyWidgetListener() {
+//     foo_subscription_ = MyWidget::GetCurrent()->RegisterCallback(
+//             base::Bind(&MyWidgetListener::OnFoo, this)));
+//   }
+//
+//   MyWidgetListener::~MyWidgetListener() {
+//      // Subscription gets deleted automatically and will deregister
+//      // the callback in the process.
+//   }
+//
+//   void OnFoo(const Foo& foo) {

[erikwright (departed), 2013/09/09 19:56:18]

nit: put this in private.

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+//     // Do something.
+//   }
+//
+//  private:
+//   scoped_ptr<CallbackRegistry<Foo>::Subscription> foo_subscription_;
+// };
+
+namespace base {
+
+namespace internal {

[erikwright (departed), 2013/09/09 19:56:18]

blank line between namespace and the implementation.

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+// Holds the methods shared by all specializations of CallbackRegistry to avoid

[erikwright (departed), 2013/09/09 19:56:18]

I'm of the opinion that this comment is unnecessary, but I'll defer to Albert.

[awong, 2013/09/09 20:04:54]

Yeah, you're right. Let's remove it too.

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+// code duplication.
+template <typename CallbackType>
+class CallbackRegistryBase {
+ public:
+  typedef std::list<CallbackType> ListType;

[awong, 2013/09/09 19:30:20]

Put this typedef in Subscription?

[Cait (Slow), 2013/09/09 19:40:31]

Done.

+
+  class Subscription {
+   public:
+    Subscription(CallbackRegistryBase<CallbackType>* list,
+                     typename ListType::iterator iter)

[awong, 2013/09/09 19:30:20]

nit: indent

[Cait (Slow), 2013/09/09 19:40:31]

Done.

+        : list_(list),
+          iter_(iter) {}
+
+    ~Subscription() {
+      if (list_->active_iterator_count_)
+        (*iter_).Reset();
+      else
+        list_->callbacks_.erase(iter_);
+    }
+
+   private:
+    CallbackRegistryBase<CallbackType>* list_;
+    typename ListType::iterator iter_;
+
+    DISALLOW_COPY_AND_ASSIGN(Subscription);
+  };
+
+  // Add a callback to the list. The callback will remain registered until the
+  // returned Subscription is destroyed, which must occur before the
+  // CallbackRegistry is destroyed.
+  scoped_ptr<Subscription> Add(const CallbackType& cb) WARN_UNUSED_RESULT {
+    DCHECK(!cb.is_null());
+    typename ListType::iterator it = callbacks_.insert(callbacks_.end(), cb);
+    return scoped_ptr<Subscription>(new Subscription(this, it));
+  }
+
+ protected:
+  // An iterator class that can be used to access the list of callbacks.
+  class Iterator {
+   public:
+    explicit Iterator(CallbackRegistryBase<CallbackType>* list)
+        : list_(list),
+          list_iter_(list_->callbacks_.begin()) {
+      ++list_->active_iterator_count_;
+    }
+
+    Iterator(const Iterator& iter)
+        : list_(iter.list_),
+          list_iter_(iter.list_iter_) {
+      ++list_->active_iterator_count_;
+    }
+
+    ~Iterator() {
+      if (list_ && --list_->active_iterator_count_ == 0) {
+        list_->Compact();
+      }
+    }
+
+    CallbackType* GetNext() {
+      while ((list_iter_ != list_->callbacks_.end()) && list_iter_->is_null())
+        list_iter_++;

[erikwright (departed), 2013/09/09 19:56:18]

++list_iter_; ?

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+
+      CallbackType* cb =
+          list_iter_ != list_->callbacks_.end() ? &(*list_iter_) : NULL;
+      list_iter_++;

[erikwright (departed), 2013/09/09 19:56:18]

ditto?

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+      return cb;
+    }
+
+   private:
+    CallbackRegistryBase<CallbackType>* list_;
+    typename ListType::iterator list_iter_;
+  };
+
+  CallbackRegistryBase()
+      : active_iterator_count_(0) {}
+
+  ~CallbackRegistryBase() {
+    DCHECK(active_iterator_count_ == 0);
+    DCHECK(callbacks_.size() == 0);
+  }
+
+  // Returns an instance of a CallbackRegistryBase::Iterator which can be used
+  // to run callbacks.
+  Iterator GetIterator() {
+    return Iterator(this);
+  }
+
+  // Compact the list: remove any entries which were NULLed out during
+  // iteration.
+  void Compact() {
+    typename ListType::iterator it = callbacks_.begin();
+    while (it != callbacks_.end()) {
+      if ((*it).is_null())
+        it = callbacks_.erase(it);
+      else
+        ++it;
+    }
+  }
+
+ private:
+

[erikwright (departed), 2013/09/09 19:56:18]

remove blank

[Cait (Slow), 2013/09/09 20:33:03]

Done.

+  ListType callbacks_;
+  int active_iterator_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallbackRegistryBase);
+};
+
+}  // namespace internal
+
+template <typename Details>
+class CallbackRegistry
+    : public base::internal::CallbackRegistryBase<

[awong, 2013/09/09 19:30:20]

The base:: still seems to be here.

[Cait (Slow), 2013/09/09 19:40:31]

Done.

+          Callback<void(const Details&)> > {
+ public:
+  typedef typename base::internal::CallbackRegistryBase<
+      base::Callback<void(const Details&)> >::Iterator Iterator;

[awong, 2013/09/09 19:30:20]

Move the iterator typedef into Notify or just directly use the full type for the
single declaration?

[Cait (Slow), 2013/09/09 19:40:31]

Done.

+  typedef typename base::internal::CallbackRegistryBase<
+      base::Callback<void(const Details&)> >::Subscription Subscription;
+  CallbackRegistry() {}
+
+  // Execute all active callbacks with |details| parameter.
+  void Notify(const Details& details) {
+    Iterator it = this->GetIterator();
+    base::Callback<void(const Details&)>* cb;
+    while((cb = it.GetNext()) != NULL) {
+      cb->Run(details);
+    }
+  }
+
+private:
+  DISALLOW_COPY_AND_ASSIGN(CallbackRegistry);
+};
+
+template <> class CallbackRegistry<void>
+    : public base::internal::CallbackRegistryBase<Closure> {
+ public:
+  CallbackRegistry() {}
+
+  // Execute all active callbacks.
+  void Notify() {
+    Iterator it = this->GetIterator();
+    Closure* cb;
+    while((cb = it.GetNext()) != NULL) {
+      cb->Run();
+    }
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CallbackRegistry);
+};
+
+}  // namespace base
+
+#endif  // BASE_CALLBACK_REGISTRY_H_