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

content/browser/startup_task_runner_unittest.cc

 // Copyright 2013 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 "content/browser/startup_task_runner.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "base/run_loop.h"
 #include "base/task_runner.h"
 
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 namespace {
 
 using base::Closure;
 using testing::_;
 using testing::Assign;
 using testing::Invoke;
 using testing::WithArg;
 
 int observer_calls = 0;
 int task_count = 0;
 int observer_result;
 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;
 }
 
 void Observer(int result) {
   observer_calls++;
   observer_result = result;
 }
 
 class StartupTaskRunnerTest : public testing::Test {
  public:
@@ skipping 32 matching lines @@
 // We can't use the real message loop, even if we want to, since doing so on
 // Android requires a complex Java infrastructure. The test would have to built
 // as a content_shell test; but content_shell startup invokes the class we are
 // trying to test.
 //
 // The mocks are not directly in TaskRunnerProxy because reference counted
 // objects seem to confuse the mocking framework
 
 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 {
  public:
   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:
   MockTaskRunner* mock_;
   ~TaskRunnerProxy() override {}
 };
 
 TEST_F(StartupTaskRunnerTest, SynchronousExecution) {
   MockTaskRunner mock_runner;
   scoped_refptr<TaskRunnerProxy> proxy = new TaskRunnerProxy(&mock_runner);
@@ skipping 130 matching lines @@
   runner.StartRunningTasksAsync();
 
   // No tasks should have run yet, since we the message loop hasn't run.
   EXPECT_EQ(GetLastTask(), 0);
 
   // Fake the actual message loop. Each time a task is run a new task should
   // be added to the queue, hence updating "task". The loop should actually run
   // 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);
   EXPECT_EQ(observer_calls, 1);
   EXPECT_EQ(observer_result, 0);
 
   // Check that running synchronously now doesn't do anything
 
   runner.RunAllTasksNow();
   EXPECT_EQ(task_count, 2);
@@ skipping 26 matching lines @@
   runner.StartRunningTasksAsync();
 
   // No tasks should have run yet, since we the message loop hasn't run.
   EXPECT_EQ(GetLastTask(), 0);
 
   // Fake the actual message loop. Each time a task is run a new task should
   // be added to the queue, hence updating "task". The loop should actually run
   // 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);
 
   EXPECT_EQ(observer_calls, 1);
   EXPECT_EQ(observer_result, 1);
 
   // Check that running synchronously now doesn't do anything
   runner.RunAllTasksNow();
   EXPECT_EQ(observer_calls, 1);
   EXPECT_EQ(task_count, 1);
 }
 }  // namespace
 }  // namespace content