Implementing sign-in promo histograms for settings

ios/chrome/browser/ui/settings/settings_collection_view_controller.mm

 // Copyright 2015 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/settings/settings_collection_view_controller.h"
 
 #include <memory>
 
 #import "base/mac/foundation_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/scoped_observer.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/browser_sync/profile_sync_service.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/util.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/core/common/signin_pref_names.h"
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state_removal_controller.h"
 #include "ios/chrome/browser/experimental_flags.h"
@@ skipping 60 matching lines @@
 
 @interface SettingsCollectionViewController (NotificationBridgeDelegate)
 // Notifies this controller that the sign in state has changed.
 - (void)onSignInStateChanged;
 @end
 
 namespace {
 
 const CGFloat kAccountProfilePhotoDimension = 40.0f;
 
+const int kAutomaticSigninPromoViewDismissCount = 20;
+
 typedef NS_ENUM(NSInteger, SectionIdentifier) {
   SectionIdentifierSignIn = kSectionIdentifierEnumZero,
   SectionIdentifierBasics,
   SectionIdentifierAdvanced,
   SectionIdentifierInfo,
   SectionIdentifierDebug,
 };
 
 typedef NS_ENUM(NSInteger, ItemType) {
   ItemTypeSignInButton = kItemTypeEnumZero,
@@ skipping 107 matching lines @@
   // TODO(crbug.com/662435): Refactor PrefObserverBridge so it owns the
   // PrefChangeRegistrar.
   // Registrar for pref changes notifications.
   PrefChangeRegistrar _prefChangeRegistrar;
 
   // Updatable Items.
   CollectionViewDetailItem* _voiceSearchDetailItem;
   CollectionViewDetailItem* _defaultSearchEngineItem;
   CollectionViewDetailItem* _savePasswordsDetailItem;
   CollectionViewDetailItem* _autoFillDetailItem;
+
+  // YES if the user used at least once the sign-in promo view buttons.
+  BOOL _signinStarted;
 }
 
 // Stops observing browser state services. This is required during the shutdown
 // phase to avoid observing services for a profile that is being killed.
 - (void)stopBrowserStateServiceObservers;
 
 @end
 
 @implementation SettingsCollectionViewController
 
@@ skipping 63 matching lines @@
 #pragma mark View lifecycle
 
 // TODO(crbug.com/661915): Refactor TemplateURLObserver and re-implement this so
 // it observes the default search engine name instead of reloading on
 // ViewWillAppear.
 - (void)viewWillAppear:(BOOL)animated {
   [super viewWillAppear:animated];
   [self updateSearchCell];
 }
 
+- (void)viewDidDisappear:(BOOL)animated {
+  [super viewDidDisappear:animated];
+  if (!_signinStarted && _signinPromoViewMediator) {
+    PrefService* prefs = _browserState->GetPrefs();
+    int displayedCount =
+        prefs->GetInteger(prefs::kIosSettingsSigninPromoDisplayedCount);
+    UMA_HISTOGRAM_COUNTS_100("MobileSignInPromo.SettingsManager.DismissalCount",
+                             displayedCount);
+  }
+}
+
 #pragma mark SettingsRootCollectionViewController
 
 - (void)loadModel {
   [super loadModel];
 
   CollectionViewModel* model = self.collectionViewModel;
 
   // Sign in/Account section
   [model addSectionWithIdentifier:SectionIdentifierSignIn];
   AuthenticationService* authService =
       AuthenticationServiceFactory::GetForBrowserState(_browserState);
   if (!authService->IsAuthenticated()) {
     if (!_hasRecordedSigninImpression) {
       // Once the Settings are open, this button impression will at most be
       // recorded once until they are closed.
       base::RecordAction(
           base::UserMetricsAction("Signin_Impression_FromSettings"));
       _hasRecordedSigninImpression = YES;
     }
-    if (experimental_flags::IsSigninPromoEnabled()) {
+    PrefService* prefs = _browserState->GetPrefs();
+    int displayedCount =
+        prefs->GetInteger(prefs::kIosSettingsSigninPromoDisplayedCount);
+    if (experimental_flags::IsSigninPromoEnabled() &&
+        displayedCount < kAutomaticSigninPromoViewDismissCount) {
       _signinPromoViewMediator =
           [[SigninPromoViewMediator alloc] initWithBrowserState:_browserState];
       _signinPromoViewMediator.consumer = self;
+      ++displayedCount;

[sdefresne, 2017/06/26 06:34:35]

nit: it is clearer in my opinion to just pass "displayedCount + 1" and to mark
the variable "displayedCount" as const

  prefs->SetInteger(prefs::kIosSettingsSeigninPromoDisplayedCount,
                    displayedCount + 1);

[jlebel, 2017/06/26 19:19:21]

Done.

+      prefs->SetInteger(prefs::kIosSettingsSigninPromoDisplayedCount,
+                        displayedCount);
     }
     [model addItem:[self signInTextItem]
         toSectionWithIdentifier:SectionIdentifierSignIn];
   } else {
     _signinPromoViewMediator = nil;
     [model addItem:[self accountCellItem]
         toSectionWithIdentifier:SectionIdentifierSignIn];
   }
 
   // Basics section
@@ skipping 59 matching lines @@
   [model addItem:[self showAutofillTypePredictionsSwitchItem]
       toSectionWithIdentifier:SectionIdentifierDebug];
   [model addItem:[self materialCatalogDetailItem]
       toSectionWithIdentifier:SectionIdentifierDebug];
 #endif  // CHROMIUM_BUILD && !defined(NDEBUG)
 }
 
 #pragma mark - Model Items
 
 - (CollectionViewItem*)signInTextItem {
-  if (experimental_flags::IsSigninPromoEnabled()) {
-    DCHECK(_signinPromoViewMediator);
+  if (_signinPromoViewMediator) {
     SigninPromoItem* signinPromoItem =
         [[SigninPromoItem alloc] initWithType:ItemTypeSigninPromo];
     signinPromoItem.configurator =
         [_signinPromoViewMediator createConfigurator];
     return signinPromoItem;
   }
   AccountSignInItem* signInTextItem =
       [[AccountSignInItem alloc] initWithType:ItemTypeSignInButton];
   signInTextItem.accessibilityIdentifier = kSettingsSignInCellId;
   UIImage* image = CircularImageFromImage(ios::GetChromeBrowserProvider()
@@ skipping 719 matching lines @@
     [self reconfigureCellsForItems:@[ signinPromoItem ]];
     if (identityChanged)
       [self.collectionViewLayout invalidateLayout];
   }
 }
 
 #pragma mark - SigninPromoViewDelegate
 
 - (void)signinPromoViewDidTapSigninWithNewAccount:
     (SigninPromoView*)signinPromoView {
+  [self sendCountTillSigninHistogram];
   DCHECK(!_signinPromoViewMediator.defaultIdentity);
   base::RecordAction(
       base::UserMetricsAction("Signin_SigninNewAccount_FromSettings"));
   [self showSignInWithIdentity:nil
                    promoAction:signin_metrics::PromoAction::
                                    PROMO_ACTION_NEW_ACCOUNT];
 }
 
 - (void)signinPromoViewDidTapSigninWithDefaultAccount:
     (SigninPromoView*)signinPromoView {
+  [self sendCountTillSigninHistogram];
   ChromeIdentity* identity = _signinPromoViewMediator.defaultIdentity;
   DCHECK(identity);
   base::RecordAction(
       base::UserMetricsAction("Signin_SigninWithDefault_FromSettings"));
   [self showSignInWithIdentity:identity
                    promoAction:signin_metrics::PromoAction::
                                    PROMO_ACTION_WITH_DEFAULT];
 }
 
 - (void)signinPromoViewDidTapSigninWithOtherAccount:
     (SigninPromoView*)signinPromoView {
+  [self sendCountTillSigninHistogram];
   DCHECK(_signinPromoViewMediator.defaultIdentity);
   base::RecordAction(
       base::UserMetricsAction("Signin_SigninNotDefault_FromSettings"));
   [self showSignInWithIdentity:nil
                    promoAction:signin_metrics::PromoAction::
                                    PROMO_ACTION_NOT_DEFAULT];
 }
 
+#pragma mark - Metrics
+
+- (void)sendCountTillSigninHistogram {

[sdefresne, 2017/06/26 06:34:35]

nit: in your other CL and the histogram.xml file you used "Til" instead of
"Till", maybe use the same naming convention here too

[jlebel, 2017/06/26 19:19:21]

Yes, I definitely made a mistake here. Thanks.

+  _signinStarted = YES;
+  PrefService* prefs = _browserState->GetPrefs();
+  int displayedCount =
+      prefs->GetInteger(prefs::kIosSettingsSigninPromoDisplayedCount);
+  UMA_HISTOGRAM_COUNTS_100("MobileSignInPromo.SettingsManager.CountTilSignin",
+                           displayedCount);
+}
+
 @end