Commit 00758091 authored by Maxim Kolosovskiy's avatar Maxim Kolosovskiy Committed by Commit Bot

Revert "Fix RunOnceClosure(OnceClosure) to take a OnceClosure by value"

This reverts commit 5bf31665.

Reason for revert: this CL probably caused GmockCallbackSupportTest.RunOnceClosureValueMultipleCall failures. 
The first failed build https://ci.chromium.org/p/chrome/builders/ci/chromeos-eve-chrome/3283

Test 'GmockCallbackSupportTest.RunOnceClosureValueMultipleCall' completed with the following status(es): 'FAILURE','FAILURE'

Test 'GmockCallbackSupportTest.RunOnceClosureValueMultipleCall' had the following logs when run:

================================================================================

[ RUN      ] GmockCallbackSupportTest.RunOnceClosureValueMultipleCall
../../base/test/gmock_callback_support_unittest.cc:174: Failure
Death test: check.Call()
    Result: died but not with expected error.
  Expected: contains regular expression "copyable_cb->data"
Actual msg:
[  DEATH   ]
Stack trace:
#0 0x57c25d527cd4 base::test::GmockCallbackSupportTest_RunOnceClosureValueMultipleCall_Test::TestBody()

[  FAILED  ] GmockCallbackSupportTest.RunOnceClosureValueMultipleCall (26 ms)
[ RUN      ] GmockCallbackSupportTest.RunOnceClosureValueMultipleCall
../../base/test/gmock_callback_support_unittest.cc:174: Failure
Death test: check.Call()
    Result: died but not with expected error.
  Expected: contains regular expression "copyable_cb->data"
Actual msg:
[  DEATH   ]
Stack trace:
#0 0x58a33c3f9cd4 base::test::GmockCallbackSupportTest_RunOnceClosureValueMultipleCall_Test::TestBody()

[  FAILED  ] GmockCallbackSupportTest.RunOnceClosureValueMultipleCall (29 ms)

Original change's description:
> Fix RunOnceClosure(OnceClosure) to take a OnceClosure by value
> 
> Bug: None
> Change-Id: Iddf32bab6d0ab7ca195db02cc24d31e669a49051
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037184
> Commit-Queue: Anand Mistry <amistry@chromium.org>
> Reviewed-by: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740079}

TBR=amistry@chromium.org,kylechar@chromium.org

Change-Id: I913831e1a782e9e684417582b5a8503c2f9a9fd2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050491Reviewed-by: default avatarMaxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740296}
parent 791195c0
......@@ -10,8 +10,6 @@
#include <utility>
#include "base/callback.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace base {
......@@ -72,28 +70,14 @@ MATCHER(IsNotNullCallback, "a non-null callback") {
}
// The Run[Once]Closure() action invokes the Run() method on the closure
// provided when the action is constructed. Function arguments passed when the
// action is run will be ignored.
// provided when the action is constructed.
ACTION_P(RunClosure, closure) {
closure.Run();
}
// This action can be invoked at most once. Any further invocation will trigger
// a CHECK failure.
inline auto RunOnceClosure(base::OnceClosure cb) {
// Mock actions need to be copyable, but OnceClosure is not. Wrap the closure
// in a base::RefCountedData<> to allow it to be copied. An alternative would
// be to use AdaptCallbackForRepeating(), but that allows the closure to be
// run more than once and silently ignores any invocation after the first.
// Since this is for use by tests, it's better to crash or CHECK-fail and
// surface the incorrect usage, rather than have a silent unexpected success.
using RefCountedOnceClosure = base::RefCountedData<base::OnceClosure>;
scoped_refptr<RefCountedOnceClosure> copyable_cb =
base::MakeRefCounted<RefCountedOnceClosure>(std::move(cb));
return [copyable_cb](auto&&...) {
CHECK(copyable_cb->data);
std::move(copyable_cb->data).Run();
};
// TODO(kylechar): This action can't take a OnceClosure by value, fix that.
ACTION_P(RunOnceClosure, closure) {
std::move(closure).Run();
}
// The Run[Once]Closure<N>() action invokes the Run() method on the N-th
......
......@@ -124,55 +124,5 @@ TEST(GmockCallbackSupportTest, RunOnceCallback0) {
EXPECT_TRUE(dst);
}
TEST(GmockCallbackSupportTest, RunClosureValue) {
MockFunction<void()> check;
bool dst = false;
EXPECT_CALL(check, Call())
.WillOnce(RunClosure(base::BindRepeating(&SetBool, true, &dst)));
check.Call();
EXPECT_TRUE(dst);
}
TEST(GmockCallbackSupportTest, RunClosureValueWithArgs) {
MockFunction<void(bool, int)> check;
bool dst = false;
EXPECT_CALL(check, Call(true, 42))
.WillOnce(RunClosure(base::BindRepeating(&SetBool, true, &dst)));
check.Call(true, 42);
EXPECT_TRUE(dst);
}
TEST(GmockCallbackSupportTest, RunOnceClosureValue) {
MockFunction<void()> check;
bool dst = false;
EXPECT_CALL(check, Call())
.WillOnce(RunOnceClosure(base::BindOnce(&SetBool, true, &dst)));
check.Call();
EXPECT_TRUE(dst);
}
TEST(GmockCallbackSupportTest, RunOnceClosureValueWithArgs) {
MockFunction<void(bool, int)> check;
bool dst = false;
EXPECT_CALL(check, Call(true, 42))
.WillOnce(RunOnceClosure(base::BindOnce(&SetBool, true, &dst)));
check.Call(true, 42);
EXPECT_TRUE(dst);
}
TEST(GmockCallbackSupportTest, RunOnceClosureValueMultipleCall) {
MockFunction<void()> check;
bool dst = false;
EXPECT_CALL(check, Call())
.WillRepeatedly(RunOnceClosure(base::BindOnce(&SetBool, true, &dst)));
check.Call();
EXPECT_TRUE(dst);
// Invoking the RunOnceClosure action more than once will trigger a
// CHECK-failure.
dst = false;
EXPECT_DEATH_IF_SUPPORTED(check.Call(), "copyable_cb->data");
}
} // namespace test
} // namespace base
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment