Pass Callback to TaskRunner by value and consume it on invocation (1)

content/browser/startup_task_runner_unittest.cc

Index: content/browser/startup_task_runner_unittest.cc
diff --git a/content/browser/startup_task_runner_unittest.cc b/content/browser/startup_task_runner_unittest.cc
index 2ecf9ca63c58c4c32e3f780f607d552e53768ab8..22d0b7d6c9c4ead69716ea27b033c59106a3ebec 100644
--- a/content/browser/startup_task_runner_unittest.cc
+++ b/content/browser/startup_task_runner_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/startup_task_runner.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
@@ -30,8 +32,8 @@ base::Closure task;
 
 // I couldn't get gMock's SaveArg to compile, hence had to save the argument
 // this way
-bool SaveTaskArg(const Closure& arg) {
-  task = arg;
+bool SaveTaskArg(base::Closure* arg) {

[gab, 2017/03/28 15:29:51]

Ouch, that was already an ugly hack and now it's even worse...

How about capturing |task| in PostNonNestableDelayedTask below beforing invoking
the mock (and not have the mock even take a Closure parameter if it doesn't care
about it)?

Also, I don't see a content/ reviewer on this CL? Had anyone looked at this?

[tzik, 2017/03/28 17:34:20]

Done.

Yes, //content is covered by kinuko's review.

+  task = std::move(*arg);
   return true;
 }
 
@@ -84,12 +86,26 @@ class StartupTaskRunnerTest : public testing::Test {
 
 class MockTaskRunner {
  public:
-  MOCK_METHOD3(
-      PostDelayedTask,
-      bool(const tracked_objects::Location&, const Closure&, base::TimeDelta));
-  MOCK_METHOD3(
-      PostNonNestableDelayedTask,
-      bool(const tracked_objects::Location&, const Closure&, base::TimeDelta));
+  MOCK_METHOD3(PostDelayedTask,
+               bool(const tracked_objects::Location&,
+                    base::Closure*,
+                    base::TimeDelta));
+  MOCK_METHOD3(PostNonNestableDelayedTask,
+               bool(const tracked_objects::Location&,
+                    base::Closure*,
+                    base::TimeDelta));
+
+  bool PostDelayedTask(const tracked_objects::Location& from_here,
+                       base::Closure task,
+                       base::TimeDelta delay) {
+    return PostDelayedTask(from_here, &task, delay);
+  }
+
+  bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here,
+                                  base::Closure task,
+                                  base::TimeDelta delay) {
+    return PostNonNestableDelayedTask(from_here, &task, delay);
+  }
 };
 
 class TaskRunnerProxy : public base::SingleThreadTaskRunner {
@@ -97,14 +113,15 @@ class TaskRunnerProxy : public base::SingleThreadTaskRunner {
   TaskRunnerProxy(MockTaskRunner* mock) : mock_(mock) {}
   bool RunsTasksOnCurrentThread() const override { return true; }
   bool PostDelayedTask(const tracked_objects::Location& location,
-                       const Closure& closure,
+                       base::Closure closure,
                        base::TimeDelta delta) override {
-    return mock_->PostDelayedTask(location, closure, delta);
+    return mock_->PostDelayedTask(location, std::move(closure), delta);
   }
   bool PostNonNestableDelayedTask(const tracked_objects::Location& location,
-                                  const Closure& closure,
+                                  base::Closure closure,
                                   base::TimeDelta delta) override {
-    return mock_->PostNonNestableDelayedTask(location, closure, delta);
+    return mock_->PostNonNestableDelayedTask(location, std::move(closure),
+                                             delta);
   }
 
  private:
@@ -255,7 +272,7 @@ TEST_F(StartupTaskRunnerTest, AsynchronousExecution) {
   // at most 3 times (once for each task plus possibly once for the observer),
   // the "4" is a backstop.
   for (int i = 0; i < 4 && observer_calls == 0; i++) {
-    task.Run();
+    std::move(task).Run();
     EXPECT_EQ(i + 1, GetLastTask());
   }
   EXPECT_EQ(task_count, 2);
@@ -302,7 +319,7 @@ TEST_F(StartupTaskRunnerTest, AsynchronousExecutionFailedTask) {
   // at most twice (once for the failed task plus possibly once for the
   // observer), the "4" is a backstop.
   for (int i = 0; i < 4 && observer_calls == 0; i++) {
-    task.Run();
+    std::move(task).Run();
   }
   EXPECT_EQ(GetLastTask(), 3);
   EXPECT_EQ(task_count, 1);