Commit b28edfcf authored by Mihai Sardarescu's avatar Mihai Sardarescu Committed by Commit Bot

Reland "Use EarlGrey wait condition instead of spinning the runloop."

This reverts commit 9b7b1b3a.

Reason for revert: Fixed downstream compilation

Original change's description:
> Revert "Use EarlGrey wait condition instead of spinning the runloop."
> 
> This reverts commit 7a4958d3.
> 
> Reason for revert: Downstream code is using signOutAndClearAccounts
> 
> Original change's description:
> > 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/+/2039551
> > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> > Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#738916}
> 
> TBR=msarda@chromium.org,sdefresne@chromium.org
> 
> Change-Id: I87b517b4669f02af19c516997eba465f51eb0c22
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1031986
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041753
> Reviewed-by: Gauthier Ambard <gambard@chromium.org>
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#738934}

TBR=msarda@chromium.org,sdefresne@chromium.org,gambard@chromium.org

Change-Id: I5be4d0bad2ac69aade4f9707a55303608bfa3644
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1031986
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041598Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738940}
parent e6adc463
...@@ -21,9 +21,12 @@ void SetUpMockAccountReconcilor(); ...@@ -21,9 +21,12 @@ void SetUpMockAccountReconcilor();
// Tears down the mock AccountReconcilor if it was previously set up. // Tears down the mock AccountReconcilor if it was previously set up.
void TearDownMockAccountReconcilor(); void TearDownMockAccountReconcilor();
// Signs the user out and clears the known accounts entirely. Returns whether // Signs the user out and starts clearing all identities from the
// the accounts were correctly removed from the keychain. // ChromeIdentityService.
bool SignOutAndClearAccounts(); void SignOutAndClearIdentities();
// Returns true when there are no identities in the ChromeIdentityService.
bool HasIdentities();
// Resets mock authentication. // Resets mock authentication.
void ResetMockAuthentication(); void ResetMockAuthentication();
......
...@@ -29,39 +29,23 @@ namespace chrome_test_util { ...@@ -29,39 +29,23 @@ namespace chrome_test_util {
namespace { namespace {
// Forgets all identities from the ChromeIdentity services. Returns true if all // Starts forgetting all identities from the ChromeIdentity services.
// identities were successfully forgotten. //
bool ForgetAllIdentities() { // 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::ChromeIdentityService* identity_service =
ios::GetChromeBrowserProvider()->GetChromeIdentityService(); ios::GetChromeBrowserProvider()->GetChromeIdentityService();
if (!identity_service->HasIdentities())
return YES;
NSArray* identities_to_remove = NSArray* identities_to_remove =
[NSArray arrayWithArray:identity_service->GetAllIdentities()]; [NSArray arrayWithArray:identity_service->GetAllIdentities()];
NSMutableArray* identities_left =
[NSMutableArray arrayWithArray:identities_to_remove];
for (ChromeIdentity* identity in identities_to_remove) { for (ChromeIdentity* identity in identities_to_remove) {
identity_service->ForgetIdentity(identity, ^(NSError* error) { identity_service->ForgetIdentity(identity, ^(NSError* error) {
if (error) { if (error) {
NSLog(@"ForgetIdentity failed: [identity = %@, error = %@]", NSLog(@"ForgetIdentity failed: [identity = %@, error = %@]",
identity.userEmail, [error localizedDescription]); 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 } // namespace
...@@ -91,7 +75,7 @@ void TearDownMockAccountReconcilor() { ...@@ -91,7 +75,7 @@ void TearDownMockAccountReconcilor() {
GaiaAuthFetcherIOS::SetShouldUseGaiaAuthFetcherIOSForTesting(true); GaiaAuthFetcherIOS::SetShouldUseGaiaAuthFetcherIOSForTesting(true);
} }
bool SignOutAndClearAccounts() { void SignOutAndClearIdentities() {
// EarlGrey monitors network requests by swizzling internal iOS network // EarlGrey monitors network requests by swizzling internal iOS network
// objects and expects them to be dealloced before the tear down. It is // objects and expects them to be dealloced before the tear down. It is
// important to autorelease all objects that make network requests to avoid // important to autorelease all objects that make network requests to avoid
...@@ -112,15 +96,23 @@ bool SignOutAndClearAccounts() { ...@@ -112,15 +96,23 @@ bool SignOutAndClearAccounts() {
browser_state->GetPrefs()->ClearPref(prefs::kGoogleServicesLastAccountId); browser_state->GetPrefs()->ClearPref(prefs::kGoogleServicesLastAccountId);
browser_state->GetPrefs()->ClearPref(prefs::kGoogleServicesLastUsername); 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 // changes to ensure that clearing the last signed in account is saved on
// disk in case Chrome crashes during shutdown. // disk in case Chrome crashes during shutdown.
browser_state->GetPrefs()->CommitPendingWrite(); 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() { void ResetMockAuthentication() {
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider() ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
->SetFakeMDMError(false); ->SetFakeMDMError(false);
......
...@@ -315,6 +315,11 @@ id ExecuteJavaScript(NSString* javascript, NSError* __autoreleasing* out_error); ...@@ -315,6 +315,11 @@ id ExecuteJavaScript(NSString* javascript, NSError* __autoreleasing* out_error);
// Signs the user out, clears the known accounts entirely and checks whether the // Signs the user out, clears the known accounts entirely and checks whether the
// accounts were correctly removed from the keychain. Induces a GREYAssert if // accounts were correctly removed from the keychain. Induces a GREYAssert if
// the operation fails. // the operation fails.
- (void)signOutAndClearIdentities;
// Same as signOutAndClearIdentities.
//
// DEPRECATED in favor of signOutAndClearIdentities
- (void)signOutAndClearAccounts; - (void)signOutAndClearAccounts;
#pragma mark - Sync Utilities (EG2) #pragma mark - Sync Utilities (EG2)
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#import "ios/web/public/web_state.h" // nogncheck #import "ios/web/public/web_state.h" // nogncheck
#endif #endif
using base::test::ios::kWaitForActionTimeout;
using base::test::ios::kWaitForJSCompletionTimeout; using base::test::ios::kWaitForJSCompletionTimeout;
using base::test::ios::kWaitForPageLoadTimeout; using base::test::ios::kWaitForPageLoadTimeout;
using base::test::ios::kWaitForUIElementTimeout; using base::test::ios::kWaitForUIElementTimeout;
...@@ -735,9 +736,21 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeEarlGreyAppInterface) ...@@ -735,9 +736,21 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeEarlGreyAppInterface)
#pragma mark - SignIn Utilities (EG2) #pragma mark - SignIn Utilities (EG2)
- (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");
}
- (void)signOutAndClearAccounts { - (void)signOutAndClearAccounts {
EG_TEST_HELPER_ASSERT_NO_ERROR( [self signOutAndClearIdentities];
[ChromeEarlGreyAppInterface signOutAndClearAccounts]);
} }
#pragma mark - Bookmarks Utilities (EG2) #pragma mark - Bookmarks Utilities (EG2)
......
...@@ -196,10 +196,15 @@ ...@@ -196,10 +196,15 @@
// Sets value for content setting. // Sets value for content setting.
+ (void)setContentSettings:(ContentSetting)setting; + (void)setContentSettings:(ContentSetting)setting;
// Signs the user out, clears the known accounts entirely and checks whether // Signs the user out from Chrome and then starts clearing the identities.
// the accounts were correctly removed from the keychain. Returns nil on //
// success, or else an NSError indicating why the operation failed. // Note: This method does not wait for identities to be cleared from the
+ (NSError*)signOutAndClearAccounts; // 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. // Returns the current WebState's VisibleURL.
+ (NSString*)webStateVisibleURL; + (NSString*)webStateVisibleURL;
......
...@@ -364,13 +364,12 @@ using chrome_test_util::BrowserCommandDispatcherForMainBVC; ...@@ -364,13 +364,12 @@ using chrome_test_util::BrowserCommandDispatcherForMainBVC;
chrome_test_util::SetContentSettingsBlockPopups(setting); chrome_test_util::SetContentSettingsBlockPopups(setting);
} }
+ (NSError*)signOutAndClearAccounts { + (void)signOutAndClearIdentities {
bool success = chrome_test_util::SignOutAndClearAccounts(); chrome_test_util::SignOutAndClearIdentities();
if (!success) { }
return testing::NSErrorWithLocalizedDescription(
@"Real accounts couldn't be cleared."); + (BOOL)hasIdentities {
} return chrome_test_util::HasIdentities();
return nil;
} }
+ (NSString*)webStateVisibleURL { + (NSString*)webStateVisibleURL {
......
...@@ -234,7 +234,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) ...@@ -234,7 +234,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
ResetAuthentication(); ResetAuthentication();
// Reset any remaining sign-in state from previous tests. // Reset any remaining sign-in state from previous tests.
[ChromeEarlGrey signOutAndClearAccounts]; [ChromeEarlGrey signOutAndClearIdentities];
[ChromeEarlGrey openNewTab]; [ChromeEarlGrey openNewTab];
_executedTestMethodSetUp = YES; _executedTestMethodSetUp = YES;
} }
...@@ -250,7 +250,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) ...@@ -250,7 +250,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
} }
// Clear any remaining test accounts and signed in users. // 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 // Re-start anything that was disabled this test, so it is running when the
// next test starts. // next test starts.
...@@ -327,7 +327,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) ...@@ -327,7 +327,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
+ (void)disableMockAuthentication { + (void)disableMockAuthentication {
// Make sure local data is cleared, before disabling mock authentication, // Make sure local data is cleared, before disabling mock authentication,
// where data may be sent to real servers. // where data may be sent to real servers.
[ChromeEarlGrey signOutAndClearAccounts]; [ChromeEarlGrey signOutAndClearIdentities];
[ChromeEarlGrey tearDownFakeSyncServer]; [ChromeEarlGrey tearDownFakeSyncServer];
TearDownMockAuthentication(); TearDownMockAuthentication();
} }
...@@ -458,7 +458,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface) ...@@ -458,7 +458,7 @@ GREY_STUB_CLASS_IN_APP_MAIN_QUEUE(ChromeTestCaseAppInterface)
ResetAuthentication(); ResetAuthentication();
// Reset any remaining sign-in state from previous tests. // Reset any remaining sign-in state from previous tests.
[ChromeEarlGrey signOutAndClearAccounts]; [ChromeEarlGrey signOutAndClearIdentities];
[ChromeEarlGrey openNewTab]; [ChromeEarlGrey openNewTab];
} }
} }
......
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