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

chrome/browser/ui/sync/one_click_signin_helper.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 #include <vector>
@@ skipping 212 matching lines @@
 
 // Add a specific email to the list of emails rejected for one-click
 // sign-in, for this profile.
 void AddEmailToOneClickRejectedList(Profile* profile,
                                     const std::string& email) {
   ListPrefUpdate updater(profile->GetPrefs(),
                          prefs::kReverseAutologinRejectedEmailList);
   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) {
   content::WebContents* web_contents = tab_util::GetWebContentsByID(child_id,
                                                                     route_id);
   if (!web_contents)
     return;
 
   OneClickSigninHelper::RedirectToNtpOrAppsPage(web_contents, source);
 }
 
 // Start syncing with the given user information.
 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;
   }
 
   // The wrapper deletes itself once it's done.
   OneClickSigninHelper::SyncStarterWrapper* wrapper =
       new OneClickSigninHelper::SyncStarterWrapper(args, start_mode);
   wrapper->Start();
 
   int action = one_click_signin::HISTOGRAM_MAX;
   switch (args.auto_accept) {
     case OneClickSigninHelper::AUTO_ACCEPT_EXPLICIT:
       break;
     case OneClickSigninHelper::AUTO_ACCEPT_ACCEPTED:
       action =
           start_mode == OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS ?
               one_click_signin::HISTOGRAM_AUTO_WITH_DEFAULTS :
               one_click_signin::HISTOGRAM_AUTO_WITH_ADVANCED;
       break;
     case OneClickSigninHelper::AUTO_ACCEPT_CONFIGURE:
       DCHECK(start_mode == OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
       action = one_click_signin::HISTOGRAM_AUTO_WITH_ADVANCED;
       break;
     default:
       NOTREACHED() << "Invalid auto_accept: " << args.auto_accept;
       break;
   }
   if (action != one_click_signin::HISTOGRAM_MAX)
-    LogOneClickHistogramValue(action);
+    OneClickSigninHelper::LogHistogramValue(action);
 }
 
 void StartExplicitSync(const OneClickSigninHelper::StartSyncArgs& args,
                        content::WebContents* contents,
                        OneClickSigninSyncStarter::StartSyncMode start_mode,
                        ConfirmEmailDialogDelegate::Action action) {
   bool enable_inline = !switches::IsEnableWebBasedSignin();
   if (action == ConfirmEmailDialogDelegate::START_SYNC) {
     StartSync(args, start_mode);
     if (!enable_inline) {
@@ skipping 396 matching lines @@
   if (client) {
     client->GetPasswordManager()->AddSubmissionCallback(
         base::Bind(&OneClickSigninHelper::PasswordSubmitted,
                    weak_pointer_factory_.GetWeakPtr()));
   }
 }
 
 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,
                                     std::string* error_message) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     VLOG(1) << "OneClickSigninHelper::CanOffer";
 
   if (error_message)
     error_message->clear();
 
@@ skipping 559 matching lines @@
 
     // Redirect to the landing page and display an error popup.
     RedirectToNtpOrAppsPage(web_contents(), source_);
     LoginUIServiceFactory::GetForProfile(profile)->
         DisplayLoginResult(browser, base::UTF8ToUTF16(error_message_));
     CleanTransientState();
     return;
   }
 
   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;
   }
 
   // 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.  Sometimes locales will redirect to a country-
   // specific TLD so just make sure it's a valid domain instead of comparing
   // for an exact match.
   GURL::Replacements replacements;
   replacements.ClearQuery();
@@ skipping 81 matching lines @@
 
   Browser* browser = chrome::FindBrowserWithWebContents(contents);
 
   VLOG(1) << "OneClickSigninHelper::DidStopLoading: signin is go."
           << " auto_accept=" << auto_accept_
           << " source=" << source_;
 
   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.
       if (!do_not_start_sync_for_testing_) {
         StartSync(
             StartSyncArgs(profile, browser, auto_accept_,
                           session_index_, email_, password_, "",
                           NULL  /* don't force sync setup in same tab */,
                           true  /* confirmation_required */, source_,
                           CreateSyncStarterCallback()),
             OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS);
       }
       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.
       if (!do_not_start_sync_for_testing_) {
         StartSync(
             StartSyncArgs(profile, browser, auto_accept_,
                           session_index_, email_, password_, "",
                           NULL  /* don't force sync setup in same tab */,
                           true  /* confirmation_required */, source_,
                           CreateSyncStarterCallback()),
             OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
       }
       break;
     case AUTO_ACCEPT_EXPLICIT: {
       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
       //   default settings.
       // - If sign in was initiated from the settings page for first time sync
       //   set up, show the advanced sync settings dialog.
       // - If sign in was initiated from the settings page due to a re-auth when
       //   sync was already setup, simply navigate back to the settings page.
       ProfileSyncService* sync_service =
           ProfileSyncServiceFactory::GetForProfile(profile);
@@ skipping 33 matching lines @@
       // so that when sign in and sync setup are successful, we can redirect to
       // the correct URL, or auto-close the gaia sign in tab.
       if (original_source == signin::SOURCE_SETTINGS) {
         // The observer deletes itself once it's done.
         new OneClickSigninSyncObserver(contents, original_continue_url_);
       }
       break;
     }
     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_;
       break;
   }
 
   CleanTransientState();
 }
 
 OneClickSigninSyncStarter::Callback
@@ skipping 14 matching lines @@
     // If the web contents is showing a blank page and not about to be closed,
     // redirect to the NTP or apps page.
     if (signin::IsContinueUrlForWebBasedSigninFlow(current_url) &&
         !signin::IsAutoCloseEnabledInURL(original_continue_url_)) {
       RedirectToNtpOrAppsPage(
           web_contents(),
           signin::GetSourceForPromoURL(original_continue_url_));
     }
   }
 }