Fix a benign use-after-move in BarrierClosure.
This use-after-move would occur if `done_callback.Run()` could be called
in `BarrierInfo.Run` when `num_callbacks_left` == 0, since at that point
`done_closure` has already been executed. This issue is benign, since
the BarrierInfo already DCHECKs that a `num_callbacks_left` > 0 before
executing `done_closure.Run()`. However, it's nice to avoid the
use-after-move more explicitly; and to avoid the BarrierInfo allocation
in cases when we know we'll never need it.
Change-Id: I568570d0582598b712adb203330dca975190cccc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043927
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Chris Fredrickson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#905890}
diff --git a/base/barrier_closure_unittest.cc b/base/barrier_closure_unittest.cc
index 7bb7285..937d64586 100644
--- a/base/barrier_closure_unittest.cc
+++ b/base/barrier_closure_unittest.cc
@@ -5,7 +5,9 @@
#include "base/barrier_closure.h"
#include "base/bind.h"
+#include "base/callback_helpers.h"
#include "base/test/bind.h"
+#include "base/test/gtest_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
@@ -15,7 +17,14 @@
base::RepeatingClosure barrier_closure = base::BarrierClosure(
0, base::BindLambdaForTesting([&count]() { ++count; }));
EXPECT_EQ(1, count);
+}
+
+TEST(BarrierClosureTest, ChecksIfCalledForZeroClosures) {
+ base::RepeatingClosure barrier_closure =
+ base::BarrierClosure(0, base::DoNothing());
EXPECT_FALSE(barrier_closure.is_null());
+
+ EXPECT_CHECK_DEATH(barrier_closure.Run());
}
TEST(BarrierClosureTest, RunAfterNumClosures) {
