Add const to BindStateBase to align RefCounted constness

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_
 
@@ skipping 18 matching lines @@
 // 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 {
  public:
   using InvokeFuncStorage = void(*)();
 
  protected:
   BindStateBase(InvokeFuncStorage polymorphic_invoke,
-                void (*destructor)(BindStateBase*));
+                void (*destructor)(const BindStateBase*));
   BindStateBase(InvokeFuncStorage polymorphic_invoke,
-                void (*destructor)(BindStateBase*),
+                void (*destructor)(const BindStateBase*),
                 bool (*is_cancelled)(const BindStateBase*));
   ~BindStateBase() = default;
 
  private:
   friend class scoped_refptr<BindStateBase>;
   template <CopyMode copy_mode>
   friend class CallbackBase;
 
   bool IsCancelled() const {
     return is_cancelled_(this);
   }
 
-  void AddRef();
-  void Release();
+  void AddRef() const;
+  void Release() const;

[Yuta Kitamura, 2016/09/20 10:04:21]

I'm not sure adding const to these is legit. In my plain understanding
they modify the refcount, so they shouldn't be const.

[tzik, 2016/09/20 10:54:30]

The refcount is not a state of the instance, but a shared state of the strong
references to the instance. Even on a const object, IMO, we should be able to
make a strong reference to it, without modifying the main part of the object.

Also, this is needed to make BindStateBase be a subclass of
RefCountedThreadSafe. That will remove the manual refcount handling in
BindStateBase, and simplify the class.

[Yuta Kitamura, 2016/09/21 06:01:36]

Ah okay, this lets us refcount const objects, right?

 
   // 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_;
 
-  AtomicRefCount ref_count_;
+  mutable AtomicRefCount ref_count_;
 
   // Pointer to a function that will properly destroy |this|.
-  void (*destructor_)(BindStateBase*);
+  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
 // CallbackBase<Copyable> uses CallbackBase<MoveOnly> for its implementation.
 template <>
@@ skipping 53 matching lines @@
   ~CallbackBase() {}
 };
 
 extern template class CallbackBase<CopyMode::MoveOnly>;
 extern template class CallbackBase<CopyMode::Copyable>;
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_CALLBACK_INTERNAL_H_