Clean up straggler classes to use embedded signin in page in the new profiles world.

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 4d80e2c3965966e71ad1ed54ecf975e5d96eb286..6c26b3d2405eaaf51789ec53f78151eb8ae47a20 100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -230,13 +230,6 @@ void AddEmailToOneClickRejectedList(Profile* profile,
   updater->AppendIfNotPresent(new base::StringValue(email));
 }
 
-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);
-}
-
 void RedirectToNtpOrAppsPageWithIds(int child_id,
                                     int route_id,
                                     signin::Source source) {
@@ -252,7 +245,7 @@ void RedirectToNtpOrAppsPageWithIds(int child_id,
 void StartSync(const OneClickSigninHelper::StartSyncArgs& args,
                OneClickSigninSyncStarter::StartSyncMode start_mode) {
   if (start_mode == OneClickSigninSyncStarter::UNDO_SYNC) {
-    LogOneClickHistogramValue(one_click_signin::HISTOGRAM_UNDO);
+    OneClickSigninHelper::LogHistogramValue(one_click_signin::HISTOGRAM_UNDO);
     return;
   }
 
@@ -280,7 +273,7 @@ void StartSync(const OneClickSigninHelper::StartSyncArgs& args,
       break;
   }
   if (action != one_click_signin::HISTOGRAM_MAX)
-    LogOneClickHistogramValue(action);
+    OneClickSigninHelper::LogHistogramValue(action);
 }
 
 void StartExplicitSync(const OneClickSigninHelper::StartSyncArgs& args,
@@ -697,69 +690,18 @@ OneClickSigninHelper::OneClickSigninHelper(content::WebContents* web_contents)
 OneClickSigninHelper::~OneClickSigninHelper() {}
 
 // static
-void OneClickSigninHelper::LogHistogramValue(
-    signin::Source source, int action) {
-  switch (source) {
-    case signin::SOURCE_START_PAGE:
-      UMA_HISTOGRAM_ENUMERATION("Signin.StartPageActions", action,

[Alexei Svitkine (slow), 2014/12/12 15:48:55]

If you're removing histograms, can you mark them as obsolete in the xml file?

[noms (inactive), 2014/12/12 15:59:29]

These histograms are described with a suffix, and the affected histogram is just
"Signin", which has other histograms that aren't these ones. I wasn't sure how
exactly to do that. (see:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...)

[Alexei Svitkine (slow), 2014/12/12 17:53:05]

In that case, I suggest the following:

Split the histogram_suffixes section into two sections. The first will contain
the entries that are still being logged (e.g. AllAccessPointActions and maybe
AndroidAccountConsistencyFirstRunActions?) while the second section will contain
the ones that have been removed. In the second section, add an <obsolete> tag at
the top.

[noms (inactive), 2014/12/16 16:55:50]

Done.

-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_NTP_LINK:
-      UMA_HISTOGRAM_ENUMERATION("Signin.NTPLinkActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_MENU:
-      UMA_HISTOGRAM_ENUMERATION("Signin.MenuActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_SETTINGS:
-      UMA_HISTOGRAM_ENUMERATION("Signin.SettingsActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_EXTENSION_INSTALL_BUBBLE:
-      UMA_HISTOGRAM_ENUMERATION("Signin.ExtensionInstallBubbleActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_APP_LAUNCHER:
-      UMA_HISTOGRAM_ENUMERATION("Signin.AppLauncherActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_APPS_PAGE_LINK:
-      UMA_HISTOGRAM_ENUMERATION("Signin.AppsPageLinkActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_BOOKMARK_BUBBLE:
-      UMA_HISTOGRAM_ENUMERATION("Signin.BookmarkBubbleActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_AVATAR_BUBBLE_SIGN_IN:
-      UMA_HISTOGRAM_ENUMERATION("Signin.AvatarBubbleActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT:
-      UMA_HISTOGRAM_ENUMERATION("Signin.AvatarBubbleActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_DEVICES_PAGE:
-      UMA_HISTOGRAM_ENUMERATION("Signin.DevicesPageActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    case signin::SOURCE_REAUTH:
-      UMA_HISTOGRAM_ENUMERATION("Signin.ReauthActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-      break;
-    default:
-      // This switch statement needs to be updated when the enum Source changes.
-      COMPILE_ASSERT(signin::SOURCE_UNKNOWN == 12,
-                     kSourceEnumHasChangedButNotThisSwitchStatement);
-      UMA_HISTOGRAM_ENUMERATION("Signin.UnknownActions", action,
-                                one_click_signin::HISTOGRAM_MAX);
-  }
+void OneClickSigninHelper::LogHistogramValue(int action) {
   UMA_HISTOGRAM_ENUMERATION("Signin.AllAccessPointActions", action,
                             one_click_signin::HISTOGRAM_MAX);
 }
 
 // static
+void OneClickSigninHelper::LogHistogramSourceValue(signin::Source source) {
+  UMA_HISTOGRAM_ENUMERATION("Signin.SigninSource", source,
+                            one_click_signin::HISTOGRAM_MAX);
+}
+
+// static
 bool OneClickSigninHelper::CanOffer(content::WebContents* web_contents,
                                     CanOfferFor can_offer_for,
                                     const std::string& email,
@@ -1339,12 +1281,8 @@ void OneClickSigninHelper::DidStopLoading(
   }
 
   if (AreWeShowingSignin(url, source_, email_)) {
-    if (!showing_signin_) {
-      if (source_ == signin::SOURCE_UNKNOWN)
-        LogOneClickHistogramValue(one_click_signin::HISTOGRAM_SHOWN);
-      else
-        LogHistogramValue(source_, one_click_signin::HISTOGRAM_SHOWN);
-    }
+    if (!showing_signin_)
+      LogHistogramValue(one_click_signin::HISTOGRAM_SHOWN);
     showing_signin_ = true;
   }
 
@@ -1446,11 +1384,11 @@ void OneClickSigninHelper::DidStopLoading(
   switch (auto_accept_) {
     case AUTO_ACCEPT_NONE:
       if (showing_signin_)
-        LogOneClickHistogramValue(one_click_signin::HISTOGRAM_DISMISSED);
+        LogHistogramValue(one_click_signin::HISTOGRAM_DISMISSED);
       break;
     case AUTO_ACCEPT_ACCEPTED:
-      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
-      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_WITH_DEFAULTS);
+      LogHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
+      LogHistogramValue(one_click_signin::HISTOGRAM_WITH_DEFAULTS);
       SigninManager::DisableOneClickSignIn(profile->GetPrefs());
       // Start syncing with the default settings - prompt the user to sign in
       // first.
@@ -1465,8 +1403,8 @@ void OneClickSigninHelper::DidStopLoading(
       }
       break;
     case AUTO_ACCEPT_CONFIGURE:
-      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
-      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_WITH_ADVANCED);
+      LogHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
+      LogHistogramValue(one_click_signin::HISTOGRAM_WITH_ADVANCED);
       SigninManager::DisableOneClickSignIn(profile->GetPrefs());
       // Display the extra confirmation (even in the SAML case) in case this
       // was an untrusted renderer.
@@ -1484,13 +1422,11 @@ void OneClickSigninHelper::DidStopLoading(
       signin::Source original_source =
           signin::GetSourceForPromoURL(original_continue_url_);
       if (switched_to_advanced_) {
-        LogHistogramValue(original_source,
-                          one_click_signin::HISTOGRAM_WITH_ADVANCED);
-        LogHistogramValue(original_source,
-                          one_click_signin::HISTOGRAM_ACCEPTED);
+        LogHistogramValue(one_click_signin::HISTOGRAM_WITH_ADVANCED);
+        LogHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
       } else {
-        LogHistogramValue(source_, one_click_signin::HISTOGRAM_ACCEPTED);
-        LogHistogramValue(source_, one_click_signin::HISTOGRAM_WITH_DEFAULTS);
+        LogHistogramValue(one_click_signin::HISTOGRAM_ACCEPTED);
+        LogHistogramValue(one_click_signin::HISTOGRAM_WITH_DEFAULTS);
       }
 
       // - If sign in was initiated from the NTP or the hotdog menu, sync with
@@ -1544,7 +1480,7 @@ void OneClickSigninHelper::DidStopLoading(
     }
     case AUTO_ACCEPT_REJECTED_FOR_PROFILE:
       AddEmailToOneClickRejectedList(profile, email_);
-      LogOneClickHistogramValue(one_click_signin::HISTOGRAM_REJECTED);
+      LogHistogramValue(one_click_signin::HISTOGRAM_REJECTED);
       break;
     default:
       NOTREACHED() << "Invalid auto_accept=" << auto_accept_;