[base] Make RunOnceCallback support move-only types

This change adds support for move-only types to
base::test::RunOnceCallback. If a move-only type is passed to the
constructor of RunOnceCallback, it will later be passed by move to
the OnceCallback that should be run. This invalidates the action,
and running it twice will result in a run-time crash.

Bug: 752720
Change-Id: I43ca5e81e9323c7c374e67de14be2637fbfea589
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379840
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803630}

base/test/gmock_callback_support.h

@@ -7,6 +7,7 @@
 
 #include <functional>
 #include <tuple>
+#include <type_traits>
 #include <utility>
 
 #include "base/callback.h"
@@ -15,51 +16,83 @@
 #include "testing/gmock/include/gmock/gmock.h"
 
 namespace base {
-namespace test {
 
-// TODO(crbug.com/752720): Simplify using std::apply once C++17 is available.
-template <typename CallbackFunc, typename ArgTuple, size_t... I>
-decltype(auto) RunOnceCallbackUnwrapped(CallbackFunc&& f,
-                                        ArgTuple&& t,
-                                        std::index_sequence<I...>) {
-  return std::move(f).Run(std::get<I>(t)...);
+namespace internal {
+
+// Small helper to get the `I`th argument.
+template <size_t I, typename... Args>
+decltype(auto) get(Args&&... args) {
+  return std::get<I>(std::forward_as_tuple(std::forward<Args>(args)...));
+}
+
+// Invokes `cb` with the arguments stored in `tuple`. Both `cb` and `tuple` are
+// perfectly forwarded, allowing callers to specify whether they should be
+// passed by move or copy.
+template <typename Callback, typename Tuple, size_t... Is>
+decltype(auto) RunImpl(Callback&& cb,
+                       Tuple&& tuple,
+                       std::index_sequence<Is...>) {
+  return std::forward<Callback>(cb).Run(
+      std::get<Is>(std::forward<Tuple>(tuple))...);
+}
+
+// Invokes `cb` with the arguments stored in `tuple`. Both `cb` and `tuple` are
+// perfectly forwarded, allowing callers to specify whether they should be
+// passed by move or copy. Needs to dispatch to the three arguments version to
+// be able to construct a `std::index_sequence` of the corresponding size.
+template <typename Callback, typename Tuple>
+decltype(auto) RunImpl(Callback&& cb, Tuple&& tuple) {
+  return RunImpl(std::forward<Callback>(cb), std::forward<Tuple>(tuple),
+                 std::make_index_sequence<
+                     std::tuple_size<std::remove_reference_t<Tuple>>::value>());
+}
+
+// Invoked when the arguments to a OnceCallback are copy constructible. In this
+// case the returned lambda will pass the arguments to the provided callback by
+// copy, allowing it to be used multiple times.
+template <size_t I,
+          typename Tuple,
+          std::enable_if_t<std::is_copy_constructible<Tuple>::value, int> = 0>
+auto RunOnceCallbackImpl(Tuple&& tuple) {
+  return [tuple = std::move(tuple)](auto&&... args) -> decltype(auto) {
+    return RunImpl(std::move(internal::get<I>(args...)), tuple);
+  };
+}
+
+// Invoked when the arguments to a OnceCallback are not copy constructible. In
+// this case the returned lambda will pass the arguments to the provided
+// callback by move, allowing it to be only used once.
+template <size_t I,
+          typename Tuple,
+          std::enable_if_t<!std::is_copy_constructible<Tuple>::value, int> = 0>
+auto RunOnceCallbackImpl(Tuple&& tuple) {
+  // Mock actions need to be copyable, but `tuple` is not. Wrap it in in a
+  // `scoped_refptr` to allow it to be copied.
+  auto tuple_ptr =
+      base::MakeRefCounted<base::RefCountedData<Tuple>>(std::move(tuple));
+  return [tuple_ptr =
+              std::move(tuple_ptr)](auto&&... args) mutable -> decltype(auto) {
+    // Since running the action will move out of the arguments, `tuple_ptr` is
+    // nulled out, so that attempting to run it twice will result in a run-time
+    // crash.
+    return RunImpl(std::move(internal::get<I>(args...)),
+                   std::move(std::exchange(tuple_ptr, nullptr)->data));
+  };
 }
 
-// TODO(crbug.com/752720): Simplify using std::apply once C++17 is available.
-template <typename CallbackFunc, typename ArgTuple, size_t... I>
-decltype(auto) RunRepeatingCallbackUnwrapped(CallbackFunc&& f,
-                                             ArgTuple&& t,
-                                             std::index_sequence<I...>) {
-  return f.Run(std::get<I>(t)...);
+// Invoked for RepeatingCallbacks. In this case the returned lambda will pass
+// the arguments to the provided callback by copy, allowing it to be used
+// multiple times. Move-only arguments are not supported.
+template <size_t I, typename Tuple>
+auto RunRepeatingCallbackImpl(Tuple&& tuple) {
+  return [tuple = std::move(tuple)](auto&&... args) -> decltype(auto) {
+    return RunImpl(internal::get<I>(args...), tuple);
+  };
 }
 
-// Functor used for RunOnceClosure<N>() and RunOnceCallback<N>() actions.
-template <size_t I, typename... Vals>
-struct RunOnceCallbackAction {
-  std::tuple<Vals...> vals;
-
-  template <typename... Args>
-  decltype(auto) operator()(Args&&... args) {
-    constexpr size_t size = std::tuple_size<decltype(vals)>::value;
-    return RunOnceCallbackUnwrapped(
-        std::get<I>(std::forward_as_tuple(std::forward<Args>(args)...)),
-        std::move(vals), std::make_index_sequence<size>{});
-  }
-};
-
-// Functor used for RunClosure<N>() and RunCallback<N>() actions.
-template <size_t I, typename... Vals>
-struct RunRepeatingCallbackAction {
-  std::tuple<Vals...> vals;
-
-  template <typename... Args>
-  decltype(auto) operator()(Args&&... args) {
-    constexpr size_t size = std::tuple_size<decltype(vals)>::value;
-    return RunRepeatingCallbackUnwrapped(
-        std::get<I>(std::forward_as_tuple(std::forward<Args>(args)...)),
-        std::move(vals), std::make_index_sequence<size>{});
-  }
-};
+}  // namespace internal
+
+namespace test {
 
 // Matchers for base::{Once,Repeating}Callback and
 // base::{Once,Repeating}Closure.
@@ -99,13 +132,17 @@ inline auto RunOnceClosure(base::OnceClosure cb) {
 // The Run[Once]Closure<N>() action invokes the Run() method on the N-th
 // (0-based) argument of the mock function.
 template <size_t I>
-RunRepeatingCallbackAction<I> RunClosure() {
-  return {};
+auto RunClosure() {
+  return [](auto&&... args) -> decltype(auto) {
+    return internal::get<I>(args...).Run();
+  };
 }
 
 template <size_t I>
-RunOnceCallbackAction<I> RunOnceClosure() {
-  return {};
+auto RunOnceClosure() {
+  return [](auto&&... args) -> decltype(auto) {
+    return std::move(internal::get<I>(args...)).Run();
+  };
 }
 
 // The Run[Once]Callback<N>(p1, p2, ..., p_k) action invokes the Run() method on
@@ -132,16 +169,24 @@ RunOnceCallbackAction<I> RunOnceClosure() {
 //   reference of the copy, instead of the original temporary object,
 //   to the callback.  This makes it easy for a user to define an
 //   RunCallback action from temporary values and have it performed later.
-template <size_t I, typename... Vals>
-RunOnceCallbackAction<I, std::decay_t<Vals>...> RunOnceCallback(
-    Vals&&... vals) {
-  return {std::forward_as_tuple(std::forward<Vals>(vals)...)};
+//
+//   3. In order to facilitate re-use of the `RunOnceCallback()` action,
+//   the arguments are copied during each run if possible. If this can't
+//   be done (e.g. one of the arguments is move-only), the arguments will
+//   be passed by move. However, since moving potentially invalidates the
+//   arguments, the resulting action is only allowed to run once in this
+//   case. Attempting to run it twice will result in a runtime crash.
+//   Using move-only arguments with `RunCallback()` is not supported.
+template <size_t I, typename... RunArgs>
+auto RunOnceCallback(RunArgs&&... run_args) {
+  return internal::RunOnceCallbackImpl<I>(
+      std::make_tuple(std::forward<RunArgs>(run_args)...));
 }
 
-template <size_t I, typename... Vals>
-RunRepeatingCallbackAction<I, std::decay_t<Vals>...> RunCallback(
-    Vals&&... vals) {
-  return {std::forward_as_tuple(std::forward<Vals>(vals)...)};
+template <size_t I, typename... RunArgs>
+auto RunCallback(RunArgs&&... run_args) {
+  return internal::RunRepeatingCallbackImpl<I>(
+      std::make_tuple(std::forward<RunArgs>(run_args)...));
 }
 
 }  // namespace test

base/test/gmock_callback_support_unittest.cc

@@ -4,11 +4,14 @@
 
 #include "base/test/gmock_callback_support.h"
 
+#include <memory>
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
 using testing::ByRef;
 using testing::MockFunction;
 
@@ -17,11 +20,17 @@ namespace test {
 
 using TestCallback = base::RepeatingCallback<void(const bool& src, bool* dst)>;
 using TestOnceCallback = base::OnceCallback<void(const bool& src, bool* dst)>;
+using TestOnceCallbackMove =
+    base::OnceCallback<void(std::unique_ptr<int>, int* dst)>;
 
 void SetBool(const bool& src, bool* dst) {
   *dst = src;
 }
 
+void SetIntFromPtr(std::unique_ptr<int> ptr, int* dst) {
+  *dst = *ptr;
+}
+
 TEST(GmockCallbackSupportTest, IsNullCallback) {
   MockFunction<void(const TestCallback&)> check;
   EXPECT_CALL(check, Call(IsNullCallback()));
@@ -124,6 +133,22 @@ TEST(GmockCallbackSupportTest, RunOnceCallback0) {
   EXPECT_TRUE(dst);
 }
 
+TEST(GmockCallbackSupportTest, RunOnceCallbackTwice) {
+  MockFunction<void(TestOnceCallback)> check;
+  bool dst = false;
+  bool src = true;
+
+  EXPECT_CALL(check, Call)
+      .WillRepeatedly(RunOnceCallback<0>(std::ref(src), &dst));
+
+  check.Call(base::BindOnce(&SetBool));
+  EXPECT_TRUE(dst);
+
+  src = false;
+  check.Call(base::BindOnce(&SetBool));
+  EXPECT_FALSE(dst);
+}
+
 TEST(GmockCallbackSupportTest, RunClosureValue) {
   MockFunction<void()> check;
   bool dst = false;
@@ -174,5 +199,58 @@ TEST(GmockCallbackSupportTest, RunOnceClosureValueMultipleCall) {
   EXPECT_DEATH_IF_SUPPORTED(check.Call(), "");
 }
 
+TEST(GmockCallbackSupportTest, RunOnceCallbackWithMoveOnlyType) {
+  MockFunction<void(TestOnceCallbackMove)> check;
+  auto val = std::make_unique<int>(42);
+  int dst = 0;
+  EXPECT_CALL(check, Call).WillOnce(RunOnceCallback<0>(std::move(val), &dst));
+
+  check.Call(base::BindOnce(&SetIntFromPtr));
+
+  EXPECT_EQ(dst, 42);
+  EXPECT_FALSE(val);
+}
+
+TEST(GmockCallbackSupportTest,
+     RunOnceCallbackMultipleTimesWithMoveOnlyArgCrashes) {
+  MockFunction<void(TestOnceCallbackMove)> check;
+  auto val = std::make_unique<int>(42);
+  int dst = 0;
+  EXPECT_CALL(check, Call)
+      .WillRepeatedly(RunOnceCallback<0>(std::move(val), &dst));
+
+  check.Call(base::BindOnce(&SetIntFromPtr));
+  EXPECT_EQ(dst, 42);
+  EXPECT_FALSE(val);
+
+  // The first `Call` has invalidated the captured std::unique_ptr. Attempting
+  // to run `Call` again should result in a runtime crash.
+  EXPECT_DEATH_IF_SUPPORTED(check.Call(base::BindOnce(&SetIntFromPtr)), "");
+}
+
+TEST(GmockCallbackSupportTest, RunOnceCallbackReturnsValue) {
+  MockFunction<int(base::OnceCallback<int(int)>)> check;
+  EXPECT_CALL(check, Call).WillRepeatedly(RunOnceCallback<0>(42));
+  EXPECT_EQ(43, check.Call(base::BindOnce([](int i) { return i + 1; })));
+  EXPECT_EQ(44, check.Call(base::BindOnce([](int i) { return i + 2; })));
+  EXPECT_EQ(45, check.Call(base::BindOnce([](int i) { return i + 3; })));
+}
+
+TEST(GmockCallbackSupportTest, RunOnceCallbackReturnsValueMoveOnly) {
+  MockFunction<int(base::OnceCallback<int(std::unique_ptr<int>)>)> check;
+  EXPECT_CALL(check, Call)
+      .WillOnce(RunOnceCallback<0>(std::make_unique<int>(42)));
+  EXPECT_EQ(43, check.Call(base::BindOnce(
+                    [](std::unique_ptr<int> i) { return *i + 1; })));
+}
+
+TEST(GmockCallbackSupportTest, RunCallbackReturnsValue) {
+  MockFunction<int(base::RepeatingCallback<int(int)>)> check;
+  EXPECT_CALL(check, Call).WillRepeatedly(RunCallback<0>(42));
+  EXPECT_EQ(43, check.Call(base::BindRepeating([](int i) { return i + 1; })));
+  EXPECT_EQ(44, check.Call(base::BindRepeating([](int i) { return i + 2; })));
+  EXPECT_EQ(45, check.Call(base::BindRepeating([](int i) { return i + 3; })));
+}
+
 }  // namespace test
 }  // namespace base