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

Unified Diff: chrome/browser/ui/chrome_pages.cc

Issue 1473543002: Implement newly designed sign-in related histograms for desktop platorms. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add comments Created 5 years 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
Index: chrome/browser/ui/chrome_pages.cc
diff --git a/chrome/browser/ui/chrome_pages.cc b/chrome/browser/ui/chrome_pages.cc
index 35609ea9824d8d74dc8ef574bc398467dcdab938..9b557a94c7cda7af3a6337e4f474bbcee940c511 100644
--- a/chrome/browser/ui/chrome_pages.cc
+++ b/chrome/browser/ui/chrome_pages.cc
@@ -357,7 +357,8 @@ void ShowSearchEngineSettings(Browser* browser) {
}
#if !defined(OS_ANDROID) && !defined(OS_IOS)
-void ShowBrowserSignin(Browser* browser, signin_metrics::Source source) {
+void ShowBrowserSignin(Browser* browser,
+ signin_metrics::AccessPoint access_point) {
Profile* original_profile = browser->profile()->GetOriginalProfile();
SigninManagerBase* manager =
SigninManagerFactory::GetForProfile(original_profile);
@@ -374,14 +375,13 @@ void ShowBrowserSignin(Browser* browser, signin_metrics::Source source) {
browser = displayer->browser();
}
- signin_metrics::LogSigninSource(source);
-
- // Since the app launcher is a separate application, it might steal focus
+ // Since the extension is a separate application, it might steal focus
// away from Chrome, and accidentally close the avatar bubble. The same will
// happen if we had to switch browser windows to show the sign in page. In
// this case, fallback to the full-tab signin page.
bool show_avatar_bubble =
- source != signin_metrics::SOURCE_APP_LAUNCHER && !switched_browser;
+ access_point != signin_metrics::ACCESS_POINT_EXTENSIONS &&
+ !switched_browser;
#if defined(OS_CHROMEOS)
// ChromeOS doesn't have the avatar bubble.
show_avatar_bubble = false;
@@ -390,16 +390,18 @@ void ShowBrowserSignin(Browser* browser, signin_metrics::Source source) {
if (show_avatar_bubble) {
browser->window()->ShowAvatarBubbleFromAvatarButton(
BrowserWindow::AVATAR_BUBBLE_MODE_SIGNIN,
- signin::ManageAccountsParams());
+ signin::ManageAccountsParams(), access_point);
} else {
- NavigateToSingletonTab(browser, GURL(signin::GetPromoURL(source, false)));
+ NavigateToSingletonTab(
+ browser, GURL(signin::GetPromoURL(
sky 2015/12/03 22:45:28 Doesn't GetPromoURL return a GURL?
gogerald1 2015/12/04 20:49:09 Done. Both the old and new code return a GURL.
+ access_point,
+ signin_metrics::REASON_SIGNIN_PRIMARY_ACCOUNT, false)));
DCHECK_GT(browser->tab_strip_model()->count(), 0);
}
}
-void ShowBrowserSigninOrSettings(
- Browser* browser,
- signin_metrics::Source source) {
+void ShowBrowserSigninOrSettings(Browser* browser,
+ signin_metrics::AccessPoint access_point) {
Profile* original_profile = browser->profile()->GetOriginalProfile();
SigninManagerBase* manager =
SigninManagerFactory::GetForProfile(original_profile);
@@ -407,7 +409,7 @@ void ShowBrowserSigninOrSettings(
if (manager->IsAuthenticated())
ShowSettings(browser);
else
- ShowBrowserSignin(browser, source);
+ ShowBrowserSignin(browser, access_point);
}
#endif

Powered by Google App Engine
This is Rietveld 408576698