Expanding signin histogram coverage.

chrome/browser/ui/sync/one_click_signin_helper.cc

Index: chrome/browser/ui/sync/one_click_signin_helper.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc b/chrome/browser/ui/sync/one_click_signin_helper.cc
index 7a898bad89ee47a1e4956eb38230e2c40297c99f..6825d7ae21bdac347e507ef84243c8c5796c0310 100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -91,6 +91,51 @@ void AddEmailToOneClickRejectedList(Profile* profile,
   updater->AppendIfNotPresent(new base::StringValue(email));
 }
 
+void LogHistogramValue(SyncPromoUI::Source source, int action) {
+  switch (source) {
+    case SyncPromoUI::SOURCE_START_PAGE:
+      UMA_HISTOGRAM_ENUMERATION("Signin.StartPageActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_NTP_LINK:
+      UMA_HISTOGRAM_ENUMERATION("Signin.NTPLinkActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_MENU:
+      UMA_HISTOGRAM_ENUMERATION("Signin.MenuActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_SETTINGS:
+      UMA_HISTOGRAM_ENUMERATION("Signin.SettingsActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_EXTENSION_INSTALL_BUBBLE:
+      UMA_HISTOGRAM_ENUMERATION("Signin.ExtensionInstallBubbleActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_WEBSTORE_INSTALL:
+      UMA_HISTOGRAM_ENUMERATION("Signin.WebstoreInstallActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    case SyncPromoUI::SOURCE_APP_LAUNCHER:
+      UMA_HISTOGRAM_ENUMERATION("Signin.AppLauncherActions", action,
+                                one_click_signin::HISTOGRAM_MAX);
+      break;
+    default:
+      NOTREACHED() << "Invalid Source";
+      return;
+  }
+  UMA_HISTOGRAM_ENUMERATION("Signin.AllAccessPointActions", action,
+                            one_click_signin::HISTOGRAM_MAX);
+}
+
+void LogOneClickHistogramValue(int action) {
+  UMA_HISTOGRAM_ENUMERATION("Signin.OneClickActions", action,
+                            one_click_signin::HISTOGRAM_MAX);
+  UMA_HISTOGRAM_ENUMERATION("Signin.AllAccessPointActions", action,
+                            one_click_signin::HISTOGRAM_MAX);
+}
+
 // Arguments used with StartSync function.  base::Bind() cannot support too
 // many args for performance reasons, so they are packaged up into a struct.
 struct StartSyncArgs {
@@ -100,14 +145,16 @@ struct StartSyncArgs {
                 const std::string& session_index,
                 const std::string& email,
                 const std::string& password,
-                bool force_same_tab_navigation)
+                bool force_same_tab_navigation,
+                SyncPromoUI::Source source)
       : profile(profile),
         browser(browser),
         auto_accept(auto_accept),
         session_index(session_index),
         email(email),
         password(password),
-        force_same_tab_navigation(force_same_tab_navigation) {
+        force_same_tab_navigation(force_same_tab_navigation),
+        source(source) {
   }
 
   Profile* profile;
@@ -117,15 +164,14 @@ struct StartSyncArgs {
   std::string email;
   std::string password;
   bool force_same_tab_navigation;
+  SyncPromoUI::Source source;

[Roger Tawa OOO till Jul 10th, 2013/02/28 20:23:54]

Is this extra member needed?  It does not seem used in StartSync.

[jwd, 2013/02/28 20:45:38]

Done.

 };
 
 // Start syncing with the given user information.
 void StartSync(const StartSyncArgs& args,
                OneClickSigninSyncStarter::StartSyncMode start_mode) {
   if (start_mode == OneClickSigninSyncStarter::UNDO_SYNC) {
-    UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse",
-                              one_click_signin::HISTOGRAM_UNDO,
-                              one_click_signin::HISTOGRAM_MAX);
+    LogOneClickHistogramValue(one_click_signin::HISTOGRAM_UNDO);
     return;
   }
   // The starter deletes itself once its done.
@@ -158,9 +204,7 @@ void StartSync(const StartSyncArgs& args,
       NOTREACHED() << "Invalid auto_accept: " << args.auto_accept;
       break;
   }
-
-  UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse", action,
-                            one_click_signin::HISTOGRAM_MAX);
+  LogOneClickHistogramValue(action);
 }
 
 void StartExplicitSync(const StartSyncArgs& args,
@@ -327,6 +371,20 @@ ConfirmEmailDialogDelegate::ConfirmEmailDialogDelegate(
     callback_(callback) {
 }
 
+// Tells when we are in the process of showing either the signin to chrome page
+// or the one click sign in to chrome page.
+bool AreWeShowingSignin(GURL url, SyncPromoUI::Source source,
+                        std::string email) {
+  GURL::Replacements replacements;
+  replacements.ClearQuery();
+  GURL clean_login_url =
+      GURL(GaiaUrls::GetInstance()->service_login_url()).ReplaceComponents(
+          replacements);
+
+  return (url.ReplaceComponents(replacements) == clean_login_url &&
+          source != SyncPromoUI::SOURCE_UNKNOWN) || !email.empty();
+}
+
 }  // namespace
 
 // The infobar asking the user if they want to use one-click sign in.
@@ -453,7 +511,8 @@ bool OneClickInfoBarDelegateImpl::Accept() {
                      StartSyncArgs(profile, browser,
                                    OneClickSigninHelper::AUTO_ACCEPT_NONE,
                                    session_index_, email_, password_,
-                                   false /* force_same_tab_navigation */)));
+                                   false /* force_same_tab_navigation */,
+                                   SyncPromoUI::SOURCE_UNKNOWN)));
   button_pressed_ = true;
   return true;
 }
@@ -502,8 +561,10 @@ void OneClickInfoBarDelegateImpl::RecordHistogramAction(int action) {
 
 OneClickSigninHelper::OneClickSigninHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
+      showing_signin_(false),
       auto_accept_(AUTO_ACCEPT_NONE),
-      source_(SyncPromoUI::SOURCE_UNKNOWN) {
+      source_(SyncPromoUI::SOURCE_UNKNOWN),
+      switched_to_advanced_(false) {
 }
 
 OneClickSigninHelper::~OneClickSigninHelper() {
@@ -840,15 +901,6 @@ void OneClickSigninHelper::ShowInfoBarUIThread(
   if (!helper)
     return;
 
-  // Save the email in the one-click signin manager.  The manager may
-  // not exist if the contents is incognito or if the profile is already
-  // connected to a Google account.
-  if (!session_index.empty())
-    helper->session_index_ = session_index;
-
-  if (!email.empty())
-    helper->email_ = email;
-
   if (auto_accept != AUTO_ACCEPT_NONE) {
     helper->auto_accept_ = auto_accept;
     helper->source_ = source;
@@ -869,6 +921,15 @@ void OneClickSigninHelper::ShowInfoBarUIThread(
     return;
   }
 
+  // Save the email in the one-click signin manager.  The manager may
+  // not exist if the contents is incognito or if the profile is already
+  // connected to a Google account.
+  if (!session_index.empty())
+    helper->session_index_ = session_index;
+
+  if (!email.empty())
+    helper->email_ = email;
+
   if (continue_url.is_valid())
     helper->continue_url_ = continue_url;
 }
@@ -916,10 +977,12 @@ void OneClickSigninHelper::RedirectToSignin() {
 
 void OneClickSigninHelper::CleanTransientState() {
   VLOG(1) << "OneClickSigninHelper::CleanTransientState";
+  showing_signin_ = false;
   email_.clear();
   password_.clear();
   auto_accept_ = AUTO_ACCEPT_NONE;
   source_ = SyncPromoUI::SOURCE_UNKNOWN;
+  switched_to_advanced_ = false;
   continue_url_ = GURL();
 
   // Post to IO thread to clear pending email.
@@ -973,6 +1036,16 @@ void OneClickSigninHelper::DidStopLoading(
     return;
   }
 
+  if (AreWeShowingSignin(url, source_, email_)) {
+    if (!showing_signin_) {
+      if (source_ == SyncPromoUI::SOURCE_UNKNOWN)
+        LogOneClickHistogramValue(one_click_signin::HISTOGRAM_SHOWN);
+      else
+        LogHistogramValue(source_, one_click_signin::HISTOGRAM_SHOWN);
+    }
+    showing_signin_ = true;
+  }
+
   // When Gaia finally redirects to the continue URL, Gaia will add some
   // extra query parameters.  So ignore the parameters when checking to see
   // if the user has continued.
@@ -986,6 +1059,7 @@ void OneClickSigninHelper::DidStopLoading(
 
   // If there is no valid email or password yet, there is nothing to do.
   if (email_.empty() || password_.empty()) {
+    VLOG(1) << "OneClickSigninHelper::DidStopLoading: nothing to do";
     if (continue_url_match_accept)
       RedirectToSignin();
     std::string unused_value;
@@ -1039,8 +1113,10 @@ void OneClickSigninHelper::DidStopLoading(
       SyncPromoUI::Source source =
           SyncPromoUI::GetSourceForSyncPromoURL(url);
       if (source != source_) {
+        advanced_source_ = source_;
         source_ = source;
-        force_same_tab_navigation = source_ == SyncPromoUI::SOURCE_SETTINGS;
+        force_same_tab_navigation = source == SyncPromoUI::SOURCE_SETTINGS;
+        switched_to_advanced_ = source == SyncPromoUI::SOURCE_SETTINGS;
       }
     }
   }
@@ -1063,9 +1139,7 @@ void OneClickSigninHelper::DidStopLoading(
   switch (auto_accept_) {
     case AUTO_ACCEPT_NONE:
       if (SyncPromoUI::UseWebBasedSigninFlow()) {
-        UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse",
-                                  one_click_signin::HISTOGRAM_DISMISSED,
-                                  one_click_signin::HISTOGRAM_MAX);
+        LogOneClickHistogramValue(one_click_signin::HISTOGRAM_DISMISSED);
       } else {
         OneClickInfoBarDelegateImpl::Create(
             InfoBarService::FromWebContents(contents), session_index_, email_,
@@ -1073,22 +1147,37 @@ void OneClickSigninHelper::DidStopLoading(
       }
       break;
     case AUTO_ACCEPT_ACCEPTED:
+      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
+      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_WITH_DEFAULTS);
       SigninManager::DisableOneClickSignIn(profile);
       browser->window()->ShowOneClickSigninBubble(
           bubble_type,
           base::Bind(&StartSync,
                      StartSyncArgs(profile, browser, auto_accept_,
                                    session_index_, email_, password_,
-                                   false /* force_same_tab_navigation */)));
+                                   false /* force_same_tab_navigation */,
+                                   source_)));
       break;
     case AUTO_ACCEPT_CONFIGURE:
+      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
+      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_WITH_ADVANCED);
       SigninManager::DisableOneClickSignIn(profile);
       StartSync(
           StartSyncArgs(profile, browser, auto_accept_, session_index_, email_,
-                        password_, false /* force_same_tab_navigation */),
+                        password_, false /* force_same_tab_navigation */,
+                        source_),
           OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
       break;
     case AUTO_ACCEPT_EXPLICIT: {
+      if (switched_to_advanced_) {
+        LogHistogramValue(advanced_source_,
+                          one_click_signin::HISTOGRAM_WITH_ADVANCED);
+        LogHistogramValue(advanced_source_,
+                          one_click_signin::HISTOGRAM_ACCEPTED);
+      } else {
+        LogHistogramValue(source_, one_click_signin::HISTOGRAM_ACCEPTED);
+        LogHistogramValue(source_, one_click_signin::HISTOGRAM_WITH_DEFAULTS);
+      }
       OneClickSigninSyncStarter::StartSyncMode start_mode =
           source_ == SyncPromoUI::SOURCE_SETTINGS ?
               OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST :
@@ -1107,13 +1196,15 @@ void OneClickSigninHelper::DidStopLoading(
                 &StartExplicitSync,
                 StartSyncArgs(profile, browser, auto_accept_,
                               session_index_, email_, password_,
-                              force_same_tab_navigation),
+                              force_same_tab_navigation,
+                              source_),
                 contents,
                 start_mode));
       } else {
         StartExplicitSync(
             StartSyncArgs(profile, browser, auto_accept_, session_index_,
-                          email_, password_, force_same_tab_navigation),
+                          email_, password_, force_same_tab_navigation,
+                          source_),
             contents,
             start_mode,
             IDS_ONE_CLICK_SIGNIN_CONFIRM_EMAIL_DIALOG_CANCEL_BUTTON);
@@ -1142,9 +1233,7 @@ void OneClickSigninHelper::DidStopLoading(
     }
     case AUTO_ACCEPT_REJECTED_FOR_PROFILE:
       AddEmailToOneClickRejectedList(profile, email_);
-      UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse",
-                                one_click_signin::HISTOGRAM_REJECTED,
-                                one_click_signin::HISTOGRAM_MAX);
+      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_REJECTED);
       break;
     default:
       NOTREACHED() << "Invalid auto_accept=" << auto_accept_;