Remove BindStateHolder and have Bind() return a Callback<> object directly.

This removes some complexity and also fixes a bug where if you call Bind() with the result of Bind(), the resulting Callback would only be valid during the first call.  Ouch.

BUG=none
TEST=new unittests


Review URL: http://codereview.chromium.org/8738001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114494 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 93 insertions, 105 deletions

diff --git a/base/callback.h b/base/callback.h
index 4bf474d..abaa438 100644
--- a/base/callback.h
+++ b/base/callback.h
@@ -129,29 +129,28 @@
 // The Callback classes represent a generic function pointer. Internally,
 // it stores a refcounted piece of state that represents the target function
 // and all its bound parameters.  Each Callback specialization has a templated
-// constructor that takes an BindStateHolder<> object.  In the context of
-// the constructor, the static type of this BindStateHolder<> object
-// uniquely identifies the function it is representing, all its bound
-// parameters, and a DoInvoke() that is capable of invoking the target.
+// constructor that takes an BindState<>*.  In the context of the constructor,
+// the static type of this BindState<> pointer uniquely identifies the
+// function it is representing, all its bound parameters, and a Run() method
+// that is capable of invoking the target.
 //
-// Callback's constructor is takes the BindStateHolder<> that has the
-// full static type and erases the target function type, and the bound
-// parameters.  It does this by storing a pointer to the specific DoInvoke()
-// function, and upcasting the state of BindStateHolder<> to a
-// BindStateBase. This is safe as long as this BindStateBase pointer
-// is only used with the stored DoInvoke() pointer.
+// Callback's constructor takes the BindState<>* that has the full static type
+// and erases the target function type as well as the types of the bound
+// parameters.  It does this by storing a pointer to the specific Run()
+// function, and upcasting the state of BindState<>* to a
+// BindStateBase*. This is safe as long as this BindStateBase pointer
+// is only used with the stored Run() pointer.
 //
-// To create BindStateHolder<> objects, we use the Bind() functions.
-// These functions, along with a set of internal templates, are reponsible for
+// To BindState<> objects are created inside the Bind() functions.
+// These functions, along with a set of internal templates, are responsible for
 //
 //  - Unwrapping the function signature into return type, and parameters
 //  - Determining the number of parameters that are bound
-//  - Creating the storage for the bound parameters
+//  - Creating the BindState storing the bound parameters
 //  - Performing compile-time asserts to avoid error-prone behavior
-//  - Returning an BindStateHolder<> with an DoInvoke() that has an arity
-//    matching the number of unbound parameters, and knows the correct
-//    refcounting semantics for the target object if we are binding a class
-//    method.
+//  - Returning an Callback<> with an arity matching the number of unbound
+//    parameters and that knows the correct refcounting semantics for the
+//    target object if we are binding a method.
 //
 // The Bind functions do the above using type-inference, and template
 // specializations.
@@ -239,27 +238,30 @@ namespace base {
 template <typename Sig>
 class Callback;
 
+namespace internal {
+template <typename Runnable, typename RunType, typename BoundArgsType>
+struct BindState;
+}  // namespace internal
+
 template <typename R>
 class Callback<R(void)> : public internal::CallbackBase {
  public:
   typedef R(RunType)();
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -285,22 +287,20 @@ class Callback<R(A1)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -327,22 +327,20 @@ class Callback<R(A1, A2)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -372,22 +370,20 @@ class Callback<R(A1, A2, A3)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -420,22 +416,20 @@ class Callback<R(A1, A2, A3, A4)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -472,22 +466,20 @@ class Callback<R(A1, A2, A3, A4, A5)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -527,22 +519,20 @@ class Callback<R(A1, A2, A3, A4, A5, A6)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5, A6);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }
 
@@ -585,22 +575,20 @@ class Callback<R(A1, A2, A3, A4, A5, A6, A7)> : public internal::CallbackBase {
  public:
   typedef R(RunType)(A1, A2, A3, A4, A5, A6, A7);
 
-  Callback() : CallbackBase(NULL, NULL) { }
+  Callback() : CallbackBase(NULL) { }
 
-  // We pass BindStateHolder by const ref to avoid incurring an
-  // unnecessary AddRef/Unref pair even though we will modify the object.
-  // We cannot use a normal reference because the compiler will warn
-  // since this is often used on a return value, which is a temporary.
-  //
   // Note that this constructor CANNOT be explicit, and that Bind() CANNOT
   // return the exact Callback<> type.  See base/bind.h for details.
-  template <typename T>
-  Callback(const internal::BindStateHolder<T>& bind_state_holder)
-      : CallbackBase(NULL, &bind_state_holder.bind_state_) {
-    // Force the assignment to a location variable of PolymorphicInvoke
+  template <typename Runnable, typename RunType, typename BoundArgsType>
+  Callback(internal::BindState<Runnable, RunType, BoundArgsType>* bind_state)
+      : CallbackBase(bind_state) {
+
+    // Force the assignment to a local variable of PolymorphicInvoke
     // so the compiler will typecheck that the passed in Run() method has
     // the correct type.
-    PolymorphicInvoke invoke_func = &T::InvokerType::Run;
+    PolymorphicInvoke invoke_func =
+        &internal::BindState<Runnable, RunType, BoundArgsType>
+            ::InvokerType::Run;
     polymorphic_invoke_ = reinterpret_cast<InvokeFuncStorage>(invoke_func);
   }