Replace RunUntilIdle() with runloop.Run()/Quit()

This CL replaces all occurrences of base::RunLoop().RunUntilIdle() in
UserCloudPolicyStoreChromeOSTest with run_loop.Run() and an associated
Quit() call from the expected callback.

Bug: none
Test: unit_tests
Change-Id: I9f8ec70381c44bb034cc19e0622b3b7cb090539a
Reviewed-on: https://chromium-review.googlesource.com/1160239
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582221}

chrome/browser/chromeos/policy/user_cloud_policy_store_chromeos_unittest.cc

@@ -42,6 +42,7 @@ using RetrievePolicyResponseType =
 using testing::_;
 using testing::AllOf;
 using testing::Eq;
+using testing::InvokeWithoutArgs;
 using testing::Mock;
 using testing::Property;
 using testing::Return;
@@ -147,15 +148,32 @@ class UserCloudPolicyStoreChromeOSTest : public testing::Test {
   void TearDown() override {
     store_->RemoveObserver(&observer_);
     store_.reset();
-    base::RunLoop().RunUntilIdle();
   }
 
   // Install an expectation on |observer_| for an error code.
-  void ExpectError(CloudPolicyStore::Status error) {
-    EXPECT_CALL(
-        observer_,
-        OnStoreError(AllOf(Eq(store_.get()),
-                           Property(&CloudPolicyStore::status, Eq(error)))));
+  // This method should be called after a store->Store()/Load() call has been
+  // initiated. The expected OnStoreError call will be initiated asynchronously
+  // from this run_loop's iteration.
+  void RunLoopAndExpectError(CloudPolicyStore::Status error) {
+    base::RunLoop run_loop;
+    EXPECT_CALL(observer_, OnStoreError(AllOf(
+                               Eq(store_.get()),
+                               Property(&CloudPolicyStore::status, Eq(error)))))
+        .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+    run_loop.Run();
+    Mock::VerifyAndClearExpectations(&observer_);
+  }
+
+  // Install an expectation on |observer_| for a successful load operation.
+  // This method should be called after a store->Store()/Load() call has been
+  // initiated. The expected OnStoreLoaded call will be initiated asynchronously
+  // from this run_loop's iteration.
+  void RunLoopAndExpectLoaded() {
+    base::RunLoop run_loop;
+    EXPECT_CALL(observer_, OnStoreLoaded(store_.get()))
+        .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+    run_loop.Run();
+    Mock::VerifyAndClearExpectations(&observer_);
   }
 
   // Triggers a store_->Load() operation, handles the expected call to
@@ -164,7 +182,6 @@ class UserCloudPolicyStoreChromeOSTest : public testing::Test {
     // Issue a load command.
     session_manager_client_->set_user_policy(cryptohome_id_, response);
     store_->Load();
-    base::RunLoop().RunUntilIdle();
   }
 
   // Verifies that store_->policy_map() has the HomepageLocation entry with
@@ -199,8 +216,7 @@ class UserCloudPolicyStoreChromeOSTest : public testing::Test {
     EXPECT_TRUE(previous_policy.Equals(store_->policy_map()));
     EXPECT_EQ(initial_status, store_->status());
 
-    EXPECT_CALL(observer_, OnStoreLoaded(store_.get()));
-    base::RunLoop().RunUntilIdle();
+    RunLoopAndExpectLoaded();
     ASSERT_TRUE(store_->policy());
     EXPECT_EQ(policy_.policy_data().SerializeAsString(),
               store_->policy()->SerializeAsString());
@@ -263,7 +279,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, InitialStoreValidationFail) {
       "garbage";
 
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -277,7 +294,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, InitialStoreMissingSignatureFailure) {
   policy_.policy().clear_new_public_key_verification_signature_deprecated();
 
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -307,7 +325,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest,
   policy_.policy().clear_new_public_key_verification_signature_deprecated();
 
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -320,7 +339,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreWithRotationValidationError) {
       "garbage";
 
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -329,9 +349,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreFail) {
   // Let store policy fail.
   session_manager_client_->set_store_policy_success(false);
 
-  ExpectError(CloudPolicyStore::STATUS_STORE_ERROR);
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_STORE_ERROR);
 
   EXPECT_FALSE(store_->policy());
   EXPECT_TRUE(store_->policy_map().empty());
@@ -343,11 +362,9 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreValidationError) {
   policy_.policy_data().clear_policy_type();
   policy_.Build();
 
-  // Store policy.
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
-
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -358,10 +375,9 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreValueValidationError) {
   policy_.payload().mutable_opennetworkconfiguration()->set_value(onc_policy);
   policy_.Build();
 
-  EXPECT_CALL(observer_, OnStoreLoaded(store_.get()));
-
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectLoaded();
+
   const CloudPolicyValidatorBase::ValidationResult* validation_result =
       store_->validation_result();
   EXPECT_EQ(CloudPolicyStore::STATUS_OK, store_->status());
@@ -378,10 +394,9 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreWithoutPolicyKey) {
   // Make the dbus call to cryptohome fail.
   cryptohome_client_.SetServiceIsAvailable(false);
 
-  // Store policy.
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -390,10 +405,9 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, StoreWithInvalidSignature) {
   // Break the signature.
   policy_.policy().mutable_policy_data_signature()->append("garbage");
 
-  // Store policy.
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   EXPECT_EQ(CloudPolicyStore::STATUS_VALIDATION_ERROR, store_->status());
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -408,9 +422,11 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, MultipleStoresWithRotation) {
   policy_.SetDefaultNewSigningKey();
   policy_.policy_data().clear_policy_type();
   policy_.Build();
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
+  // Store policy
   store_->Store(policy_.policy());
-  base::RunLoop().RunUntilIdle();
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
+
   // Still the initial public key is exposed.
   EXPECT_EQ(initial_public_key, store_->policy_signature_public_key());
 
@@ -427,9 +443,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, MultipleStoresWithRotation) {
 }
 
 TEST_F(UserCloudPolicyStoreChromeOSTest, Load) {
-  EXPECT_CALL(observer_, OnStoreLoaded(store_.get()));
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad(policy_.GetBlob()));
-  Mock::VerifyAndClearExpectations(&observer_);
+  RunLoopAndExpectLoaded();
 
   // Verify that the policy has been loaded.
   ASSERT_TRUE(store_->policy());
@@ -442,9 +457,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, Load) {
 }
 
 TEST_F(UserCloudPolicyStoreChromeOSTest, LoadNoPolicy) {
-  EXPECT_CALL(observer_, OnStoreLoaded(store_.get()));
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad(""));
-  Mock::VerifyAndClearExpectations(&observer_);
+  RunLoopAndExpectLoaded();
 
   // Verify no policy has been installed.
   EXPECT_FALSE(store_->policy());
@@ -454,8 +468,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, LoadNoPolicy) {
 }
 
 TEST_F(UserCloudPolicyStoreChromeOSTest, LoadInvalidPolicy) {
-  ExpectError(CloudPolicyStore::STATUS_PARSE_ERROR);
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad("invalid"));
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_PARSE_ERROR);
 
   // Verify no policy has been installed.
   EXPECT_FALSE(store_->policy());
@@ -468,8 +482,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, LoadValidationError) {
   policy_.policy_data().clear_policy_type();
   policy_.Build();
 
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad(policy_.GetBlob()));
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   VerifyStoreHasValidationError();
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -477,8 +491,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, LoadValidationError) {
 TEST_F(UserCloudPolicyStoreChromeOSTest, LoadNoKey) {
   // The loaded policy can't be verified without the public key.
   ASSERT_TRUE(base::DeleteFile(user_policy_key_file(), false));
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad(policy_.GetBlob()));
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   VerifyStoreHasValidationError();
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }
@@ -486,8 +500,8 @@ TEST_F(UserCloudPolicyStoreChromeOSTest, LoadNoKey) {
 TEST_F(UserCloudPolicyStoreChromeOSTest, LoadInvalidSignature) {
   // Break the signature.
   policy_.policy().mutable_policy_data_signature()->append("garbage");
-  ExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   ASSERT_NO_FATAL_FAILURE(PerformPolicyLoad(policy_.GetBlob()));
+  RunLoopAndExpectError(CloudPolicyStore::STATUS_VALIDATION_ERROR);
   VerifyStoreHasValidationError();
   EXPECT_EQ(std::string(), store_->policy_signature_public_key());
 }