Remove Runnable concept from base/bind_internal.h

`Runnable` concept was useful when bind_internal.h was a huge generated
file. However, it's an unneeded layer these days.
This CL removes `Runnable` from bind_internal.h to simplify its impl.
After this CL, BindState holds the bound Functor directly in its internal
storage.

This CL contains:
 * Merge bind_internal_win.h into bind_internal.h
 * Move typechecks in Bind() into MakeBindStateType.
 * Remove no longer used HasIsMethodTag in bind_helper.h.
 * Remove InvokeHelper specialization for IgnoreResult.
 * Merge assertion-only InvokeHelper specialization into another
   specialization.
 * Factor out the type setup in BindState into MakeBindStateType.

BUG=554299

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

base/BUILD.gn

@@ -231,7 +231,6 @@ component("base") {
     "bind_helpers.cc",
     "bind_helpers.h",
     "bind_internal.h",
-    "bind_internal_win.h",
     "bit_cast.h",
     "bits.h",
     "build_time.cc",

base/base.gypi

@@ -121,7 +121,6 @@
           'bind_helpers.cc',
           'bind_helpers.h',
           'bind_internal.h',
-          'bind_internal_win.h',
           'bit_cast.h',
           'bits.h',
           'build_time.cc',

base/bind.h

@@ -21,55 +21,19 @@
 // If you're reading the implementation, before proceeding further, you should
 // read the top comment of base/bind_internal.h for a definition of common
 // terms and concepts.
-//
-// RETURN TYPES
-//
-// Though Bind()'s result is meant to be stored in a Callback<> type, it
-// cannot actually return the exact type without requiring a large amount
-// of extra template specializations. The problem is that in order to
-// discern the correct specialization of Callback<>, Bind would need to
-// unwrap the function signature to determine the signature's arity, and
-// whether or not it is a method.
-//
-// Each unique combination of (arity, function_type, num_prebound) where
-// function_type is one of {function, method, const_method} would require
-// one specialization.  We eventually have to do a similar number of
-// specializations anyways in the implementation (see the Invoker<>,
-// classes).  However, it is avoidable in Bind if we return the result
-// via an indirection like we do below.
-//
-// TODO(ajwong): We might be able to avoid this now, but need to test.
-//
-// It is possible to move most of the static_assert into BindState<>, but it
-// feels a little nicer to have the asserts here so people do not need to crack
-// open bind_internal.h.  On the other hand, it makes Bind() harder to read.
 
 namespace base {
 
 template <typename Functor, typename... 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;
-
-  const bool is_method = internal::HasIsMethodTag<RunnableType>::value;
-
-  // For methods, we need to be careful for parameter 1.  We do not require
-  // a scoped_refptr because BindState<> itself takes care of AddRef() for
-  // methods. We also disallow binding of an array as the method's target
-  // object.
-  static_assert(!internal::BindsArrayToFirstArg<is_method, Args...>::value,
-                "first bound argument to method cannot be array");
-  static_assert(
-      !internal::HasRefCountedParamAsRawPtr<is_method, Args...>::value,
-      "a parameter is a refcounted type and needs scoped_refptr");
-
-  using BindState = internal::BindState<RunnableType, Args...>;
+inline base::Callback<MakeUnboundRunType<Functor, Args...>> Bind(
+    Functor&& functor,
+    Args&&... args) {
+  using BindState = internal::MakeBindStateType<Functor, Args...>;
   using UnboundRunType = MakeUnboundRunType<Functor, Args...>;
-  using CallbackType = Callback<UnboundRunType>;
   using Invoker = internal::Invoker<BindState, UnboundRunType>;
 
-  return CallbackType(new BindState(internal::MakeRunnable(functor),
+  using CallbackType = Callback<UnboundRunType>;
+  return CallbackType(new BindState(std::forward<Functor>(functor),
                                     std::forward<Args>(args)...),
                       &Invoker::Run);
 }

base/bind_helpers.h

@@ -176,86 +176,6 @@ struct IsWeakReceiver;
 
 namespace internal {
 
-// Use the Substitution Failure Is Not An Error (SFINAE) trick to inspect T
-// for the existence of AddRef() and Release() functions of the correct
-// signature.
-//
-// http://en.wikipedia.org/wiki/Substitution_failure_is_not_an_error
-// http://stackoverflow.com/questions/257288/is-it-possible-to-write-a-c-template-to-check-for-a-functions-existence
-// http://stackoverflow.com/questions/4358584/sfinae-approach-comparison
-// http://stackoverflow.com/questions/1966362/sfinae-to-check-for-inherited-member-functions
-//
-// The last link in particular show the method used below.
-//
-// For SFINAE to work with inherited methods, we need to pull some extra tricks
-// with multiple inheritance.  In the more standard formulation, the overloads
-// of Check would be:
-//
-//   template <typename C>
-//   Yes NotTheCheckWeWant(Helper<&C::TargetFunc>*);
-//
-//   template <typename C>
-//   No NotTheCheckWeWant(...);
-//
-//   static const bool value = sizeof(NotTheCheckWeWant<T>(0)) == sizeof(Yes);
-//
-// The problem here is that template resolution will not match
-// C::TargetFunc if TargetFunc does not exist directly in C.  That is, if
-// TargetFunc in inherited from an ancestor, &C::TargetFunc will not match,
-// |value| will be false.  This formulation only checks for whether or
-// not TargetFunc exist directly in the class being introspected.
-//
-// To get around this, we play a dirty trick with multiple inheritance.
-// First, We create a class BaseMixin that declares each function that we
-// want to probe for.  Then we create a class Base that inherits from both T
-// (the class we wish to probe) and BaseMixin.  Note that the function
-// signature in BaseMixin does not need to match the signature of the function
-// we are probing for; thus it's easiest to just use void().
-//
-// Now, if TargetFunc exists somewhere in T, then &Base::TargetFunc has an
-// ambiguous resolution between BaseMixin and T.  This lets us write the
-// following:
-//
-//   template <typename C>
-//   No GoodCheck(Helper<&C::TargetFunc>*);
-//
-//   template <typename C>
-//   Yes GoodCheck(...);
-//
-//   static const bool value = sizeof(GoodCheck<Base>(0)) == sizeof(Yes);
-//
-// Notice here that the variadic version of GoodCheck() returns Yes here
-// instead of No like the previous one. Also notice that we calculate |value|
-// by specializing GoodCheck() on Base instead of T.
-//
-// We've reversed the roles of the variadic, and Helper overloads.
-// GoodCheck(Helper<&C::TargetFunc>*), when C = Base, fails to be a valid
-// substitution if T::TargetFunc exists. Thus GoodCheck<Base>(0) will resolve
-// to the variadic version if T has TargetFunc.  If T::TargetFunc does not
-// exist, then &C::TargetFunc is not ambiguous, and the overload resolution
-// will prefer GoodCheck(Helper<&C::TargetFunc>*).
-//
-// This method of SFINAE will correctly probe for inherited names, but it cannot
-// typecheck those names.  It's still a good enough sanity check though.
-//
-// Works on gcc-4.2, gcc-4.4, and Visual Studio 2008.
-//
-
-template <typename T>
-class HasIsMethodTag {
-  using Yes = char[1];
-  using No = char[2];
-
-  template <typename U>
-  static Yes& Check(typename U::IsMethod*);
-
-  template <typename U>
-  static No& Check(...);
-
- public:
-  enum { value = sizeof(Check<T>(0)) == sizeof(Yes) };
-};
-
 template <typename T>
 class UnretainedWrapper {
  public:
@@ -287,6 +207,7 @@ class RetainedRefWrapper {
 template <typename T>
 struct IgnoreResultHelper {
   explicit IgnoreResultHelper(T functor) : functor_(std::move(functor)) {}
+  explicit operator bool() const { return !!functor_; }
 
   T functor_;
 };