Decouple Invoker from BindState

Move most of type-level logic out of BindState, and decouple Invoker
from BindState. So that we can use BindState with another Invoker impl.
- UnboundRunType calculation is moved to a separate helper template as
  MakeUnboundRuntype.
- Indices generation to extract tuple is moved from BindState to
  Invoker.
- WeakPtr handling is moved from BindState to Invoker.

This is a preparation CL as well to implement a OneShot variant of
Callback. That will share the same Callback and BindState impl with
different Invoker::Run impl and different Callback tag as Copyable
Callback and MoveOnly Callback have.

BUG=554299

Review-Url: https://codereview.chromium.org/2034633002
Cr-Commit-Position: refs/heads/master@{#402446}

base/bind.h

@@ -46,30 +46,12 @@
 
 namespace base {
 
-namespace internal {
-
-// Don't use Alias Template directly here to avoid a compile error on MSVC2013.
-template <typename Functor, typename... Args>
-struct MakeUnboundRunTypeImpl {
-  using Type =
-      typename BindState<
-          typename FunctorTraits<Functor>::RunnableType,
-          typename FunctorTraits<Functor>::RunType,
-          Args...>::UnboundRunType;
-};
-
-}  // namespace internal
-
 template <typename Functor, typename... Args>
-using MakeUnboundRunType =
-    typename internal::MakeUnboundRunTypeImpl<Functor, Args...>::Type;
-
-template <typename Functor, typename... Args>
-base::Callback<MakeUnboundRunType<Functor, Args...>>
+inline base::Callback<MakeUnboundRunType<Functor, Args...>>
 Bind(Functor functor, Args&&... args) {
   // Type aliases for how to store and run the functor.
   using RunnableType = typename internal::FunctorTraits<Functor>::RunnableType;
-  using RunType = typename internal::FunctorTraits<Functor>::RunType;
+
   const bool is_method = internal::HasIsMethodTag<RunnableType>::value;
 
   // For methods, we need to be careful for parameter 1.  We do not require
@@ -82,11 +64,14 @@ Bind(Functor functor, Args&&... args) {
       !internal::HasRefCountedParamAsRawPtr<is_method, Args...>::value,
       "a parameter is a refcounted type and needs scoped_refptr");
 
-  using BindState = internal::BindState<RunnableType, RunType, Args...>;
+  using BindState = internal::BindState<RunnableType, Args...>;
+  using UnboundRunType = MakeUnboundRunType<Functor, Args...>;
+  using CallbackType = Callback<UnboundRunType>;
+  using Invoker = internal::Invoker<BindState, UnboundRunType>;
 
-  return Callback<typename BindState::UnboundRunType>(
-      new BindState(internal::MakeRunnable(functor),
-                    std::forward<Args>(args)...));
+  return CallbackType(new BindState(internal::MakeRunnable(functor),
+                                    std::forward<Args>(args)...),
+                      &Invoker::Run);
 }
 
 }  // namespace base

base/bind_helpers.h

@@ -462,7 +462,7 @@ T Unwrap(const PassedWrapper<T>& o) {
 //
 // The first argument should be the type of the object that will be received by
 // the method.
-template <bool IsMethod, typename... Args>
+template <bool is_method, typename... Args>
 struct IsWeakMethod : std::false_type {};
 
 template <typename T, typename... Args>
@@ -549,19 +549,25 @@ struct MakeFunctionTypeImpl<R, TypeList<Args...>> {
 template <typename R, typename ArgList>
 using MakeFunctionType = typename MakeFunctionTypeImpl<R, ArgList>::Type;
 
-// Used for ExtractArgs.
+// Used for ExtractArgs and ExtractReturnType.
 template <typename Signature>
 struct ExtractArgsImpl;
 
 template <typename R, typename... Args>
 struct ExtractArgsImpl<R(Args...)> {
-  using Type = TypeList<Args...>;
+  using ReturnType = R;
+  using ArgsList = TypeList<Args...>;
 };
 
 // A type-level function that extracts function arguments into a TypeList.
 // E.g. ExtractArgs<R(A, B, C)> is evaluated to TypeList<A, B, C>.
 template <typename Signature>
-using ExtractArgs = typename ExtractArgsImpl<Signature>::Type;
+using ExtractArgs = typename ExtractArgsImpl<Signature>::ArgsList;
+
+// A type-level function that extracts the return type of a function.
+// E.g. ExtractReturnType<R(A, B, C)> is evaluated to R.
+template <typename Signature>
+using ExtractReturnType = typename ExtractArgsImpl<Signature>::ReturnType;
 
 }  // namespace internal
 

base/bind_internal.h

@@ -300,15 +300,15 @@ struct InvokeHelper;
 template <typename ReturnType>
 struct InvokeHelper<false, ReturnType> {
   template <typename Runnable, typename... RunArgs>
-  static ReturnType MakeItSo(Runnable&& runnable, RunArgs&&... args) {
+  static inline ReturnType MakeItSo(Runnable&& runnable, RunArgs&&... args) {
     return std::forward<Runnable>(runnable).Run(std::forward<RunArgs>(args)...);
   }
 };
 
 template <>
 struct InvokeHelper<false, void> {
   template <typename Runnable, typename... RunArgs>
-  static void MakeItSo(Runnable&& runnable, RunArgs&&... args) {
+  static inline void MakeItSo(Runnable&& runnable, RunArgs&&... args) {
     std::forward<Runnable>(runnable).Run(std::forward<RunArgs>(args)...);
   }
 };
@@ -343,27 +343,42 @@ struct InvokeHelper<true, ReturnType> {
 // Invoker<>
 //
 // See description at the top of the file.
-template <typename BoundIndices, typename StorageType,
-          bool is_weak_call, typename UnboundForwardRunType>
+template <typename StorageType, typename UnboundRunType>
 struct Invoker;
 
-template <size_t... bound_indices,
-          typename StorageType,
-          bool is_weak_call,
-          typename R,
-          typename... UnboundArgs>
-struct Invoker<IndexSequence<bound_indices...>,
-               StorageType,
-               is_weak_call,
-               R(UnboundArgs...)> {
+template <typename StorageType, typename R, typename... UnboundArgs>
+struct Invoker<StorageType, R(UnboundArgs...)> {
   static R Run(BindStateBase* base, UnboundArgs&&... unbound_args) {
-    const StorageType* storage = static_cast<StorageType*>(base);
     // Local references to make debugger stepping easier. If in a debugger,
     // you really want to warp ahead and step through the
     // InvokeHelper<>::MakeItSo() call below.
+    const StorageType* storage = static_cast<StorageType*>(base);
+    static constexpr size_t num_bound_args =
+        std::tuple_size<decltype(storage->bound_args_)>::value;
+    return RunImpl(storage->runnable_,
+                   storage->bound_args_,
+                   MakeIndexSequence<num_bound_args>(),
+                   std::forward<UnboundArgs>(unbound_args)...);
+  }
+
+  template <typename Runnable, typename BoundArgsTuple, size_t... indices>
+  static inline R RunImpl(Runnable&& runnable,
+                          BoundArgsTuple&& bound,
+                          IndexSequence<indices...>,
+                          UnboundArgs&&... unbound_args) {
+    static constexpr bool is_method =
+        HasIsMethodTag<typename std::decay<Runnable>::type>::value;
+
+    using DecayedArgsTuple = typename std::decay<BoundArgsTuple>::type;
+    static constexpr bool is_weak_call =
+        IsWeakMethod<is_method,
+                     typename std::tuple_element<
+                         indices,
+                         DecayedArgsTuple>::type...>::value;
+
     return InvokeHelper<is_weak_call, R>::MakeItSo(
-        storage->runnable_,
-        Unwrap(std::get<bound_indices>(storage->bound_args_))...,
+        std::forward<Runnable>(runnable),
+        Unwrap(std::get<indices>(std::forward<BoundArgsTuple>(bound)))...,
         std::forward<UnboundArgs>(unbound_args)...);
   }
 };
@@ -387,44 +402,34 @@ template <bool is_method, typename... BoundArgs>
 using MakeArgsStorage = typename MakeArgsStorageImpl<
   is_method, typename std::decay<BoundArgs>::type...>::Type;
 
+// Used to implement MakeUnboundRunType.
+template <typename Functor, typename... BoundArgs>
+struct MakeUnboundRunTypeImpl {
+  using RunType = typename FunctorTraits<Functor>::RunType;
+  using ReturnType = ExtractReturnType<RunType>;
+  using Args = ExtractArgs<RunType>;
+  using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), Args>;
+  using Type = MakeFunctionType<ReturnType, UnboundArgs>;
+};
+
 // BindState<>
 //
 // This stores all the state passed into Bind() and is also where most
 // of the template resolution magic occurs.
 //
 // Runnable is the functor we are binding arguments to.
-// RunType is type of the Run() function that the Invoker<> should use.
-// Normally, this is the same as the RunType of the Runnable, but it can
-// be different if an adapter like IgnoreResult() has been used.
 //
 // BoundArgs contains the storage type for all the bound arguments.
-template <typename Runnable, typename RunType, typename... BoundArgs>
-struct BindState;
-
-template <typename Runnable,
-          typename R,
-          typename... Args,
-          typename... BoundArgs>
-struct BindState<Runnable, R(Args...), BoundArgs...> final
-    : public BindStateBase {
+template <typename Runnable, typename... BoundArgs>
+struct BindState final : public BindStateBase {
  private:
-  using StorageType = BindState<Runnable, R(Args...), BoundArgs...>;
   using RunnableType = typename std::decay<Runnable>::type;
 
-  enum { is_method = HasIsMethodTag<RunnableType>::value };
-  enum { is_weak_call = IsWeakMethod<
-      is_method, typename std::decay<BoundArgs>::type...>::value};
-
-  using BoundIndices = MakeIndexSequence<sizeof...(BoundArgs)>;
-  using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), TypeList<Args...>>;
+  static constexpr bool is_method = HasIsMethodTag<RunnableType>::value;
 
  public:
-  using UnboundRunType = MakeFunctionType<R, UnboundArgs>;
-  using InvokerType =
-      Invoker<BoundIndices, StorageType, is_weak_call, UnboundRunType>;
-
   template <typename... ForwardArgs>
-  BindState(RunnableType runnable, ForwardArgs&&... bound_args)
+  explicit BindState(RunnableType runnable, ForwardArgs&&... bound_args)
       : BindStateBase(&Destroy),
         runnable_(std::move(runnable)),
         bound_args_(std::forward<ForwardArgs>(bound_args)...) {}
@@ -441,6 +446,13 @@ struct BindState<Runnable, R(Args...), BoundArgs...> final
 };
 
 }  // namespace internal
+
+// Returns a RunType of bound functor.
+// E.g. MakeUnboundRunType<R(A, B, C), A, B> is evaluated to R(C).
+template <typename Functor, typename... BoundArgs>
+using MakeUnboundRunType =
+    typename internal::MakeUnboundRunTypeImpl<Functor, BoundArgs...>::Type;
+
 }  // namespace base
 
 #endif  // BASE_BIND_INTERNAL_H_

base/callback.h

@@ -345,31 +345,23 @@
 // please include "base/callback_forward.h" instead.
 
 namespace base {
-namespace internal {
-template <typename Runnable, typename RunType, typename... BoundArgsType>
-struct BindState;
-}  // namespace internal
 
 template <typename R, typename... Args, internal::CopyMode copy_mode>
 class Callback<R(Args...), copy_mode>
     : public internal::CallbackBase<copy_mode> {
+ private:
+  using PolymorphicInvoke = R (*)(internal::BindStateBase*, Args&&...);
+
  public:
   // MSVC 2013 doesn't support Type Alias of function types.
   // Revisit this after we update it to newer version.
   typedef R RunType(Args...);
 
   Callback() : internal::CallbackBase<copy_mode>(nullptr) {}
 
-  template <typename Runnable, typename BindRunType, typename... BoundArgs>
-  explicit Callback(
-      internal::BindState<Runnable, BindRunType, BoundArgs...>* bind_state)
+  Callback(internal::BindStateBase* bind_state,
+           PolymorphicInvoke invoke_func)
       : internal::CallbackBase<copy_mode>(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 =
-        &internal::BindState<Runnable, BindRunType, BoundArgs...>
-            ::InvokerType::Run;
     using InvokeFuncStorage =
         typename internal::CallbackBase<copy_mode>::InvokeFuncStorage;
     this->polymorphic_invoke_ =
@@ -396,9 +388,6 @@ class Callback<R(Args...), copy_mode>
         reinterpret_cast<PolymorphicInvoke>(this->polymorphic_invoke_);
     return f(this->bind_state_.get(), std::forward<Args>(args)...);
   }
-
- private:
-  using PolymorphicInvoke = R (*)(internal::BindStateBase*, Args&&...);
 };
 
 }  // namespace base

base/callback_unittest.cc

@@ -14,63 +14,38 @@
 
 namespace base {
 
-namespace {
-
-struct FakeInvoker {
-  // MSVC 2013 doesn't support Type Alias of function types.
-  // Revisit this after we update it to newer version.
-  typedef void RunType(internal::BindStateBase*);
-  static void Run(internal::BindStateBase*) {
-  }
-};
-
-}  // namespace
-
-namespace internal {
+void NopInvokeFunc(internal::BindStateBase*) {}
 
 // White-box testpoints to inject into a Callback<> object for checking
 // comparators and emptiness APIs.  Use a BindState that is specialized
 // based on a type we declared in the anonymous namespace above to remove any
 // chance of colliding with another instantiation and breaking the
 // one-definition-rule.
-template <>
-struct BindState<void(), void(), FakeInvoker>
-    : public BindStateBase {
- public:
-  BindState() : BindStateBase(&Destroy) {}
-  using InvokerType = FakeInvoker;
+struct FakeBindState1 : internal::BindStateBase {
+  FakeBindState1() : BindStateBase(&Destroy) {}
  private:
-  ~BindState() {}
-  static void Destroy(BindStateBase* self) {
-    delete static_cast<BindState*>(self);
+  ~FakeBindState1() {}
+  static void Destroy(internal::BindStateBase* self) {
+    delete static_cast<FakeBindState1*>(self);
   }
 };
 
-template <>
-struct BindState<void(), void(), FakeInvoker, FakeInvoker>
-    : public BindStateBase {
- public:
-  BindState() : BindStateBase(&Destroy) {}
-  using InvokerType = FakeInvoker;
+struct FakeBindState2 : internal::BindStateBase {
+  FakeBindState2() : BindStateBase(&Destroy) {}
  private:
-  ~BindState() {}
-  static void Destroy(BindStateBase* self) {
-    delete static_cast<BindState*>(self);
+  ~FakeBindState2() {}
+  static void Destroy(internal::BindStateBase* self) {
+    delete static_cast<FakeBindState2*>(self);
   }
 };
-}  // namespace internal
 
 namespace {
 
-using FakeBindState1 = internal::BindState<void(), void(), FakeInvoker>;
-using FakeBindState2 =
-    internal::BindState<void(), void(), FakeInvoker, FakeInvoker>;
-
 class CallbackTest : public ::testing::Test {
  public:
   CallbackTest()
-      : callback_a_(new FakeBindState1()),
-        callback_b_(new FakeBindState2()) {
+      : callback_a_(new FakeBindState1(), &NopInvokeFunc),
+        callback_b_(new FakeBindState2(), &NopInvokeFunc) {
   }
 
   ~CallbackTest() override {}
@@ -113,7 +88,7 @@ TEST_F(CallbackTest, Equals) {
   EXPECT_FALSE(callback_b_.Equals(callback_a_));
 
   // We should compare based on instance, not type.
-  Callback<void()> callback_c(new FakeBindState1());
+  Callback<void()> callback_c(new FakeBindState1(), &NopInvokeFunc);
   Callback<void()> callback_a2 = callback_a_;
   EXPECT_TRUE(callback_a_.Equals(callback_a2));
   EXPECT_FALSE(callback_a_.Equals(callback_c));