Use EarlGrey wait condition instead of spinning the runloop.

The sign-in external URL tests were spinning the runloop while waiting
for the identities to be removed from the ChromeIdentityService. This
was consistently failing with EarlGrey2.

This CL uses an EarlGrey wait condition instead of spinning the runloop
while waiting for the identities to be removed.

Bug: 1031986

Change-Id: I0bcf6bda55caa506fe3a4d54ee607efc5c2e4d37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2039551Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738916}

ios/chrome/test/app/signin_test_util.h

@@ -21,9 +21,12 @@ void SetUpMockAccountReconcilor();
 // Tears down the mock AccountReconcilor if it was previously set up.
 void TearDownMockAccountReconcilor();
 
-// Signs the user out and clears the known accounts entirely. Returns whether
-// the accounts were correctly removed from the keychain.
-bool SignOutAndClearAccounts();
+// Signs the user out and starts clearing all identities from the
+// ChromeIdentityService.
+void SignOutAndClearIdentities();
+
+// Returns true when there are no identities in the ChromeIdentityService.
+bool HasIdentities();
 
 // Resets mock authentication.
 void ResetMockAuthentication();

ios/chrome/test/app/signin_test_util.mm

@@ -29,39 +29,23 @@ namespace chrome_test_util {
 
 namespace {
 
-// Forgets all identities from the ChromeIdentity services. Returns true if all
-// identities were successfully forgotten.
-bool ForgetAllIdentities() {
+// Starts forgetting all identities from the ChromeIdentity services.
+//
+// Note: Forgetting an identity is a asynchronous operation. This function does
+// not wait for the forget identity operation to finish.
+void StartForgetAllIdentities() {
   ios::ChromeIdentityService* identity_service =
       ios::GetChromeBrowserProvider()->GetChromeIdentityService();
-
-  if (!identity_service->HasIdentities())
-    return YES;
-
   NSArray* identities_to_remove =
       [NSArray arrayWithArray:identity_service->GetAllIdentities()];
-  NSMutableArray* identities_left =
-      [NSMutableArray arrayWithArray:identities_to_remove];
-
   for (ChromeIdentity* identity in identities_to_remove) {
     identity_service->ForgetIdentity(identity, ^(NSError* error) {
       if (error) {
         NSLog(@"ForgetIdentity failed: [identity = %@, error = %@]",
               identity.userEmail, [error localizedDescription]);
       }
-      [identities_left removeObject:identity];
     });
   }
-
-  // Wait maximum 10s for all forget identitites to be forgotten.
-  NSDate* deadline = [NSDate dateWithTimeIntervalSinceNow:10.0];
-  while ([identities_left count] &&
-         [[NSDate date] compare:deadline] != NSOrderedDescending) {
-    base::test::ios::SpinRunLoopWithMaxDelay(
-        base::TimeDelta::FromSecondsD(0.01));
-  }
-
-  return !identity_service->HasIdentities();
 }
 
 }  // namespace
@@ -91,7 +75,7 @@ void TearDownMockAccountReconcilor() {
   GaiaAuthFetcherIOS::SetShouldUseGaiaAuthFetcherIOSForTesting(true);
 }
 
-bool SignOutAndClearAccounts() {
+void SignOutAndClearIdentities() {
   // EarlGrey monitors network requests by swizzling internal iOS network
   // objects and expects them to be dealloced before the tear down. It is
   // important to autorelease all objects that make network requests to avoid
@@ -112,15 +96,23 @@ bool SignOutAndClearAccounts() {
     browser_state->GetPrefs()->ClearPref(prefs::kGoogleServicesLastAccountId);
     browser_state->GetPrefs()->ClearPref(prefs::kGoogleServicesLastUsername);
 
-    // |SignOutAndClearAccounts()| is called during shutdown. Commit all pref
+    // |SignOutAndClearIdentities()| is called during shutdown. Commit all pref
     // changes to ensure that clearing the last signed in account is saved on
     // disk in case Chrome crashes during shutdown.
     browser_state->GetPrefs()->CommitPendingWrite();
 
-    return ForgetAllIdentities();
+    // Once the browser was signed out, start clearing all identities from the
+    // ChromeIdentityService.
+    StartForgetAllIdentities();
   }
 }
 
+bool HasIdentities() {
+  ios::ChromeIdentityService* identity_service =
+      ios::GetChromeBrowserProvider()->GetChromeIdentityService();
+  return identity_service->HasIdentities();
+}
+
 void ResetMockAuthentication() {
   ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
       ->SetFakeMDMError(false);

ios/chrome/test/earl_grey/chrome_earl_grey.h

@@ -315,7 +315,7 @@ id ExecuteJavaScript(NSString* javascript, NSError* __autoreleasing* out_error);
 // Signs the user out, clears the known accounts entirely and checks whether the
 // accounts were correctly removed from the keychain. Induces a GREYAssert if
 // the operation fails.
-- (void)signOutAndClearAccounts;
+- (void)signOutAndClearIdentities;
 
 #pragma mark - Sync Utilities (EG2)
 

ios/chrome/test/earl_grey/chrome_earl_grey.mm

@@ -32,6 +32,7 @@
 #import "ios/web/public/web_state.h"                             // nogncheck
 #endif
 
+using base::test::ios::kWaitForActionTimeout;
 using base::test::ios::kWaitForJSCompletionTimeout;
 using base::test::ios::kWaitForPageLoadTimeout;
 using base::test::ios::kWaitForUIElementTimeout;
@@ -735,9 +736,17 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeEarlGreyAppInterface)
 
 #pragma mark - SignIn Utilities (EG2)
 
-- (void)signOutAndClearAccounts {
-  EG_TEST_HELPER_ASSERT_NO_ERROR(
-      [ChromeEarlGreyAppInterface signOutAndClearAccounts]);
+- (void)signOutAndClearIdentities {
+  [ChromeEarlGreyAppInterface signOutAndClearIdentities];
+
+  GREYCondition* allIdentitiesCleared = [GREYCondition
+      conditionWithName:@"All Chrome identities were cleared"
+                  block:^{
+                    return ![ChromeEarlGreyAppInterface hasIdentities];
+                  }];
+  bool success = [allIdentitiesCleared waitWithTimeout:kWaitForActionTimeout];
+  EG_TEST_HELPER_ASSERT_TRUE(success,
+                             @"Failed waiting for identities to be cleared");
 }
 
 #pragma mark - Bookmarks Utilities (EG2)

ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.h

@@ -196,10 +196,15 @@
 // Sets value for content setting.
 + (void)setContentSettings:(ContentSetting)setting;
 
-// Signs the user out, clears the known accounts entirely and checks whether
-// the accounts were correctly removed from the keychain. Returns nil on
-// success, or else an NSError indicating why the operation failed.
-+ (NSError*)signOutAndClearAccounts;
+// Signs the user out from Chrome and then starts clearing the identities.
+//
+// Note: This method does not wait for identities to be cleared from the
+// keychain. To wait for this operation to finish, please use an GREYCondition
+// and wait for +hasIdentities to return NO.
++ (void)signOutAndClearIdentities;
+
+// Returns YES if there is at at least identity in the ChromeIdentityService.
++ (BOOL)hasIdentities;
 
 // Returns the current WebState's VisibleURL.
 + (NSString*)webStateVisibleURL;

ios/chrome/test/earl_grey/chrome_earl_grey_app_interface.mm

@@ -364,13 +364,12 @@ using chrome_test_util::BrowserCommandDispatcherForMainBVC;
   chrome_test_util::SetContentSettingsBlockPopups(setting);
 }
 
-+ (NSError*)signOutAndClearAccounts {
-  bool success = chrome_test_util::SignOutAndClearAccounts();
-  if (!success) {
-    return testing::NSErrorWithLocalizedDescription(
-        @"Real accounts couldn't be cleared.");
-  }
-  return nil;
++ (void)signOutAndClearIdentities {
+  chrome_test_util::SignOutAndClearIdentities();
+}
+
++ (BOOL)hasIdentities {
+  return chrome_test_util::HasIdentities();
 }
 
 + (NSString*)webStateVisibleURL {

ios/chrome/test/earl_grey/chrome_test_case.mm

@@ -234,7 +234,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
   ResetAuthentication();
 
   // Reset any remaining sign-in state from previous tests.
-  [ChromeEarlGrey signOutAndClearAccounts];
+  [ChromeEarlGrey signOutAndClearIdentities];
   [ChromeEarlGrey openNewTab];
   _executedTestMethodSetUp = YES;
 }
@@ -250,7 +250,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
   }
 
   // Clear any remaining test accounts and signed in users.
-  [ChromeEarlGrey signOutAndClearAccounts];
+  [ChromeEarlGrey signOutAndClearIdentities];
 
   // Re-start anything that was disabled this test, so it is running when the
   // next test starts.
@@ -327,7 +327,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
 + (void)disableMockAuthentication {
   // Make sure local data is cleared, before disabling mock authentication,
   // where data may be sent to real servers.
-  [ChromeEarlGrey signOutAndClearAccounts];
+  [ChromeEarlGrey signOutAndClearIdentities];
   [ChromeEarlGrey tearDownFakeSyncServer];
   TearDownMockAuthentication();
 }
@@ -458,7 +458,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
       ResetAuthentication();
 
       // Reset any remaining sign-in state from previous tests.
-      [ChromeEarlGrey signOutAndClearAccounts];
+      [ChromeEarlGrey signOutAndClearIdentities];
       [ChromeEarlGrey openNewTab];
     }
   }