Fix flaky UploadClientTest.CreateUploadClient test
The flakiness seems to be a test issue: apparently calling waiter.Signal
by the callback directly sporadically blocks. The fix moves this call
to a threadpool, and after that change 100 runs got no failures under
stress (without the fix about 20 runs out of 100 crash).
I would prefer to move whole callback to the thread in
DmServerUploadService, but all my attempts to do so still ended up with
the test being flaky.
We may need a better fix inside the code rather than the test, but for
now I see no other option.
(cherry picked from commit 2b9f58095ad003bb644ad827d3083cd2de8ffb5f)
Bug: 1133968
Change-Id: I8ecde7f5c65911b75679cae5130985713233016f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2448657
Auto-Submit: Leonid Baraz <[email protected]>
Commit-Queue: Zach Trudo <[email protected]>
Reviewed-by: Zach Trudo <[email protected]>
Reviewed-by: Leonid Baraz <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#813758}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2458949
Commit-Queue: Leonid Baraz <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#121}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/chrome/browser/policy/messaging_layer/upload/upload_client_unittest.cc b/chrome/browser/policy/messaging_layer/upload/upload_client_unittest.cc
index b979736..bf31131 100644
--- a/chrome/browser/policy/messaging_layer/upload/upload_client_unittest.cc
+++ b/chrome/browser/policy/messaging_layer/upload/upload_client_unittest.cc
@@ -86,18 +86,22 @@
class TestCallbackWaiterWithCounter : public TestCallbackWaiter {
public:
- explicit TestCallbackWaiterWithCounter(int counter_limit)
- : counter_limit_(counter_limit) {}
+ explicit TestCallbackWaiterWithCounter(size_t counter_limit)
+ : counter_limit_(counter_limit) {
+ DCHECK_GT(counter_limit, 0u);
+ }
void Signal() override {
- DCHECK_GT(counter_limit_, 0);
- if (--counter_limit_ == 0) {
- run_loop_->Quit();
+ const size_t old_count = counter_limit_.fetch_sub(1);
+ DCHECK_GT(old_count, 0u);
+ if (old_count > 1) {
+ return;
}
+ run_loop_->Quit();
}
private:
- std::atomic<int> counter_limit_;
+ std::atomic<size_t> counter_limit_;
};
class UploadClientTest : public ::testing::Test {
@@ -152,7 +156,10 @@
.WillRepeatedly(WithArgs<1>(
Invoke([&waiter](AppInstallReportHandler::ClientCallback& callback) {
std::move(callback).Run(true);
- waiter.Signal();
+ base::ThreadPool::PostTask(
+ FROM_HERE, {base::TaskPriority::BEST_EFFORT},
+ base::BindOnce(&TestCallbackWaiterWithCounter::Signal,
+ base::Unretained(&waiter)));
})));
TestEvent<StatusOr<std::unique_ptr<UploadClient>>> e;
