base::Bind: Implement CancelableCallback to replace CancelableTaske.

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 the callback.
+//
+// EXAMPLE USAGE:
+//
+// In the following example, the Fetcher object cancels an outstanding callback
+// posted to the MessageLoop on destruction so the fetch operation does not
+// occur after the object has been destroyed.
+//
+// class Fetcher {
+//  public:
+//   ~Fetcher() {
+//     // The system is shutting down; cancel the fetch operation.
+//     if (!cancelable_.is_null())
+//       cancelable_.Cancel();
+//   }
+//
+//   // Asynchronously fetches data.
+//   Fetch(const base::Closure& callback) {
+//     cancelable_.Reset(base::Bind(&Fetcher::FetchNow, base::Unretained(this),
+//                                  callback));
+//     MessageLoop::current()->PostTask(FROM_HERE, cancelable_.callback());
+//   }
+//
+//  private:
+//   // Must be called on the IO thread.
+//   FetchNow(const base::Closure& callback) {
+//     ...
+//   }
+//
+//   // Used to cancel the callback on destruction.
+//   CancelableCallback cancelable_;
+// };

[awong, 2011/11/23 23:41:07]

This use case looks exactly like what you'd use a WeakPtrFactory for.

Can we modify to show creating callbacks that we might cancel independent of one
another?

Also, the end of this comment should include a disucssion of the threading
restrictions and lifetime guarantees of the target callback.

[James Hawkins, 2011/11/24 00:33:45]

Done.

+//
+
+#ifndef BASE_CANCELABLE_CALLBACK_H_
+#define BASE_CANCELABLE_CALLBACK_H_
+#pragma once
+
+#include "base/callback.h"
+#include "base/base_export.h"
+#include "base/memory/weak_ptr.h"
+
+namespace base {
+
+// Wrapper used to cancel a callback.

[awong, 2011/11/23 23:41:07]

nit: The file comment already covers this. Remove this comment?

[James Hawkins, 2011/11/24 00:11:15]

Done.

+class BASE_EXPORT CancelableCallback {
+ public:
+  CancelableCallback();
+
+  // |callback| must not be null.
+  explicit CancelableCallback(const base::Closure& callback);
+
+  ~CancelableCallback();
+
+  // Cancels the stored callback.

[awong, 2011/11/23 23:41:07]

This should also specify that it's going to drop references to the wrapped
callback.

Both the Cancel() and Reset() should have strongly published semantics on when
the resources in the wrapped callback are derefed.

[James Hawkins, 2011/11/24 00:11:15]

Done.
Strongly published as in I need to add more comments?

+  void Cancel();
+
+  // Sets |callback| as the closure that may be cancelled. |callback| may not
+  // be null. Outstanding and any previously wrapped callbacks are reset.

[awong, 2011/11/23 23:41:07]

This this be "are reset" or "are canceled?"

[James Hawkins, 2011/11/24 00:11:15]

Done.

+  void Reset(const base::Closure& callback);
+
+  // Returns a callback that can be disabled by calling Cancel().
+  const base::Closure& callback() const;
+
+  // Returns true if the wrapped callback is null.

[awong, 2011/11/23 23:41:07]

This isn't true is it?  It's if the CancelableCallback itself has been Reset w/o
a callback?

All constructors/mutators assert the wrapped callback is not null.

[James Hawkins, 2011/11/24 00:11:15]

In the current implementation it is the former (which happens to be the case for
the latter as well).
Are you asking me to add these checks?

+  bool is_null() const;
+
+ private:
+  // Closure that runs the stored callback.

[awong, 2011/11/23 23:41:07]

Comment is too close to function name. remove?

[James Hawkins, 2011/11/24 00:11:15]

Done.

+  void RunCallback();
+
+  // Helper method to bind |forwarder_| using a weak pointer from
+  // |weak_factory_|.

[awong, 2011/11/23 23:41:07]

nit: How about InitializeForwarder instead of BindForwarder?

[James Hawkins, 2011/11/24 00:11:15]

Done.

+  void BindForwarder();
+
+  // Used to ensure RunCallback() is not run when this object is destroyed.
+  base::WeakPtrFactory<CancelableCallback> weak_factory_;
+
+  // The wrapper closure.
+  base::Closure forwarder_;
+
+  // The stored closure that may be cancelled.
+  base::Closure callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(CancelableCallback);
+};
+
+}  // namespace base
+
+#endif  // BASE_CANCELABLE_CALLBACK_H_