base::Bind: Implement CancelableCallback to replace CancelableTaske.

base/cancelable_callback_unittest.cc

+// 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.
+
+#include "base/cancelable_callback.h"

[csilv, 2011/11/23 19:37:30]

add includes for base/bind.h, base/bind_helpers.h

[James Hawkins, 2011/11/23 22:12:52]

Done.

+#include "base/message_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace {
+
+class Counter {

[awong, 2011/11/23 19:58:23]

Why use a class instead of a bare function like:

void Increment(int* count) { (*count)++; }

?

[James Hawkins, 2011/11/23 22:12:52]

This is an evolved version of a class I copied from a similar test. I'll
simplify it to your suggestion.

+ public:
+  Counter() : count_(0) { }
+  void IncrementCount() { count_++; }
+  void IncrementCountBy(int n) { count_ += n; }
+  int count() const { return count_; }
+
+ private:
+  int count_;
+};
+
+TEST(CancelableCallbackTest, Cancel) {

[awong, 2011/11/23 19:58:23]

For things related to bind/callback, I've been trying to use a convention where
we add a comment above each test which lists in bullets the specific assertions
the test tries to cover.  Can we do something similar for these tests?

For instance, the properties we're asserting here are:
  - Callback can be run multiple times.
  - After Cancel(), Run() completes, but has no effect.

[James Hawkins, 2011/11/23 22:12:52]

Done.

+  Counter counter;
+  CancelableCallback cancelable(base::Bind(&Counter::IncrementCount,
+                                           base::Unretained(&counter)));
+
+  base::Closure callback = cancelable.callback();
+  callback.Run();
+  EXPECT_EQ(1, counter.count());
+
+  callback.Run();
+  EXPECT_EQ(2, counter.count());
+
+  cancelable.Cancel();
+  callback.Run();
+  EXPECT_EQ(2, counter.count());
+}
+
+TEST(CancelableCallbackTest, MultipleCancel) {
+  Counter counter;
+  CancelableCallback cancelable(base::Bind(&Counter::IncrementCount,
+                                           base::Unretained(&counter)));
+
+  base::Closure callback1 = cancelable.callback();
+  cancelable.Cancel();
+
+  base::Closure callback2 = cancelable.callback();
+  base::Closure callback3 = cancelable.callback();
+  cancelable.Cancel();

[groby-ooo-7-16, 2011/11/23 19:18:57]

I suspect it would be interesting for people to know what happens if the second
cancel _isn't_ there.

[James Hawkins, 2011/11/23 19:37:10]

How so?

[groby-ooo-7-16, 2011/11/23 19:43:14]

Will callback2/3 be running without it? (It won't, if I read the code correctly
- but it'd be nice to point that out)

The second Cancel() in this test seems to imply that it still will be run. 

On 2011/11/23 19:37:10, James Hawkins wrote:

[James Hawkins, 2011/11/23 22:12:52]

I've rearranged this, per-offline.

+
+  callback1.Run();
+  EXPECT_EQ(0, counter.count());
+
+  callback2.Run();
+  EXPECT_EQ(0, counter.count());
+
+  cancelable.Cancel();
+  callback3.Run();
+  EXPECT_EQ(0, counter.count());
+}
+
+TEST(CancelableCallbackTest, NArity) {

[awong, 2011/11/23 19:58:23]

What's NArity mean in this context?

If you're testing that you can support callbacks that have bound arguments, I
would actually remove this test since it's double-covering the point of
bind_unittests.

[James Hawkins, 2011/11/23 22:12:52]

Done.

+  Counter counter;
+  CancelableCallback cancelable(
+      base::Bind(&Counter::IncrementCountBy, base::Unretained(&counter), 5));
+
+  base::Closure callback = cancelable.callback();
+  callback.Run();
+  EXPECT_EQ(5, counter.count());
+
+  callback.Run();
+  EXPECT_EQ(10, counter.count());
+
+  cancelable.Cancel();
+  callback.Run();
+  EXPECT_EQ(10, counter.count());
+}
+
+TEST(CancelableCallbackTest, CallbackCanceledOnDestruction) {
+  Counter counter;
+  base::Closure callback;
+
+  {
+    CancelableCallback cancelable(base::Bind(&Counter::IncrementCount,
+                                             base::Unretained(&counter)));
+
+    callback = cancelable.callback();
+    callback.Run();
+    EXPECT_EQ(1, counter.count());
+  }
+
+  callback.Run();
+  EXPECT_EQ(1, counter.count());
+}
+
+TEST(CancelableCallbackTest, Reset) {
+  Counter counter;
+  CancelableCallback cancelable(base::Bind(&Counter::IncrementCount,
+                                           base::Unretained(&counter)));
+
+  base::Closure callback = cancelable.callback();
+  callback.Run();
+  EXPECT_EQ(1, counter.count());
+
+  callback.Run();
+  EXPECT_EQ(2, counter.count());
+
+  cancelable.Reset(base::Bind(&Counter::IncrementCountBy,
+                              base::Unretained(&counter), 3));

[awong, 2011/11/23 19:58:23]

Also EXPECT_TRUE(cancelable.is_null())?

[James Hawkins, 2011/11/23 22:12:52]

|cancelable| is not null at this point. I've added more EXPECTs/ASSERTs in this
test to elucidate the details.

+
+  // The stale copy of the cancelable callback is no longer active.
+  callback.Run();
+  EXPECT_EQ(2, counter.count());
+
+  base::Closure callback2 = cancelable.callback();
+  callback2.Run();
+  EXPECT_EQ(5, counter.count());
+}
+
+TEST(CancelableCallbackTest, IsNull) {
+  CancelableCallback cancelable;
+  EXPECT_TRUE(cancelable.is_null());
+
+  Counter counter;
+  cancelable.Reset(base::Bind(&Counter::IncrementCount,
+                              base::Unretained(&counter)));
+  EXPECT_FALSE(cancelable.is_null());
+
+  cancelable.Cancel();
+  EXPECT_TRUE(cancelable.is_null());
+}
+
+TEST(CancelableCallbackTest, PostTask) {
+  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+
+  Counter counter;
+  CancelableCallback cancelable(base::Bind(&Counter::IncrementCount,
+                                           base::Unretained(&counter)));
+
+  MessageLoop::current()->PostTask(FROM_HERE, cancelable.callback());
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  MessageLoop::current()->Run();
+
+  EXPECT_EQ(1, counter.count());
+
+  MessageLoop::current()->PostTask(FROM_HERE, cancelable.callback());
+  MessageLoop::current()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+
+  // Cancel before running the message loop.
+  cancelable.Cancel();
+  MessageLoop::current()->Run();
+
+  // Callback never ran due to cancellation; count is the same.
+  EXPECT_EQ(1, counter.count());
+}
+
+}  // namespace
+}  // namespace base