Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6188)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/ui/sync/one_click_signin_helper.cc

Issue 12362002: Expanding signin histogram coverage. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 7 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/ui/sync/one_click_signin_helper.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..2cb7c8f8724218ecba9fefbdc14da1e15a007d25 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 {
@@ -123,9 +168,7 @@ struct StartSyncArgs {
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.
@@ -136,20 +179,15 @@ void StartSync(const StartSyncArgs& args,
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_ACCEPTED:
action = one_click_signin::HISTOGRAM_AUTO_WITH_DEFAULTS;
break;
- case OneClickSigninHelper::AUTO_ACCEPT_NONE:
- action =
- start_mode == OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS ?
- one_click_signin::HISTOGRAM_WITH_DEFAULTS :
- one_click_signin::HISTOGRAM_WITH_ADVANCED;
- break;
case OneClickSigninHelper::AUTO_ACCEPT_CONFIGURE:
DCHECK(start_mode == OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
action = one_click_signin::HISTOGRAM_AUTO_WITH_ADVANCED;
@@ -158,9 +196,8 @@ void StartSync(const StartSyncArgs& args,
NOTREACHED() << "Invalid auto_accept: " << args.auto_accept;
break;
}
-
- UMA_HISTOGRAM_ENUMERATION("AutoLogin.Reverse", action,
- one_click_signin::HISTOGRAM_MAX);
+ if (args.auto_accept != OneClickSigninHelper::AUTO_ACCEPT_EXPLICIT)
+ LogOneClickHistogramValue(action);
Roger Tawa OOO till Jul 10th 2013/02/28 20:54:27 Nit: would it be better to check: action !=
jwd 2013/02/28 21:01:49 Ya, that's nicer. Done.
}
void StartExplicitSync(const StartSyncArgs& args,
@@ -327,6 +364,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.
@@ -502,8 +553,11 @@ 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),
+ original_source_(SyncPromoUI::SOURCE_UNKNOWN) {
}
OneClickSigninHelper::~OneClickSigninHelper() {
@@ -840,15 +894,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 +914,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 +970,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;
Roger Tawa OOO till Jul 10th 2013/02/28 20:54:27 Set original_source_ back to unknown.
jwd 2013/02/28 21:01:49 Done.
continue_url_ = GURL();
// Post to IO thread to clear pending email.
@@ -973,6 +1029,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 +1052,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 +1106,10 @@ void OneClickSigninHelper::DidStopLoading(
SyncPromoUI::Source source =
SyncPromoUI::GetSourceForSyncPromoURL(url);
if (source != source_) {
+ original_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 +1132,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,6 +1140,8 @@ 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,
@@ -1082,6 +1151,8 @@ void OneClickSigninHelper::DidStopLoading(
false /* force_same_tab_navigation */)));
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_,
@@ -1089,6 +1160,15 @@ void OneClickSigninHelper::DidStopLoading(
OneClickSigninSyncStarter::CONFIGURE_SYNC_FIRST);
break;
case AUTO_ACCEPT_EXPLICIT: {
+ if (switched_to_advanced_) {
+ LogHistogramValue(original_source_,
+ one_click_signin::HISTOGRAM_WITH_ADVANCED);
+ LogHistogramValue(original_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 :
@@ -1142,9 +1222,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_;
« no previous file with comments | « chrome/browser/ui/sync/one_click_signin_helper.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698