Commit 49cc8b7b authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Commit Bot

[iOS] Fix EarlGrey deadlock due to unfinished animation.

Currently we only stop the activity indicator spinning animation when
the user presses 'Cancel' on the user sign-in screen. This otherwise
infinite animation causes deadlocks in EarlGrey during the
authentication flow.

We move all activity indicator handling to the user sign-in delegate.

Bug: 1005509
Change-Id: Ia775ab32346d8b67ef4842d5952a250d3208a11c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144014Reviewed-by: default avatarJérôme Lebel <jlebel@chromium.org>
Reviewed-by: default avatarNohemi Fernandez <fernandex@chromium.org>
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760459}
parent 40f846f4
...@@ -16,6 +16,17 @@ class Browser; ...@@ -16,6 +16,17 @@ class Browser;
@class ChromeIdentity; @class ChromeIdentity;
@class UIViewController; @class UIViewController;
// Handles completion of AuthenticationFlow operations.
@protocol AuthenticationFlowDelegate <NSObject>
// Indicates that a user dialog is presented from the authentication flow.
- (void)didPresentDialog;
// Indicates that a user dialog is dismissed from the authentication flow.
- (void)didDismissDialog;
@end
// |AuthenticationFlow| manages the authentication flow for a given identity. // |AuthenticationFlow| manages the authentication flow for a given identity.
// //
// A new instance of |AuthenticationFlow| should be used each time an identity // A new instance of |AuthenticationFlow| should be used each time an identity
...@@ -59,6 +70,9 @@ class Browser; ...@@ -59,6 +70,9 @@ class Browser;
// The dispatcher used to clear browsing data. // The dispatcher used to clear browsing data.
@property(nonatomic, weak) id<BrowsingDataCommands> dispatcher; @property(nonatomic, weak) id<BrowsingDataCommands> dispatcher;
// The delegate.
@property(nonatomic, weak) id<AuthenticationFlowDelegate> delegate;
@end @end
// Private methods in AuthenticationFlow to test. // Private methods in AuthenticationFlow to test.
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#import "ios/chrome/browser/ui/authentication/authentication_flow.h" #import "ios/chrome/browser/ui/authentication/authentication_flow.h"
#import "base/ios/block_types.h"
#include "base/logging.h" #include "base/logging.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h" #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/main/browser.h" #include "ios/chrome/browser/main/browser.h"
...@@ -408,6 +409,7 @@ NSError* IdentityMissingError() { ...@@ -408,6 +409,7 @@ NSError* IdentityMissingError() {
DCHECK_NE(SHOULD_CLEAR_DATA_USER_CHOICE, shouldClearData); DCHECK_NE(SHOULD_CLEAR_DATA_USER_CHOICE, shouldClearData);
_shouldSignOut = YES; _shouldSignOut = YES;
_shouldClearData = shouldClearData; _shouldClearData = shouldClearData;
[self continueSignin]; [self continueSignin];
} }
...@@ -443,8 +445,28 @@ NSError* IdentityMissingError() { ...@@ -443,8 +445,28 @@ NSError* IdentityMissingError() {
[self cancelFlow]; [self cancelFlow];
} }
- (UIViewController*)presentingViewController { - (void)dismissPresentingViewControllerAnimated:(BOOL)animated
return _presentingViewController; completion:(ProceduralBlock)completion {
[_presentingViewController dismissViewControllerAnimated:animated
completion:^() {
[_delegate didDismissDialog];
if (completion) {
completion();
}
}];
}
- (void)presentViewController:(UIViewController*)viewController
animated:(BOOL)animated
completion:(ProceduralBlock)completion {
[_presentingViewController presentViewController:viewController
animated:animated
completion:^() {
[_delegate didPresentDialog];
if (completion) {
completion();
}
}];
} }
#pragma mark - Used for testing #pragma mark - Used for testing
......
...@@ -96,8 +96,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10; ...@@ -96,8 +96,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
if (_navigationController) { if (_navigationController) {
[_navigationController cleanUpSettings]; [_navigationController cleanUpSettings];
_navigationController = nil; _navigationController = nil;
[[_delegate presentingViewController] dismissViewControllerAnimated:NO [_delegate dismissPresentingViewControllerAnimated:NO completion:nil];
completion:nil];
} }
[self stopWatchdogTimer]; [self stopWatchdogTimer];
} }
...@@ -282,10 +281,9 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10; ...@@ -282,10 +281,9 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
fromEmail:lastSignedInEmail fromEmail:lastSignedInEmail
toEmail:[identity userEmail] toEmail:[identity userEmail]
isSignedIn:isSignedIn]; isSignedIn:isSignedIn];
[[_delegate presentingViewController] [_delegate presentViewController:_navigationController
presentViewController:_navigationController animated:YES
animated:YES completion:nil];
completion:nil];
} }
- (void)clearDataFromBrowser:(Browser*)browser - (void)clearDataFromBrowser:(Browser*)browser
...@@ -465,8 +463,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10; ...@@ -465,8 +463,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
[[strongSelf delegate] didChooseClearDataPolicy:shouldClearData]; [[strongSelf delegate] didChooseClearDataPolicy:shouldClearData];
}; };
[_navigationController cleanUpSettings]; [_navigationController cleanUpSettings];
[[_delegate presentingViewController] dismissViewControllerAnimated:YES [_delegate dismissPresentingViewControllerAnimated:YES completion:block];
completion:block];
} }
#pragma mark - SettingsNavigationControllerDelegate #pragma mark - SettingsNavigationControllerDelegate
...@@ -483,8 +480,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10; ...@@ -483,8 +480,7 @@ const int64_t kAuthenticationFlowTimeoutSeconds = 10;
[[strongSelf delegate] didChooseCancel]; [[strongSelf delegate] didChooseCancel];
}; };
[_navigationController cleanUpSettings]; [_navigationController cleanUpSettings];
[[_delegate presentingViewController] dismissViewControllerAnimated:YES [_delegate dismissPresentingViewControllerAnimated:YES completion:block];
completion:block];
} }
- (void)settingsWasDismissed { - (void)settingsWasDismissed {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#import "base/ios/block_types.h"
#include "ios/chrome/browser/signin/constants.h" #include "ios/chrome/browser/signin/constants.h"
@class UIViewController; @class UIViewController;
...@@ -38,8 +39,14 @@ ...@@ -38,8 +39,14 @@
// Indicates that the user cancelled signing in to a managed account. // Indicates that the user cancelled signing in to a managed account.
- (void)didCancelManagedConfirmation; - (void)didCancelManagedConfirmation;
// The view controller that is showing the sign-in flow. // Dismisses the view controller that is showing the sign-in flow.
@property(readonly) UIViewController* presentingViewController; - (void)dismissPresentingViewControllerAnimated:(BOOL)animated
completion:(ProceduralBlock)completion;
// Presents a view controller on the sign-in flow.
- (void)presentViewController:(UIViewController*)viewController
animated:(BOOL)animated
completion:(ProceduralBlock)completion;
@end @end
......
...@@ -494,6 +494,7 @@ const CGFloat kFadeOutAnimationDuration = 0.16f; ...@@ -494,6 +494,7 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
presentingViewController:self.viewController]; presentingViewController:self.viewController];
authenticationFlow.dispatcher = HandlerForProtocol( authenticationFlow.dispatcher = HandlerForProtocol(
self.browser->GetCommandDispatcher(), BrowsingDataCommands); self.browser->GetCommandDispatcher(), BrowsingDataCommands);
authenticationFlow.delegate = self.viewController;
[self.mediator [self.mediator
authenticateWithIdentity:self.unifiedConsentCoordinator.selectedIdentity authenticateWithIdentity:self.unifiedConsentCoordinator.selectedIdentity
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
#import <UIKit/UIKit.h> #import <UIKit/UIKit.h>
#import "ios/chrome/browser/ui/authentication/authentication_flow.h"
// Delegate that interacts with the user sign-in coordinator. // Delegate that interacts with the user sign-in coordinator.
@protocol UserSigninViewControllerDelegate @protocol UserSigninViewControllerDelegate
...@@ -29,7 +31,8 @@ ...@@ -29,7 +31,8 @@
@end @end
// View controller used to show sign-in UI. // View controller used to show sign-in UI.
@interface UserSigninViewController : UIViewController @interface UserSigninViewController
: UIViewController <AuthenticationFlowDelegate>
// The delegate. // The delegate.
@property(nonatomic, weak) id<UserSigninViewControllerDelegate> delegate; @property(nonatomic, weak) id<UserSigninViewControllerDelegate> delegate;
......
...@@ -142,6 +142,16 @@ enum AuthenticationButtonType { ...@@ -142,6 +142,16 @@ enum AuthenticationButtonType {
: UIInterfaceOrientationMaskPortrait; : UIInterfaceOrientationMaskPortrait;
} }
#pragma mark - AuthenticationFlowDelegate
- (void)didPresentDialog {
[self.activityIndicator stopAnimating];
}
- (void)didDismissDialog {
[self.activityIndicator startAnimating];
}
#pragma mark - MDCActivityIndicator #pragma mark - MDCActivityIndicator
- (void)startAnimatingActivityIndicator { - (void)startAnimatingActivityIndicator {
......
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