[iOS] Cleanup SigninInnteractionController

Creating SigninResult enum with 3 values:
 + CANCELED
 + SUCCESS
 + SIGNED_IN_AND_OPEN_SETTINGS

This enum is used by SigninInteractionController, when the user taps on
either "cancel", "yes I'm in", or "settings".

This patch prepares the new UI to present the settings before starting
the sync:
crrev.com/c/1539036

Bug: 883075
Change-Id: Ic5db6ff7d67d0ec7ba356741391c13f51198cf36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538509
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644885}

ios/chrome/browser/ui/signin_interaction/signin_interaction_controller.h

@@ -9,7 +9,6 @@
 
 #import "base/ios/block_types.h"
 #include "components/signin/core/browser/signin_metrics.h"
-#include "ios/chrome/browser/signin/constants.h"
 
 @protocol ApplicationCommands;
 namespace ios {
@@ -18,6 +17,20 @@ class ChromeBrowserState;
 @class ChromeIdentity;
 @protocol SigninInteractionPresenting;
 
+// Sign-in result from SigninInteractionController.
+typedef NS_ENUM(NSInteger, SigninResult) {
+  // The user canceled sign-in.
+  SigninResultCanceled,
+  // The user signed in.
+  SigninResultSuccess,
+  // The user wants to check the synn settings before to start the sync.
+  SigninResultSignedInnAndOpennSettings,
+};
+
+// The type of the completion handler block when sign-in is finished.
+typedef void (^SigninInteractionControllerCompletionCallback)(
+    SigninResult signinResult);
+
 // Interaction controller for sign-in related operations. This class is mainly a
 // proxy for |ChromeSigninViewController|, calling directly
 // |ChromeIdentityInteractionManager| for the no-accounts case.
@@ -41,16 +54,19 @@ class ChromeBrowserState;
 // * |completion| will be called when the operation is done, and
 //   |succeeded| will notify the caller on whether the user is now signed in.
 - (void)signInWithIdentity:(ChromeIdentity*)identity
-                completion:(signin_ui::CompletionCallback)completion;
+                completion:
+                    (SigninInteractionControllerCompletionCallback)completion;
 
 // Re-authenticate the user. This method will always show a sign-in web flow.
 // The completion block will be called when the operation is done, and
 // |succeeded| will notify the caller on whether the user has been
 // correctly re-authenticated.
-- (void)reAuthenticateWithCompletion:(signin_ui::CompletionCallback)completion;
+- (void)reAuthenticateWithCompletion:
+    (SigninInteractionControllerCompletionCallback)completion;
 
 // Starts the flow to add an identity via ChromeIdentityInteractionManager.
-- (void)addAccountWithCompletion:(signin_ui::CompletionCallback)completion;
+- (void)addAccountWithCompletion:
+    (SigninInteractionControllerCompletionCallback)completion;
 
 // Cancels any current process. Calls the completion callback when done.
 - (void)cancel;

ios/chrome/browser/ui/signin_interaction/signin_interaction_controller.mm

@@ -29,8 +29,6 @@
 #error "This file requires ARC support."
 #endif
 
-using signin_ui::CompletionCallback;
-
 @interface SigninInteractionController ()<
     ChromeIdentityInteractionManagerDelegate,
     ChromeSigninViewControllerDelegate> {
@@ -40,7 +38,7 @@ using signin_ui::CompletionCallback;
   BOOL isCancelling_;
   BOOL isDismissing_;
   BOOL interactionManagerDismissalIgnored_;
-  CompletionCallback completionCallback_;
+  SigninInteractionControllerCompletionCallback completionCallback_;
   ChromeSigninViewController* signinViewController_;
   ChromeIdentityInteractionManager* identityInteractionManager_;
   ChromeIdentity* signInIdentity_;
@@ -102,7 +100,8 @@ using signin_ui::CompletionCallback;
 }
 
 - (void)signInWithIdentity:(ChromeIdentity*)identity
-                completion:(signin_ui::CompletionCallback)completion {
+                completion:
+                    (SigninInteractionControllerCompletionCallback)completion {
   signin_metrics::LogSigninAccessPointStarted(accessPoint_, promoAction_);
   completionCallback_ = [completion copy];
   ios::ChromeIdentityService* identityService =
@@ -123,7 +122,7 @@ using signin_ui::CompletionCallback;
     if (!identityInteractionManager_) {
       // Abort sign-in if the ChromeIdentityInteractionManager returned is
       // nil (this can happen when the iOS internal provider is not used).
-      [self runCompletionCallbackWithSuccess:NO showAccountsSettings:NO];
+      [self runCompletionCallbackWithSigninResult:SigninResultCanceled];
       return;
     }
 
@@ -135,7 +134,8 @@ using signin_ui::CompletionCallback;
   }
 }
 
-- (void)reAuthenticateWithCompletion:(CompletionCallback)completion {
+- (void)reAuthenticateWithCompletion:
+    (SigninInteractionControllerCompletionCallback)completion {
   signin_metrics::LogSigninAccessPointStarted(accessPoint_, promoAction_);
   completionCallback_ = [completion copy];
   CoreAccountInfo accountInfo =
@@ -173,7 +173,8 @@ using signin_ui::CompletionCallback;
                     }];
 }
 
-- (void)addAccountWithCompletion:(CompletionCallback)completion {
+- (void)addAccountWithCompletion:
+    (SigninInteractionControllerCompletionCallback)completion {
   completionCallback_ = [completion copy];
   identityInteractionManager_ =
       ios::GetChromeBrowserProvider()
@@ -197,13 +198,13 @@ using signin_ui::CompletionCallback;
   if (error) {
     // Filter out cancel and errors handled internally by ChromeIdentity.
     if (!ShouldHandleSigninError(error)) {
-      [self runCompletionCallbackWithSuccess:NO showAccountsSettings:NO];
+      [self runCompletionCallbackWithSigninResult:SigninResultCanceled];
       return;
     }
 
     __weak SigninInteractionController* weakSelf = self;
     ProceduralBlock dismissAction = ^{
-      [weakSelf runCompletionCallbackWithSuccess:NO showAccountsSettings:NO];
+      [weakSelf runCompletionCallbackWithSigninResult:SigninResultCanceled];
     };
     [self.presenter presentError:error dismissAction:dismissAction];
     return;
@@ -211,7 +212,7 @@ using signin_ui::CompletionCallback;
   if (shouldSignIn) {
     [self showSigninViewControllerWithIdentity:identity identityAdded:YES];
   } else {
-    [self runCompletionCallbackWithSuccess:YES showAccountsSettings:NO];
+    [self runCompletionCallbackWithSigninResult:SigninResultSuccess];
   }
 }
 
@@ -289,18 +290,14 @@ using signin_ui::CompletionCallback;
   }
 }
 
-- (void)dismissSigninViewControllerWithSignInSuccess:(BOOL)success
-                                showAccountsSettings:
-                                    (BOOL)showAccountsSettings {
+- (void)dismissSigninViewControllerWithSigninResult:(SigninResult)signinResult {
   DCHECK(signinViewController_);
   if ((isCancelling_ && !isDismissing_) || !self.presenter.isPresenting) {
-    [self runCompletionCallbackWithSuccess:success
-                      showAccountsSettings:showAccountsSettings];
+    [self runCompletionCallbackWithSigninResult:signinResult];
     return;
   }
   ProceduralBlock completion = ^{
-    [self runCompletionCallbackWithSuccess:success
-                      showAccountsSettings:showAccountsSettings];
+    [self runCompletionCallbackWithSigninResult:signinResult];
   };
   [self dismissPresentedViewControllersAnimated:YES completion:completion];
 }
@@ -317,8 +314,7 @@ using signin_ui::CompletionCallback;
 
 - (void)didSkipSignIn:(ChromeSigninViewController*)controller {
   DCHECK_EQ(controller, signinViewController_);
-  [self dismissSigninViewControllerWithSignInSuccess:NO
-                                showAccountsSettings:NO];
+  [self dismissSigninViewControllerWithSigninResult:SigninResultCanceled];
 }
 
 - (void)didSignIn:(ChromeSigninViewController*)controller {
@@ -339,28 +335,27 @@ using signin_ui::CompletionCallback;
           ->GetChromeIdentityService()
           ->ForgetIdentity(identity, nil);
     }
-    [self dismissSigninViewControllerWithSignInSuccess:NO
-                                  showAccountsSettings:NO];
+    [self dismissSigninViewControllerWithSigninResult:SigninResultCanceled];
   }
 }
 
 - (void)didFailSignIn:(ChromeSigninViewController*)controller {
   DCHECK_EQ(controller, signinViewController_);
-  [self dismissSigninViewControllerWithSignInSuccess:NO
-                                showAccountsSettings:NO];
+  [self dismissSigninViewControllerWithSigninResult:SigninResultCanceled];
 }
 
 - (void)didAcceptSignIn:(ChromeSigninViewController*)controller
     showAccountsSettings:(BOOL)showAccountsSettings {
   DCHECK_EQ(controller, signinViewController_);
-  [self dismissSigninViewControllerWithSignInSuccess:YES
-                                showAccountsSettings:showAccountsSettings];
+  SigninResult signinResult = showAccountsSettings
+                                  ? SigninResultSignedInnAndOpennSettings
+                                  : SigninResultSuccess;
+  [self dismissSigninViewControllerWithSigninResult:signinResult];
 }
 
 #pragma mark - Utility methods
 
-- (void)runCompletionCallbackWithSuccess:(BOOL)success
-                    showAccountsSettings:(BOOL)showAccountsSettings {
+- (void)runCompletionCallbackWithSigninResult:(SigninResult)signinResult {
   // In order to avoid awkward double transitions, |identityInteractionManager_|
   // is not dismissed when requested (except when canceling). However, in case
   // of errors, |identityInteractionManager_| needs to be directly dismissed,
@@ -369,17 +364,13 @@ using signin_ui::CompletionCallback;
     [self dismissPresentedViewControllersAnimated:YES completion:nil];
   }
 
-  if (showAccountsSettings) {
-    [self.presenter showAccountsSettings];
-  }
-
   // Cleaning up and calling the |completionCallback_| should be done last.
   identityInteractionManager_ = nil;
   signinViewController_ = nil;
   // Ensure self is not destroyed in the callbacks.
   SigninInteractionController* strongSelf = self;
   if (completionCallback_) {
-    completionCallback_(success);
+    completionCallback_(signinResult);
     completionCallback_ = nil;
   }
   strongSelf = nil;

ios/chrome/browser/ui/signin_interaction/signin_interaction_coordinator.mm

@@ -37,15 +37,12 @@
 // Bookkeeping for the top-most view controller.
 @property(nonatomic, strong) UIViewController* topViewController;
 
+// Sign-in completion.
+@property(nonatomic, copy) signin_ui::CompletionCallback signinCompletion;
+
 @end
 
 @implementation SigninInteractionCoordinator
-@synthesize alertCoordinator = _alertCoordinator;
-@synthesize browserState = _browserState;
-@synthesize controller = _controller;
-@synthesize dispatcher = _dispatcher;
-@synthesize presentingViewController = _presentingViewController;
-@synthesize topViewController = _topViewController;
 
 - (instancetype)initWithBrowserState:(ios::ChromeBrowserState*)browserState
                           dispatcher:(id<ApplicationCommands>)dispatcher {
@@ -70,11 +67,11 @@
 
   [self setupForSigninOperationWithAccessPoint:accessPoint
                                    promoAction:promoAction
-                      presentingViewController:viewController];
+                      presentingViewController:viewController
+                                    completion:completion];
 
-  [self.controller
-      signInWithIdentity:identity
-              completion:[self callbackToClearStateWithCompletion:completion]];
+  [self.controller signInWithIdentity:identity
+                           completion:[self callbackToClearState]];
 }
 
 - (void)reAuthenticateWithAccessPoint:(signin_metrics::AccessPoint)accessPoint
@@ -89,10 +86,10 @@
 
   [self setupForSigninOperationWithAccessPoint:accessPoint
                                    promoAction:promoAction
-                      presentingViewController:viewController];
+                      presentingViewController:viewController
+                                    completion:completion];
 
-  [self.controller reAuthenticateWithCompletion:
-                       [self callbackToClearStateWithCompletion:completion]];
+  [self.controller reAuthenticateWithCompletion:[self callbackToClearState]];
 }
 
 - (void)addAccountWithAccessPoint:(signin_metrics::AccessPoint)accessPoint
@@ -106,10 +103,10 @@
 
   [self setupForSigninOperationWithAccessPoint:accessPoint
                                    promoAction:promoAction
-                      presentingViewController:viewController];
+                      presentingViewController:viewController
+                                    completion:completion];
 
-  [self.controller addAccountWithCompletion:
-                       [self callbackToClearStateWithCompletion:completion]];
+  [self.controller addAccountWithCompletion:[self callbackToClearState]];
 }
 
 - (void)cancel {
@@ -171,16 +168,6 @@
   self.alertCoordinator = nil;
 }
 
-- (void)showAccountsSettings {
-  if (unified_consent::IsUnifiedConsentFeatureEnabled()) {
-    [self.dispatcher showGoogleServicesSettingsFromViewController:
-                         self.presentingViewController];
-  } else {
-    [self.dispatcher
-        showAccountsSettingsFromViewController:self.presentingViewController];
-  }
-}
-
 - (BOOL)isPresenting {
   return self.presentingViewController.presentedViewController != nil;
 }
@@ -188,12 +175,17 @@
 #pragma mark - Private Methods
 
 // Sets up relevant instance variables for a sign in operation.
-- (void)
-setupForSigninOperationWithAccessPoint:(signin_metrics::AccessPoint)accessPoint
-                           promoAction:(signin_metrics::PromoAction)promoAction
-              presentingViewController:
-                  (UIViewController*)presentingViewController {
+- (void)setupForSigninOperationWithAccessPoint:
+            (signin_metrics::AccessPoint)accessPoint
+                                   promoAction:
+                                       (signin_metrics::PromoAction)promoAction
+                      presentingViewController:
+                          (UIViewController*)presentingViewController
+                                    completion:(signin_ui::CompletionCallback)
+                                                   completion {
   DCHECK(![self isPresenting]);
+  DCHECK(!self.signinCompletion);
+  self.signinCompletion = completion;
   self.presentingViewController = presentingViewController;
   self.topViewController = presentingViewController;
 
@@ -207,19 +199,40 @@ setupForSigninOperationWithAccessPoint:(signin_metrics::AccessPoint)accessPoint
 
 // Returns a callback that clears the state of the coordinator and runs
 // |completion|.
-- (signin_ui::CompletionCallback)callbackToClearStateWithCompletion:
-    (signin_ui::CompletionCallback)completion {
+- (SigninInteractionControllerCompletionCallback)callbackToClearState {
   __weak SigninInteractionCoordinator* weakSelf = self;
-  signin_ui::CompletionCallback completionCallback = ^(BOOL success) {
-    weakSelf.controller = nil;
-    weakSelf.presentingViewController = nil;
-    weakSelf.topViewController = nil;
-    weakSelf.alertCoordinator = nil;
-    if (completion) {
-      completion(success);
-    }
-  };
+  SigninInteractionControllerCompletionCallback completionCallback =
+      ^(SigninResult signinResult) {
+        [weakSelf
+            signinInteractionControllerCompletionWithSigninResult:signinResult];
+      };
   return completionCallback;
 }
 
+// Called when SigninInteractionController is completed.
+- (void)signinInteractionControllerCompletionWithSigninResult:
+    (SigninResult)signinResult {
+  self.controller = nil;
+  self.topViewController = nil;
+  self.alertCoordinator = nil;
+  if (signinResult == SigninResultSignedInnAndOpennSettings) {
+    [self showAccountsSettings];
+  }
+  if (self.signinCompletion) {
+    self.signinCompletion(signinResult != SigninResultCanceled);
+    self.signinCompletion = nil;
+  }
+}
+
+// Shows the accounts settings UI.
+- (void)showAccountsSettings {
+  if (unified_consent::IsUnifiedConsentFeatureEnabled()) {
+    [self.dispatcher showGoogleServicesSettingsFromViewController:
+                         self.presentingViewController];
+  } else {
+    [self.dispatcher
+        showAccountsSettingsFromViewController:self.presentingViewController];
+  }
+}
+
 @end

ios/chrome/browser/ui/signin_interaction/signin_interaction_presenting.h

@@ -36,9 +36,6 @@
 // Dismisses the error dialog.
 - (void)dismissError;
 
-// Shows the accounts settings UI.
-- (void)showAccountsSettings;
-
 // Indicates whether the object is currently presenting.
 @property(nonatomic, assign, readonly, getter=isPresenting) BOOL presenting;