Commit 5324aced authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Adopt ScopedAllowBlockingForTesting in FakeServer

We've wondered why some tests failed flakily instead of failing
deterministically due to blocking operations not being allowed in the
UI thread.

One explanation is the FakeServer's direct use of
base::ThreadRestrictions::SetIOAllowed(), which doesn't revert the
allowing of IO. This has the side effect that it would hide any misuse
of blocking IO operations in sync integration tests.

This patch fixes one remaining test surfaced as part of this work.

Bug: 1004312,1009372
Change-Id: Id8e57485860873e78093c18069935a646514a469
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1883630
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710730}
parent a8f41a37
...@@ -186,6 +186,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientSecondaryAccountSyncTest, ...@@ -186,6 +186,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientSecondaryAccountSyncTest,
// Save the cache GUID to file to remember after restart, for test // Save the cache GUID to file to remember after restart, for test
// verification purposes only. // verification purposes only.
base::ScopedAllowBlockingForTesting allow_blocking;
ASSERT_NE(-1, base::WriteFile(GetTestFilePathForCacheGuid(), ASSERT_NE(-1, base::WriteFile(GetTestFilePathForCacheGuid(),
cache_guid.c_str(), cache_guid.size())); cache_guid.c_str(), cache_guid.size()));
} }
......
...@@ -41,7 +41,7 @@ FakeServer::FakeServer() ...@@ -41,7 +41,7 @@ FakeServer::FakeServer()
error_type_(sync_pb::SyncEnums::SUCCESS), error_type_(sync_pb::SyncEnums::SUCCESS),
alternate_triggered_errors_(false), alternate_triggered_errors_(false),
request_counter_(0) { request_counter_(0) {
base::ThreadRestrictions::SetIOAllowed(true); base::ScopedAllowBlockingForTesting allow_blocking;
loopback_server_storage_ = std::make_unique<base::ScopedTempDir>(); loopback_server_storage_ = std::make_unique<base::ScopedTempDir>();
if (!loopback_server_storage_->CreateUniqueTempDir()) { if (!loopback_server_storage_->CreateUniqueTempDir()) {
NOTREACHED() << "Creating temp dir failed."; NOTREACHED() << "Creating temp dir failed.";
...@@ -56,7 +56,7 @@ FakeServer::FakeServer(const base::FilePath& user_data_dir) ...@@ -56,7 +56,7 @@ FakeServer::FakeServer(const base::FilePath& user_data_dir)
error_type_(sync_pb::SyncEnums::SUCCESS), error_type_(sync_pb::SyncEnums::SUCCESS),
alternate_triggered_errors_(false), alternate_triggered_errors_(false),
request_counter_(0) { request_counter_(0) {
base::ThreadRestrictions::SetIOAllowed(true); base::ScopedAllowBlockingForTesting allow_blocking;
base::FilePath loopback_server_path = base::FilePath loopback_server_path =
user_data_dir.AppendASCII("FakeSyncServer"); user_data_dir.AppendASCII("FakeSyncServer");
loopback_server_ = std::make_unique<syncer::LoopbackServer>( loopback_server_ = std::make_unique<syncer::LoopbackServer>(
...@@ -64,7 +64,10 @@ FakeServer::FakeServer(const base::FilePath& user_data_dir) ...@@ -64,7 +64,10 @@ FakeServer::FakeServer(const base::FilePath& user_data_dir)
loopback_server_->set_observer_for_tests(this); loopback_server_->set_observer_for_tests(this);
} }
FakeServer::~FakeServer() {} FakeServer::~FakeServer() {
base::ScopedAllowBlockingForTesting allow_blocking;
loopback_server_storage_.reset();
}
namespace { namespace {
...@@ -306,7 +309,7 @@ net::HttpStatusCode FakeServer::HandleParsedCommand( ...@@ -306,7 +309,7 @@ net::HttpStatusCode FakeServer::HandleParsedCommand(
net::HttpStatusCode FakeServer::SendToLoopbackServer( net::HttpStatusCode FakeServer::SendToLoopbackServer(
const sync_pb::ClientToServerMessage& message, const sync_pb::ClientToServerMessage& message,
sync_pb::ClientToServerResponse* response) { sync_pb::ClientToServerResponse* response) {
base::ThreadRestrictions::SetIOAllowed(true); base::ScopedAllowBlockingForTesting allow_blocking;
return loopback_server_->HandleCommand(message, response); return loopback_server_->HandleCommand(message, response);
} }
...@@ -438,6 +441,7 @@ bool FakeServer::ModifyBookmarkEntity( ...@@ -438,6 +441,7 @@ bool FakeServer::ModifyBookmarkEntity(
void FakeServer::ClearServerData() { void FakeServer::ClearServerData() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
base::ScopedAllowBlockingForTesting allow_blocking;
loopback_server_->ClearServerData(); loopback_server_->ClearServerData();
} }
......
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