[iOS] Add logging for user sign-in operations in architecture migration.

Adds logging for first run and sign-in flows.

Bug: 971989
Change-Id: Ieec769b0db013959396be3ee533eb84f206cb9cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087403
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749673}

ios/chrome/browser/ui/authentication/signin/BUILD.gn

@@ -40,5 +40,6 @@ source_set("signin_impl") {
     "//ios/chrome/browser/ui/authentication/signin/add_account_signin",
     "//ios/chrome/browser/ui/authentication/signin/advanced_settings_signin",
     "//ios/chrome/browser/ui/authentication/signin/user_signin",
+    "//ios/chrome/browser/ui/authentication/signin/user_signin/logging",
   ]
 }

ios/chrome/browser/ui/authentication/signin/signin_coordinator.mm

@@ -6,6 +6,7 @@
 
 #import "ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_coordinator.h"
 #import "ios/chrome/browser/ui/authentication/signin/advanced_settings_signin/advanced_settings_signin_coordinator.h"
+#import "ios/chrome/browser/ui/authentication/signin/user_signin/logging/first_run_signin_logger.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_constants.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_coordinator.h"
 
@@ -25,38 +26,44 @@ using signin_metrics::PromoAction;
                                        identity:(ChromeIdentity*)identity
                                     accessPoint:(AccessPoint)accessPoint
                                     promoAction:(PromoAction)promoAction {
+  UserSigninLogger* logger =
+      [[UserSigninLogger alloc] initWithAccessPoint:accessPoint
+                                        promoAction:promoAction];
   return [[UserSigninCoordinator alloc]
       initWithBaseViewController:viewController
                          browser:browser
                         identity:identity
-                     accessPoint:accessPoint
-                     promoAction:promoAction
-                    signinIntent:UserSigninIntentSignin];
+                    signinIntent:UserSigninIntentSignin
+                          logger:logger];
 }
 
 + (instancetype)firstRunCoordinatorWithBaseViewController:
                     (UINavigationController*)viewController
                                                   browser:(Browser*)browser {
+  UserSigninLogger* logger = [[FirstRunSigninLogger alloc]
+      initWithAccessPoint:AccessPoint::ACCESS_POINT_START_PAGE
+              promoAction:PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO];
   return [[UserSigninCoordinator alloc]
       initWithBaseViewController:viewController
                          browser:browser
                         identity:nil
-                     accessPoint:AccessPoint::ACCESS_POINT_START_PAGE
-                     promoAction:PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO
-                    signinIntent:UserSigninIntentFirstRun];
+                    signinIntent:UserSigninIntentFirstRun
+                          logger:logger];
 }
 
 + (instancetype)
     upgradeSigninPromoCoordinatorWithBaseViewController:
         (UIViewController*)viewController
                                                 browser:(Browser*)browser {
+  UserSigninLogger* logger = [[UserSigninLogger alloc]
+      initWithAccessPoint:AccessPoint::ACCESS_POINT_SIGNIN_PROMO
+              promoAction:PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO];
   return [[UserSigninCoordinator alloc]
       initWithBaseViewController:viewController
                          browser:browser
                         identity:nil
-                     accessPoint:AccessPoint::ACCESS_POINT_SIGNIN_PROMO
-                     promoAction:PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO
-                    signinIntent:UserSigninIntentUpgrade];
+                    signinIntent:UserSigninIntentUpgrade
+                          logger:logger];
 }
 
 + (instancetype)

ios/chrome/browser/ui/authentication/signin/user_signin/BUILD.gn

@@ -28,6 +28,7 @@ source_set("user_signin") {
     "//ios/chrome/browser/sync",
     "//ios/chrome/browser/ui/authentication",
     "//ios/chrome/browser/ui/authentication/signin:signin_protected",
+    "//ios/chrome/browser/ui/authentication/signin/user_signin/logging",
     "//ios/chrome/browser/ui/authentication/unified_consent",
     "//ios/chrome/browser/ui/collection_view/cells",
     "//ios/chrome/browser/ui/commands",

ios/chrome/browser/ui/authentication/signin/user_signin/logging/BUILD.gn

+# Copyright 2020 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import("//build/config/chrome_build.gni")
+
+source_set("logging") {
+  configs += [ "//build/config/compiler:enable_arc" ]
+  sources = [
+    "first_run_signin_logger.h",
+    "first_run_signin_logger.mm",
+    "user_signin_logger.h",
+    "user_signin_logger.mm",
+  ]
+  deps = [
+    "//components/signin/public/base",
+    "//ios/chrome/browser/ui/promos",
+    "//ios/chrome/browser/ui/settings/sync/utils",
+  ]
+  public_deps =
+      [ "//ios/chrome/browser/ui/authentication/signin:signin_headers" ]
+}

ios/chrome/browser/ui/authentication/signin/user_signin/logging/first_run_signin_logger.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_FIRST_RUN_SIGNIN_LOGGER_H_
+#define IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_FIRST_RUN_SIGNIN_LOGGER_H_
+
+#import "ios/chrome/browser/ui/authentication/signin/user_signin/logging/user_signin_logger.h"
+
+// Logs metrics for Chrome first-run operations.
+@interface FirstRunSigninLogger : UserSigninLogger
+@end
+
+#endif  // IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_FIRST_RUN_SIGNIN_LOGGER_H_

ios/chrome/browser/ui/authentication/signin/user_signin/logging/first_run_signin_logger.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/ui/authentication/signin/user_signin/logging/first_run_signin_logger.h"
+
+#import "base/metrics/user_metrics.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using signin_metrics::AccessPoint;
+using signin_metrics::LogSigninAccessPointStarted;
+using signin_metrics::PromoAction;
+using signin_metrics::RecordSigninUserActionForAccessPoint;
+
+@interface FirstRunSigninLogger ()
+
+// Presenter for showing sync-related UI.
+@property(nonatomic, assign) BOOL hasRecordedSigninStarted;
+
+@end
+
+@implementation FirstRunSigninLogger
+
+#pragma mark - Public
+
+- (void)logSigninStarted {
+  if (!self.hasRecordedSigninStarted) {
+    self.hasRecordedSigninStarted = YES;
+    LogSigninAccessPointStarted(self.accessPoint, self.promoAction);
+    RecordSigninUserActionForAccessPoint(self.accessPoint, self.promoAction);
+  }
+}
+
+@end

ios/chrome/browser/ui/authentication/signin/user_signin/logging/user_signin_logger.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_USER_SIGNIN_LOGGER_H_
+#define IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_USER_SIGNIN_LOGGER_H_
+
+#import "components/signin/public/base/signin_metrics.h"
+#import "ios/chrome/browser/ui/authentication/signin/signin_enums.h"
+#import "ios/chrome/browser/ui/settings/sync/utils/sync_presenter.h"
+
+// Logs metrics for user sign-in operations.
+@interface UserSigninLogger : NSObject
+
+- (instancetype)init NS_UNAVAILABLE;
+- (instancetype)initWithAccessPoint:(signin_metrics::AccessPoint)accessPoint
+                        promoAction:(signin_metrics::PromoAction)promoAction
+    NS_DESIGNATED_INITIALIZER;
+
+// View where the sign-in button was displayed.
+@property(nonatomic, assign, readonly) signin_metrics::AccessPoint accessPoint;
+
+// Promo button used to trigger the sign-in.
+@property(nonatomic, assign, readonly) signin_metrics::PromoAction promoAction;
+
+// Logs sign-in started when the user consent screen is first displayed.
+- (void)logSigninStarted;
+
+// Logs sign-in completed when the user has attempted sign-in and obtained a
+// result.
+- (void)logSigninCompletedWithResult:(SigninCoordinatorResult)signinResult
+               advancedSettingsShown:(BOOL)advancedSettingsShown;
+
+// Logs sign-in cancellation when sign-in is in progress.
+// TODO(crbug.com/971989): Used temporarily to log when sign-in is interrupted,
+// |logSigninCompletedWithResult| should be used following the architecture
+// migration.
+- (void)logUndoSignin;
+
+@end
+
+#endif  // IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_USER_SIGNIN_LOGGING_USER_SIGNIN_LOGGER_H_

ios/chrome/browser/ui/authentication/signin/user_signin/logging/user_signin_logger.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/ui/authentication/signin/user_signin/logging/user_signin_logger.h"
+
+#import "base/metrics/user_metrics.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+using base::RecordAction;
+using base::UserMetricsAction;
+using signin_metrics::AccessPoint;
+using signin_metrics::LogSigninAccessPointStarted;
+using signin_metrics::LogSigninAccessPointCompleted;
+using signin_metrics::PromoAction;
+
+@implementation UserSigninLogger
+
+#pragma mark - Public
+
+- (instancetype)initWithAccessPoint:(AccessPoint)accessPoint
+                        promoAction:(PromoAction)promoAction {
+  self = [super init];
+  if (self) {
+    _accessPoint = accessPoint;
+    _promoAction = promoAction;
+  }
+  return self;
+}
+
+- (void)logSigninStarted {
+  LogSigninAccessPointStarted(self.accessPoint, self.promoAction);
+}
+
+- (void)logSigninCompletedWithResult:(SigninCoordinatorResult)signinResult
+               advancedSettingsShown:(BOOL)advancedSettingsShown {
+  switch (signinResult) {
+    case SigninCoordinatorResultSuccess: {
+      LogSigninAccessPointCompleted(self.accessPoint, self.promoAction);
+      if (advancedSettingsShown) {
+        RecordAction(
+            UserMetricsAction("Signin_Signin_WithAdvancedSyncSettings"));
+      } else {
+        RecordAction(
+            UserMetricsAction("Signin_Signin_WithDefaultSyncSettings"));
+      }
+      break;
+    }
+    case SigninCoordinatorResultCanceledByUser: {
+      RecordAction(UserMetricsAction("Signin_Undo_Signin"));
+      break;
+    }
+    case SigninCoordinatorResultInterrupted: {
+      // TODO(crbug.com/951145): Add metric when the sign-in has been
+      // interrupted.
+      break;
+    }
+  }
+}
+
+- (void)logUndoSignin {
+  RecordAction(UserMetricsAction("Signin_Undo_Signin"));
+}
+
+@end

ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_coordinator.h

@@ -8,6 +8,8 @@
 #import "ios/chrome/browser/ui/authentication/signin/signin_coordinator.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_constants.h"
 
+@class UserSigninLogger;
+
 // Coordinates the user sign-in with different intents:
 //  + user sign-in when triggered from UI (settings, bookmarks...)
 //  + first run sign-in
@@ -25,15 +27,13 @@
 // Designated initializer.
 // |viewController| presents the sign-in.
 // |identity| is the identity preselected with the sign-in opens.
-// |accessPoint| is the view where the sign-in button was displayed.
-// |promoAction| is promo button used to trigger the sign-in.
-- (instancetype)
-    initWithBaseViewController:(UIViewController*)viewController
+// |signinIntent| is the intent for the UI displayed in the sign-in flow.
+// |logger| is the logger for sign-in flow operations. This should not be nil.
+- (instancetype)initWithBaseViewController:(UIViewController*)viewController
                                    browser:(Browser*)browser
                                   identity:(ChromeIdentity*)identity
-                   accessPoint:(signin_metrics::AccessPoint)accessPoint
-                   promoAction:(signin_metrics::PromoAction)promoAction
                               signinIntent:(UserSigninIntent)signinIntent
+                                    logger:(UserSigninLogger*)logger
     NS_DESIGNATED_INITIALIZER;
 
 @end

ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_coordinator.mm

@@ -6,7 +6,6 @@
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_coordinator.h"
 
 #import "base/mac/foundation_util.h"
-#import "base/metrics/user_metrics.h"
 #import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/signin/authentication_service_factory.h"
 #import "ios/chrome/browser/signin/identity_manager_factory.h"
@@ -14,6 +13,7 @@
 #import "ios/chrome/browser/sync/sync_setup_service_factory.h"
 #import "ios/chrome/browser/ui/authentication/authentication_flow.h"
 #import "ios/chrome/browser/ui/authentication/signin/signin_coordinator+protected.h"
+#import "ios/chrome/browser/ui/authentication/signin/user_signin/logging/user_signin_logger.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_mediator.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_view_controller.h"
 #import "ios/chrome/browser/ui/authentication/unified_consent/unified_consent_coordinator.h"
@@ -49,11 +49,9 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 // Mediator that handles the sign-in authentication state.
 @property(nonatomic, strong) UserSigninMediator* mediator;
 // Suggested identity shown at sign-in.
-@property(nonatomic, strong) ChromeIdentity* defaultIdentity;
-// View where the sign-in button was displayed.
-@property(nonatomic, assign) AccessPoint accessPoint;
-// Promo button used to trigger the sign-in.
-@property(nonatomic, assign) PromoAction promoAction;
+@property(nonatomic, strong, readonly) ChromeIdentity* defaultIdentity;
+// Logger for sign-in operations.
+@property(nonatomic, strong, readonly) UserSigninLogger* logger;
 // Sign-in intent.
 @property(nonatomic, assign, readonly) UserSigninIntent signinIntent;
 
@@ -66,15 +64,14 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 - (instancetype)initWithBaseViewController:(UIViewController*)viewController
                                    browser:(Browser*)browser
                                   identity:(ChromeIdentity*)identity
-                               accessPoint:(AccessPoint)accessPoint
-                               promoAction:(PromoAction)promoAction
-                              signinIntent:(UserSigninIntent)signinIntent {
+                              signinIntent:(UserSigninIntent)signinIntent
+                                    logger:(UserSigninLogger*)logger {
   self = [super initWithBaseViewController:viewController browser:browser];
   if (self) {
     _defaultIdentity = identity;
-    _accessPoint = accessPoint;
-    _promoAction = promoAction;
     _signinIntent = signinIntent;
+    DCHECK(logger);
+    _logger = logger;
   }
   return self;
 }
@@ -109,7 +106,7 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
   // Set UnifiedConsentCoordinator properties.
   self.unifiedConsentCoordinator.selectedIdentity = self.defaultIdentity;
   self.unifiedConsentCoordinator.autoOpenIdentityPicker =
-      self.promoAction == PromoAction::PROMO_ACTION_NOT_DEFAULT;
+      self.logger.promoAction == PromoAction::PROMO_ACTION_NOT_DEFAULT;
 
   [self.unifiedConsentCoordinator start];
 
@@ -130,6 +127,8 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
       break;
     }
   }
+
+  [self.logger logSigninStarted];
 }
 
 - (void)interruptWithAction:(SigninCoordinatorInterruptAction)action
@@ -138,6 +137,12 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
   // we do not expect the sign-in to be interrupted for first run.
   DCHECK(self.signinIntent != UserSigninIntentFirstRun);
 
+  if (self.mediator.isAuthenticationInProgress) {
+    // TODO(crbug.com/971989): Rename this metric after the architecture
+    // migration.
+    [self.logger logUndoSignin];
+  }
+
   __weak UserSigninCoordinator* weakSelf = self;
   if (self.addAccountSigninCoordinator) {
     // |self.addAccountSigninCoordinator| needs to be interupted before
@@ -210,12 +215,13 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 }
 
 - (void)userSigninViewControllerDidTapOnAddAccount {
+  DCHECK(!self.addAccountSigninCoordinator);
   [self notifyUserSigninAttempted];
 
   self.addAccountSigninCoordinator = [SigninCoordinator
       addAccountCoordinatorWithBaseViewController:self.viewController
                                           browser:self.browser
-                                      accessPoint:self.accessPoint];
+                                      accessPoint:self.logger.accessPoint];
 
   __weak UserSigninCoordinator* weakSelf = self;
   self.addAccountSigninCoordinator.signinCompletion =
@@ -235,6 +241,9 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 
 - (void)userSigninViewControllerDidTapOnSkipSignin {
   [self.mediator cancelSignin];
+  // TODO(crbug.com/971989): Remove this metric after the architecture
+  // migration.
+  [self.logger logUndoSignin];
 }
 
 - (void)userSigninViewControllerDidTapOnSignin {
@@ -254,14 +263,9 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 
 - (int)userSigninMediatorGetConsentConfirmationId {
   if (self.userSigninMediatorGetSettingsLinkWasTapped) {
-    base::RecordAction(
-        base::UserMetricsAction("Signin_Signin_WithAdvancedSyncSettings"));
     return self.unifiedConsentCoordinator.openSettingsStringId;
-  } else {
-    base::RecordAction(
-        base::UserMetricsAction("Signin_Signin_WithDefaultSyncSettings"));
-    return self.viewController.acceptSigninButtonStringId;
   }
+  return self.viewController.acceptSigninButtonStringId;
 }
 
 - (const std::vector<int>&)userSigninMediatorGetConsentStringIds {
@@ -270,9 +274,12 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
 
 - (void)userSigninMediatorSigninFinishedWithResult:
     (SigninCoordinatorResult)signinResult {
+  [self.logger logSigninCompletedWithResult:signinResult
+                      advancedSettingsShown:self.unifiedConsentCoordinator
+                                                .settingsLinkWasTapped];
+
   BOOL settingsWasTapped = self.unifiedConsentCoordinator.settingsLinkWasTapped;
   ChromeIdentity* identity = self.unifiedConsentCoordinator.selectedIdentity;
-  [self recordSigninMetricsWithResult:signinResult];
   __weak UserSigninCoordinator* weakSelf = self;
   ProceduralBlock completion = ^void() {
     [weakSelf viewControllerDismissedWithResult:signinResult
@@ -336,26 +343,6 @@ const CGFloat kFadeOutAnimationDuration = 0.16f;
   [self.advancedSettingsSigninCoordinator start];
 }
 
-// Records the metrics when the sign-in is finished.
-- (void)recordSigninMetricsWithResult:(SigninCoordinatorResult)signinResult {
-  switch (signinResult) {
-    case SigninCoordinatorResultSuccess: {
-      signin_metrics::LogSigninAccessPointCompleted(self.accessPoint,
-                                                    self.promoAction);
-      break;
-    }
-    case SigninCoordinatorResultCanceledByUser: {
-      base::RecordAction(base::UserMetricsAction("Signin_Undo_Signin"));
-      break;
-    }
-    case SigninCoordinatorResultInterrupted: {
-      // TODO(crbug.com/951145): Add metric when the sign-in has been
-      // interrupted.
-      break;
-    }
-  }
-}
-
 // Displays the sign-in screen with transitions specific to first-run.
 - (void)presentFirstRun {
   DCHECK(self.viewController);

ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_mediator.h

@@ -69,6 +69,9 @@ class UnifiedConsentService;
 // The delegate.
 @property(nonatomic, weak) id<UserSigninMediatorDelegate> delegate;
 
+// Whether the authentication operation is in progress.
+@property(nonatomic, assign, readonly) BOOL isAuthenticationInProgress;
+
 // Enters the authentication state following identity selection. If there is an
 // error transitions to the identity selection state, otherwise enters the final
 // authentication completed state.

ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_mediator.mm

@@ -87,10 +87,7 @@
   if (!self.isAuthenticationInProgress) {
     return;
   }
-  // TODO(crbug.com/971989): Remove this metric following the architecture
-  // migration in the case that the flow has been dismissed by the user and
-  // rename in the case sign-in has been interrupted.
-  base::RecordAction(base::UserMetricsAction("Signin_Undo_Signin"));
+
   // TODO(crbug.com/1056634): Support cancelAndDismiss with animation parameter.
   [self.authenticationFlow cancelAndDismiss];
   self.authenticationService->SignOut(signin_metrics::ABORT_SIGNIN,