Make BindStateBase RefCountedThreadSafe

base/callback_internal.h

 // Copyright (c) 2012 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.
 
 // This file contains utility functions and classes that help the
 // implementation, and management of the Callback objects.
 
 #ifndef BASE_CALLBACK_INTERNAL_H_
 #define BASE_CALLBACK_INTERNAL_H_
 
 #include "base/atomic_ref_count.h"
 #include "base/base_export.h"
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 
 namespace base {
+
+struct FakeBindState1;
+struct FakeBindState2;
+
 namespace internal {
 template <CopyMode copy_mode>
 class CallbackBase;
 
+template <typename Functor, typename... Args>
+struct BindState;
+
+class BindStateBase;
+
 // BindStateBase is used to provide an opaque handle that the Callback
 // class can use to represent a function object with bound arguments.  It
 // behaves as an existential type that is used by a corresponding
 // DoInvoke function to perform the function execution.  This allows
 // us to shield the Callback class from the types of the bound argument via
 // "type erasure."
 // At the base level, the only task is to add reference counting data. Don't use
 // RefCountedThreadSafe since it requires the destructor to be a virtual method.
 // Creating a vtable for every BindState template instantiation results in a lot
 // of bloat. Its only task is to call the destructor which can be done with a
 // function pointer.
-class BASE_EXPORT BindStateBase {
+
+struct BASE_EXPORT BindStateBaseRefCountTraits {
+  static void Destruct(const BindStateBase* obj);

[dcheng, 2016/09/21 22:07:50]

Does it make sense to inline the definition in the header? I'm wondering if it
will have any binary size impact.

+};
+
+class BASE_EXPORT BindStateBase
+    : public RefCountedThreadSafe<BindStateBase, BindStateBaseRefCountTraits> {
  public:
   using InvokeFuncStorage = void(*)();
 
  protected:
   BindStateBase(InvokeFuncStorage polymorphic_invoke,
                 void (*destructor)(const BindStateBase*));
   BindStateBase(InvokeFuncStorage polymorphic_invoke,
                 void (*destructor)(const BindStateBase*),
                 bool (*is_cancelled)(const BindStateBase*));
-  ~BindStateBase() = default;
 
  private:
   friend class scoped_refptr<BindStateBase>;
   template <CopyMode copy_mode>
   friend class CallbackBase;
 
+  // Add known callers of the destructor as friend classes. Note that making the
+  // destructor protected hits a chromium style check done by the clang plugin.
+  friend struct BindStateBaseRefCountTraits;
+  template <typename Functor, typename... BoundArgs>
+  friend struct BindState;

[dcheng, 2016/09/21 22:07:50]

It feels a bit funny that we have to friend both the traits object and our
derived class.

+  friend struct base::FakeBindState1;
+  friend struct base::FakeBindState2;

[dcheng, 2016/09/21 22:07:50]

I'm a bit sad about having to friend these test only structs. I wonder... can
the tests just store this thing in a scoped_refptr?

+
+  ~BindStateBase() = default;
+
   bool IsCancelled() const {
     return is_cancelled_(this);
   }
 
-  void AddRef() const;
-  void Release() const;
-
   // In C++, it is safe to cast function pointers to function pointers of
   // another type. It is not okay to use void*. We create a InvokeFuncStorage
   // that that can store our function pointer, and then cast it back to
   // the original type on usage.
   InvokeFuncStorage polymorphic_invoke_;
 
-  mutable AtomicRefCount ref_count_;
-
   // Pointer to a function that will properly destroy |this|.
   void (*destructor_)(const BindStateBase*);
   bool (*is_cancelled_)(const BindStateBase*);
 
   DISALLOW_COPY_AND_ASSIGN(BindStateBase);
 };
 
 // Holds the Callback methods that don't require specialization to reduce
 // template bloat.
 // CallbackBase<MoveOnly> is a direct base class of MoveOnly callbacks, and
@@ skipping 55 matching lines @@
   ~CallbackBase() {}
 };
 
 extern template class CallbackBase<CopyMode::MoveOnly>;
 extern template class CallbackBase<CopyMode::Copyable>;
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_CALLBACK_INTERNAL_H_