Maintaing the proper order of initialization WeakPtrFactory

base/cancelable_callback.h

 // Copyright (c) 2011 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.
 //
 // CancelableCallback is a wrapper around base::Callback that allows
 // cancellation of a callback. CancelableCallback takes a reference on the
 // wrapped callback until this object is destroyed or Reset()/Cancel() are
 // called.
 //
 // NOTE:
@@ skipping 96 matching lines @@
   }
 
   // Helper method to bind |forwarder_| using a weak pointer from
   // |weak_factory_|.
   void InitializeForwarder() {
     forwarder_ = base::Bind(&CancelableCallback<void(void)>::Forward,
                             weak_factory_.GetWeakPtr());
   }
 
   // Used to ensure Forward() is not run when this object is destroyed.
   base::WeakPtrFactory<CancelableCallback<void(void)> > weak_factory_;

[Nico, 2014/11/06 22:59:49]

Heh, changing that down there and not up here looks like a bug too. We should
probably convert this class to use a variadic template instead of the manual
copypasta we have today, together with fixing this bug. ckehoe, do you want to
give this a try?

 
   // The wrapper closure.
   base::Callback<void(void)> forwarder_;
 
   // The stored closure that may be cancelled.
   base::Callback<void(void)> callback_;
 
   DISALLOW_COPY_AND_ASSIGN(CancelableCallback);
 };
 
@@ skipping 67 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(CancelableCallback);
 };
 
 template <typename A1, typename A2>
 class CancelableCallback<void(A1, A2)> {
  public:
   CancelableCallback() : weak_factory_(this) {}
 
   // |callback| must not be null.
   explicit CancelableCallback(const base::Callback<void(A1, A2)>& callback)
       : weak_factory_(this),

[Charlie, 2014/11/06 22:57:06]

I'm getting an error in the constructor now that these arguments are in a
different order. Wow, am I the only customer of this code?

         callback_(callback) {
     DCHECK(!callback.is_null());
     InitializeForwarder();
   }
 
   ~CancelableCallback() {}
 
   // Cancels and drops the reference to the wrapped callback.
   void Cancel() {
     weak_factory_.InvalidateWeakPtrs();
@@ skipping 30 matching lines @@
     callback_.Run(a1, a2);
   }
 
   // Helper method to bind |forwarder_| using a weak pointer from
   // |weak_factory_|.
   void InitializeForwarder() {
     forwarder_ = base::Bind(&CancelableCallback<void(A1, A2)>::Forward,
                             weak_factory_.GetWeakPtr());
   }
 
-  // Used to ensure Forward() is not run when this object is destroyed.
-  base::WeakPtrFactory<CancelableCallback<void(A1, A2)> > weak_factory_;
-
   // The wrapper closure.
   base::Callback<void(A1, A2)> forwarder_;
 
   // The stored closure that may be cancelled.
   base::Callback<void(A1, A2)> callback_;
 
+  // Used to ensure Forward() is not run when this object is destroyed.
+  base::WeakPtrFactory<CancelableCallback<void(A1, A2)> > weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(CancelableCallback);
 };
 
 typedef CancelableCallback<void(void)> CancelableClosure;
 
 }  // namespace base
 
 #endif  // BASE_CANCELABLE_CALLBACK_H_