Fix for test flakiness.

ReportClient creation made fully asynchronous,
and problematic test can now wait for initialization to complete.

Bug: b:169427520
Change-Id: Ic6566e9973dc48de86b36c8b1180d688de84ce76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444409Reviewed-by: Zach Trudo <zatrudo@google.com>
Commit-Queue: Leonid Baraz <lbaraz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813206}

chrome/browser/policy/messaging_layer/public/report_client.cc

@@ -38,7 +38,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
-#if defined(OS_CHROMEOS)
+#ifdef OS_CHROMEOS
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
@@ -57,7 +57,7 @@ namespace {
 // NOT THREAD SAFE - these functions must be called on the main thread.
 // TODO(chromium:1078512) Wrap CloudPolicyClient in a new object so that its
 // methods, constructor, and destructor are accessed on the correct thread.
-#if defined(OS_CHROMEOS)
+#ifdef OS_CHROMEOS
 void BuildCloudPolicyClient(
     base::OnceCallback<
         void(StatusOr<std::unique_ptr<policy::CloudPolicyClient>>)> build_cb) {
@@ -376,7 +376,7 @@ void ReportingClient::InitializingContext::OnLeaderPromotionResult(
     StatusOr<ReportingClient::InitializationStateTracker::ReleaseLeaderCallback>
         promo_result) {
   if (promo_result.status().error_code() == error::FAILED_PRECONDITION) {
-    // Between building this InitializationContext and attempting to promote to
+    // Between building this InitializingContext and attempting to promote to
     // leader, the ReportingClient was configured. Ok response.
     Complete(Status::StatusOK());
     return;
@@ -449,11 +449,41 @@ void ReportingClient::InitializingContext::OnStorageModuleConfigured(
   }
 
   client_config_->storage = storage_result.ValueOrDie();
+  Schedule(
+      base::BindOnce(&ReportingClient::InitializingContext::CreateUploadClient,
+                     base::Unretained(this)));
+}
+
+void ReportingClient::InitializingContext::CreateUploadClient() {
+  ReportingClient* const instance = GetInstance();
+  DCHECK(!instance->upload_client_);
+  UploadClient::Create(
+      std::move(client_config_->cloud_policy_client),
+      base::BindRepeating(&StorageModule::ReportSuccess,
+                          client_config_->storage),
+      base::BindOnce(&InitializingContext::OnUploadClientCreated,
+                     base::Unretained(this)));
+}
+
+void ReportingClient::InitializingContext::OnUploadClientCreated(
+    StatusOr<std::unique_ptr<UploadClient>> upload_client_result) {
+  if (!upload_client_result.ok()) {
+    Complete(Status(error::FAILED_PRECONDITION,
+                    base::StrCat({"Unable to create UploadClient: ",
+                                  upload_client_result.status().message()})));
+    return;
+  }
   Schedule(&ReportingClient::InitializingContext::UpdateConfiguration,
-           base::Unretained(this));
+           base::Unretained(this),
+           std::move(upload_client_result.ValueOrDie()));
 }
 
-void ReportingClient::InitializingContext::UpdateConfiguration() {
+void ReportingClient::InitializingContext::UpdateConfiguration(
+    std::unique_ptr<UploadClient> upload_client) {
+  ReportingClient* const instance = GetInstance();
+  DCHECK(!instance->upload_client_);
+  instance->upload_client_ = std::move(upload_client);
+
   std::move(update_config_cb_)
       .Run(std::move(client_config_),
            base::BindOnce(&ReportingClient::InitializingContext::Complete,
@@ -603,13 +633,7 @@ void ReportingClient::BuildRequestQueue(
 StatusOr<std::unique_ptr<Storage::UploaderInterface>>
 ReportingClient::BuildUploader(Priority priority) {
   ReportingClient* const instance = GetInstance();
-  if (instance->upload_client_ == nullptr) {
-    ASSIGN_OR_RETURN(
-        instance->upload_client_,
-        UploadClient::Create(std::move(instance->config_->cloud_policy_client),
-                             base::BindRepeating(&StorageModule::ReportSuccess,
-                                                 instance->config_->storage)));
-  }
+  DCHECK(instance->upload_client_);
   return Uploader::Create(
       base::BindOnce(&UploadClient::EnqueueUpload,
                      base::Unretained(instance->upload_client_.get())));

chrome/browser/policy/messaging_layer/public/report_client.h

@@ -133,7 +133,11 @@ class ReportingClient {
     void OnStorageModuleConfigured(
         StatusOr<scoped_refptr<StorageModule>> storage_result);
 
-    void UpdateConfiguration();
+    void CreateUploadClient();
+    void OnUploadClientCreated(
+        StatusOr<std::unique_ptr<UploadClient>> upload_client_result);
+
+    void UpdateConfiguration(std::unique_ptr<UploadClient> upload_client);
 
     // Complete calls response with |client_config_|
     void Complete(Status status);

chrome/browser/policy/messaging_layer/public/report_client_unittest.cc

@@ -21,12 +21,11 @@
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// #if defined(OS_CHROMEOS)
-// #include "chrome/browser/chromeos/settings/device_settings_service.h"
-// #include "components/policy/proto/chrome_device_policy.pb.h"
-// #else
-// #include "chrome/browser/policy/chrome_browser_policy_connector.h"
-// #endif
+#ifdef OS_CHROMEOS
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/user_manager/scoped_user_manager.h"
+#endif  // OS_CHROMEOS
 
 namespace reporting {
 namespace {
@@ -54,16 +53,44 @@ class TestCallbackWaiter {
 class ReportingClientTest : public testing::Test {
  public:
   void SetUp() override {
+#ifdef OS_CHROMEOS
+    // Set up fake primary profile.
+    auto mock_user_manager =
+        std::make_unique<testing::NiceMock<chromeos::FakeChromeUserManager>>();
+    profile_ = std::make_unique<TestingProfile>(
+        base::FilePath(FILE_PATH_LITERAL("/home/chronos/u-0123456789abcdef")));
+    const AccountId account_id(AccountId::FromUserEmailGaiaId(
+        profile_->GetProfileUserName(), "12345"));
+    const user_manager::User* user =
+        mock_user_manager->AddPublicAccountUser(account_id);
+    mock_user_manager->UserLoggedIn(account_id, user->username_hash(),
+                                    /*browser_restart=*/false,
+                                    /*is_child=*/false);
+    user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
+        std::move(mock_user_manager));
+#endif  // OS_CHROMEOS
+
+    // Provide a mock cloud policy client.
     auto client = std::make_unique<policy::MockCloudPolicyClient>();
     client->SetDMToken(
         policy::DMToken::CreateValidTokenForTesting("FAKE_DM_TOKEN").value());
     ReportingClient::Setup_test(std::move(client));
   }
 
-  void TearDown() override { ReportingClient::Reset_test(); }
+  void TearDown() override {
+    ReportingClient::Reset_test();
+#ifdef OS_CHROMEOS
+    user_manager_.reset();
+    profile_.reset();
+#endif  // OS_CHROMEOS
+  }
 
  protected:
   content::BrowserTaskEnvironment task_envrionment_;
+#ifdef OS_CHROMEOS
+  std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<user_manager::ScopedUserManager> user_manager_;
+#endif  // OS_CHROMEOS
   const DMToken dm_token_ = DMToken::CreateValidTokenForTesting("TOKEN");
   const Destination destination_ = Destination::UPLOAD_EVENTS;
   const Priority priority_ = Priority::IMMEDIATE;

chrome/browser/policy/messaging_layer/upload/dm_server_upload_service.cc

@@ -26,13 +26,14 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 
-#if defined(OS_CHROMEOS)
+#ifdef OS_CHROMEOS
 #include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 
 namespace reporting {
 namespace {
 
+// This function must run on UI thread.
 StatusOr<Profile*> GetPrimaryProfile() {
   if (!user_manager::UserManager::IsInitialized()) {
     return Status(error::FAILED_PRECONDITION, "User manager not initialized");
@@ -250,18 +251,39 @@ void DmServerUploader::ProcessSuccessfulUploadAddition(
   highest_successful_sequence_ = sequencing_information;
 }
 
-StatusOr<std::unique_ptr<DmServerUploadService>> DmServerUploadService::Create(
+void DmServerUploadService::Create(
     std::unique_ptr<policy::CloudPolicyClient> client,
-    ReportSuccessfulUploadCallback upload_cb) {
+    ReportSuccessfulUploadCallback report_upload_success_cb,
+    base::OnceCallback<void(StatusOr<std::unique_ptr<DmServerUploadService>>)>
+        created_cb) {
   if (client == nullptr) {
-    return Status(error::INVALID_ARGUMENT, "client may not be nullptr.");
+    std::move(created_cb)
+        .Run(Status(error::INVALID_ARGUMENT, "client may not be nullptr."));
+    return;
   }
-  auto uploader =
-      base::WrapUnique(new DmServerUploadService(std::move(client), upload_cb));
 
-  RETURN_IF_ERROR(uploader->InitRecordHandlers());
-
-  return uploader;
+  auto uploader = base::WrapUnique(
+      new DmServerUploadService(std::move(client), report_upload_success_cb));
+#ifdef OS_CHROMEOS
+  content::GetUIThreadTaskRunner({})->PostTaskAndReplyWithResult(
+      FROM_HERE, base::BindOnce(&GetPrimaryProfile),
+      base::BindOnce(
+          [](std::unique_ptr<DmServerUploadService> uploader,
+             base::OnceCallback<void(
+                 StatusOr<std::unique_ptr<DmServerUploadService>>)> created_cb,
+             StatusOr<Profile*> primary_profile_result) {
+            if (!primary_profile_result.ok()) {
+              std::move(created_cb).Run(primary_profile_result.status());
+              return;
+            }
+            InitRecordHandlers(std::move(uploader),
+                               primary_profile_result.ValueOrDie(),
+                               std::move(created_cb));
+          },
+          std::move(uploader), std::move(created_cb)));
+#else
+  InitRecordHandlers(std::move(uploader), std::move(created_cb));
+#endif  // OS_CHROMEOS
 }
 
 DmServerUploadService::DmServerUploadService(
@@ -295,24 +317,74 @@ Status DmServerUploadService::EnqueueUpload(
   return Status::StatusOK();
 }
 
-Status DmServerUploadService::InitRecordHandlers() {
-  auto* client = GetClient();
-  if (client == nullptr) {
-    return Status(error::FAILED_PRECONDITION, "Client was null");
+namespace {
+
+class CollectorCallback {
+ public:
+  CollectorCallback(size_t count, base::OnceClosure done_cb)
+      : count_(count), done_cb_(std::move(done_cb)) {
+    DCHECK_GT(count, 0u);
+  }
+  CollectorCallback(CollectorCallback& other) = delete;
+  CollectorCallback& operator=(CollectorCallback& other) = delete;
+  ~CollectorCallback() { std::move(done_cb_).Run(); }
+
+  void Decrement() {
+    size_t old_count = count_.fetch_sub(1);
+    DCHECK_GT(old_count, 0u);
+    if (old_count > 1) {
+      return;
+    }
+    delete this;
   }
 
-  record_handlers_->PushBack(std::make_unique<AppInstallReportHandler>(client),
-                             base::DoNothing());
+ private:
+  std::atomic<size_t> count_;
+  base::OnceClosure done_cb_;
+};
+}  // namespace
 
-  // Temporary wrapper for MeetDeviceTelementry
+void DmServerUploadService::InitRecordHandlers(
+    std::unique_ptr<DmServerUploadService> uploader,
 #ifdef OS_CHROMEOS
-  ASSIGN_OR_RETURN(Profile* const primary_profile, GetPrimaryProfile());
-  record_handlers_->PushBack(std::make_unique<MeetDeviceTelemetryReportHandler>(
-                                 primary_profile, client),
-                             base::DoNothing());
+    Profile* primary_profile,
 #endif  // OS_CHROMEOS
+    base::OnceCallback<void(StatusOr<std::unique_ptr<DmServerUploadService>>)>
+        created_cb) {
+  auto* client = uploader->GetClient();
+  if (client == nullptr) {
+    std::move(created_cb)
+        .Run(Status(error::FAILED_PRECONDITION, "Client was null"));
+    return;
+  }
 
-  return Status::StatusOK();
+  std::vector<std::unique_ptr<RecordHandler>> handlers;
+  handlers.emplace_back(std::make_unique<AppInstallReportHandler>(client));
+#ifdef OS_CHROMEOS
+  // Temporary wrapper for MeetDeviceTelemetry
+  handlers.emplace_back(std::make_unique<MeetDeviceTelemetryReportHandler>(
+      primary_profile, client));
+#endif  // OS_CHROMEOS
+
+  // Copy record_handlers_ aside, because uploader is going to be moved.
+  auto record_handlers = uploader->record_handlers_;
+
+  // collector_cb self-destructs upon completion.
+  CollectorCallback* const collector_cb = new CollectorCallback(
+      handlers.size(),
+      base::BindOnce(
+          [](std::unique_ptr<DmServerUploadService> uploader,
+             base::OnceCallback<void(
+                 StatusOr<std::unique_ptr<DmServerUploadService>>)>
+                 created_cb) {
+            std::move(created_cb).Run(std::move(uploader));
+          },
+          std::move(uploader), std::move(created_cb)));
+  for (auto& handler : handlers) {
+    record_handlers->PushBack(std::move(handler),
+                              base::BindOnce(&CollectorCallback::Decrement,
+                                             base::Unretained(collector_cb)));
+  }
 }
 
 void DmServerUploadService::UploadCompletion(

chrome/browser/policy/messaging_layer/upload/dm_server_upload_service.h

@@ -21,6 +21,10 @@
 #include "components/policy/proto/record_constants.pb.h"
 #include "net/base/backoff_entry.h"
 
+#ifdef OS_CHROMEOS
+#include "chrome/browser/profiles/profile.h"
+#endif  // OS_CHROMEOS
+
 namespace reporting {
 
 // DmServerUploadService uploads events to the DMServer. It does not manage
@@ -131,18 +135,20 @@ class DmServerUploadService {
     SEQUENCE_CHECKER(sequence_checker_);
   };
 
-  // Will create a DMServerUploadService with handlers.
-  // On successful completion returns a DMServerUploadService.
-  // If |client| is null, will return error::INVALID_ARGUMENT.
+  // Will asynchronously create a DMServerUploadService with handlers.
+  // On successful completion will call |created_cb| with DMServerUploadService.
+  // If |client| is null, will call |created_cb| with error::INVALID_ARGUMENT.
   // If any handlers fail to create, or the policy::CloudPolicyClient is null,
-  // will return error::UNAVAILABLE.
+  // will call |created_cb| with error::UNAVAILABLE.
   //
   // |client| must not be null.
-  // |completion_cb| should report back to the holder of the created object
-  // whenever a record set is successfully uploaded.
-  static StatusOr<std::unique_ptr<DmServerUploadService>> Create(
+  // |report_upload_success_cb| should report back to the holder of the created
+  // object whenever a record set is successfully uploaded.
+  static void Create(
       std::unique_ptr<policy::CloudPolicyClient> client,
-      ReportSuccessfulUploadCallback completion_cb);
+      ReportSuccessfulUploadCallback report_upload_success_cb,
+      base::OnceCallback<void(StatusOr<std::unique_ptr<DmServerUploadService>>)>
+          created_cb);
   ~DmServerUploadService();
 
   Status EnqueueUpload(std::unique_ptr<std::vector<EncryptedRecord>> record);
@@ -151,7 +157,13 @@ class DmServerUploadService {
   DmServerUploadService(std::unique_ptr<policy::CloudPolicyClient> client,
                         ReportSuccessfulUploadCallback completion_cb);
 
-  Status InitRecordHandlers();
+  static void InitRecordHandlers(
+      std::unique_ptr<DmServerUploadService> uploader,
+#ifdef OS_CHROMEOS
+      Profile* primary_profile,
+#endif  // OS_CHROMEOS
+      base::OnceCallback<void(StatusOr<std::unique_ptr<DmServerUploadService>>)>
+          created_cb);
 
   void UploadCompletion(StatusOr<SequencingInformation>) const;
 

chrome/browser/policy/messaging_layer/upload/dm_server_upload_service_unittest.cc

@@ -8,7 +8,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/synchronization/waitable_event.h"
 #include "base/task_runner.h"
 #include "base/test/task_environment.h"
 #include "chrome/browser/policy/messaging_layer/util/shared_vector.h"
@@ -16,6 +15,7 @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
+#include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace reporting {
@@ -24,10 +24,48 @@ namespace {
 using testing::_;
 using testing::Return;
 
+// Usage (in tests only):
+//
+//   TestEvent<ResType> e;
+//   ... Do some async work passing e.cb() as a completion callback of
+//   base::OnceCallback<void(ResType* res)> type which also may perform some
+//   other action specified by |done| callback provided by the caller.
+//   ... = e.result();  // Will wait for e.cb() to be called and return the
+//   collected result.
+//
+template <typename ResType>
+class TestEvent {
+ public:
+  TestEvent() : run_loop_(std::make_unique<base::RunLoop>()) {}
+  ~TestEvent() = default;
+  TestEvent(const TestEvent& other) = delete;
+  TestEvent& operator=(const TestEvent& other) = delete;
+  ResType result() {
+    run_loop_->Run();
+    return std::forward<ResType>(result_);
+  }
+
+  // Completion callback to hand over to the processing method.
+  base::OnceCallback<void(ResType res)> cb() {
+    return base::BindOnce(
+        [](base::RunLoop* run_loop, ResType* result, ResType res) {
+          *result = std::forward<ResType>(res);
+          run_loop->Quit();
+        },
+        base::Unretained(run_loop_.get()), base::Unretained(&result_));
+  }
+
+ private:
+  std::unique_ptr<base::RunLoop> run_loop_;
+  ResType result_;
+};
+
 // Ensures that profile cannot be null.
 TEST(DmServerUploadServiceTest, DeniesNullptrProfile) {
-  auto result =
-      DmServerUploadService::Create(/*profile=*/nullptr, base::DoNothing());
+  content::BrowserTaskEnvironment task_envrionment;
+  TestEvent<StatusOr<std::unique_ptr<DmServerUploadService>>> e;
+  DmServerUploadService::Create(/*profile=*/nullptr, base::DoNothing(), e.cb());
+  StatusOr<std::unique_ptr<DmServerUploadService>> result = e.result();
   EXPECT_FALSE(result.ok());
   EXPECT_EQ(result.status().error_code(), error::INVALID_ARGUMENT);
 }
@@ -109,7 +147,7 @@ class DmServerUploaderTest : public testing::Test {
   }
 
  protected:
-  base::test::TaskEnvironment task_envrionment_{
+  content::BrowserTaskEnvironment task_envrionment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
 
   TestRecordHandler* handler_;

chrome/browser/policy/messaging_layer/upload/upload_client.cc

@@ -17,15 +17,28 @@
 namespace reporting {
 
 // static
-StatusOr<std::unique_ptr<UploadClient>> UploadClient::Create(
+void UploadClient::Create(
     std::unique_ptr<policy::CloudPolicyClient> cloud_policy_client,
-    ReportSuccessfulUploadCallback report_success_cb) {
+    ReportSuccessfulUploadCallback report_upload_success_cb,
+    base::OnceCallback<void(StatusOr<std::unique_ptr<UploadClient>>)>
+        created_cb) {
   auto upload_client = base::WrapUnique(new UploadClient());
-  ASSIGN_OR_RETURN(upload_client->dm_server_upload_service_,
-                   DmServerUploadService::Create(std::move(cloud_policy_client),
-                                                 report_success_cb));
-
-  return upload_client;
+  DmServerUploadService::Create(
+      std::move(cloud_policy_client), report_upload_success_cb,
+      base::BindOnce(
+          [](std::unique_ptr<UploadClient> upload_client,
+             base::OnceCallback<void(StatusOr<std::unique_ptr<UploadClient>>)>
+                 created_cb,
+             StatusOr<std::unique_ptr<DmServerUploadService>> uploader) {
+            if (!uploader.ok()) {
+              std::move(created_cb).Run(uploader.status());
+              return;
+            }
+            upload_client->dm_server_upload_service_ =
+                std::move(uploader.ValueOrDie());
+            std::move(created_cb).Run(std::move(upload_client));
+          },
+          std::move(upload_client), std::move(created_cb)));
 }
 
 Status UploadClient::EnqueueUpload(

chrome/browser/policy/messaging_layer/upload/upload_client.h

@@ -26,9 +26,11 @@ class UploadClient {
   using ReportSuccessfulUploadCallback =
       base::RepeatingCallback<void(SequencingInformation)>;
 
-  static StatusOr<std::unique_ptr<UploadClient>> Create(
+  static void Create(
       std::unique_ptr<policy::CloudPolicyClient> cloud_policy_client,
-      ReportSuccessfulUploadCallback report_success_cb);
+      ReportSuccessfulUploadCallback report_upload_success_cb,
+      base::OnceCallback<void(StatusOr<std::unique_ptr<UploadClient>>)>
+          created_cb);
 
   ~UploadClient();
   UploadClient(const UploadClient& other) = delete;

chrome/browser/policy/messaging_layer/upload/upload_client_unittest.cc

@@ -16,6 +16,8 @@
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
 #include "components/policy/proto/record.pb.h"
 #include "components/policy/proto/record_constants.pb.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/browser_task_environment.h"
 #include "services/network/test/test_network_connection_tracker.h"
 
@@ -34,6 +36,42 @@ using testing::Invoke;
 using testing::InvokeArgument;
 using testing::WithArgs;
 
+// Usage (in tests only):
+//
+//   TestEvent<ResType> e;
+//   ... Do some async work passing e.cb() as a completion callback of
+//   base::OnceCallback<void(ResType* res)> type which also may perform some
+//   other action specified by |done| callback provided by the caller.
+//   ... = e.result();  // Will wait for e.cb() to be called and return the
+//   collected result.
+//
+template <typename ResType>
+class TestEvent {
+ public:
+  TestEvent() : run_loop_(std::make_unique<base::RunLoop>()) {}
+  ~TestEvent() = default;
+  TestEvent(const TestEvent& other) = delete;
+  TestEvent& operator=(const TestEvent& other) = delete;
+  ResType result() {
+    run_loop_->Run();
+    return std::forward<ResType>(result_);
+  }
+
+  // Completion callback to hand over to the processing method.
+  base::OnceCallback<void(ResType res)> cb() {
+    return base::BindOnce(
+        [](base::RunLoop* run_loop, ResType* result, ResType res) {
+          *result = std::forward<ResType>(res);
+          run_loop->Quit();
+        },
+        base::Unretained(run_loop_.get()), base::Unretained(&result_));
+  }
+
+ private:
+  std::unique_ptr<base::RunLoop> run_loop_;
+  ResType result_;
+};
+
 class TestCallbackWaiter {
  public:
   TestCallbackWaiter() : run_loop_(std::make_unique<base::RunLoop>()) {}
@@ -62,26 +100,45 @@ class TestCallbackWaiterWithCounter : public TestCallbackWaiter {
   std::atomic<int> counter_limit_;
 };
 
-TEST(UploadClientTest, CreateUploadClient) {
-  content::BrowserTaskEnvironment task_envrionment_;
+class UploadClientTest : public ::testing::Test {
+ public:
+  UploadClientTest() = default;
 
+ protected:
+  void SetUp() override {
 #ifdef OS_CHROMEOS
     // Set up fake primary profile.
     auto mock_user_manager =
         std::make_unique<testing::NiceMock<chromeos::FakeChromeUserManager>>();
-  auto profile = std::make_unique<TestingProfile>(
+    profile_ = std::make_unique<TestingProfile>(
         base::FilePath(FILE_PATH_LITERAL("/home/chronos/u-0123456789abcdef")));
-  const AccountId account_id(
-      AccountId::FromUserEmailGaiaId(profile->GetProfileUserName(), "12345"));
+    const AccountId account_id(AccountId::FromUserEmailGaiaId(
+        profile_->GetProfileUserName(), "12345"));
     const user_manager::User* user =
         mock_user_manager->AddPublicAccountUser(account_id);
     mock_user_manager->UserLoggedIn(account_id, user->username_hash(),
                                     /*browser_restart=*/false,
                                     /*is_child=*/false);
-  auto user_manager = std::make_unique<user_manager::ScopedUserManager>(
+    user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
         std::move(mock_user_manager));
 #endif  // OS_CHROMEOS
+  }
+
+  void TearDown() override {
+#ifdef OS_CHROMEOS
+    user_manager_.reset();
+    profile_.reset();
+#endif  // OS_CHROMEOS
+  }
 
+  content::BrowserTaskEnvironment task_envrionment_;
+#ifdef OS_CHROMEOS
+  std::unique_ptr<TestingProfile> profile_;
+  std::unique_ptr<user_manager::ScopedUserManager> user_manager_;
+#endif  // OS_CHROMEOS
+};
+
+TEST_F(UploadClientTest, CreateUploadClient) {
   const int kExpectedCallTimes = 10;
   const uint64_t kGenerationId = 1234;
 
@@ -98,9 +155,9 @@ TEST(UploadClientTest, CreateUploadClient) {
             waiter.Signal();
           })));
 
-  auto upload_client_result =
-      UploadClient::Create(std::move(client), base::DoNothing());
-
+  TestEvent<StatusOr<std::unique_ptr<UploadClient>>> e;
+  UploadClient::Create(std::move(client), base::DoNothing(), e.cb());
+  StatusOr<std::unique_ptr<UploadClient>> upload_client_result = e.result();
   ASSERT_OK(upload_client_result) << upload_client_result.status();
 
   base::Value data{base::Value::Type::DICTIONARY};