Commit 96570d47 authored by Mihai Sardarescu's avatar Mihai Sardarescu Committed by Commit Bot

[dice] Log Log Signin.SigninStartedAccessPoint.* histograms for Settings

This CL passes the information that the default promo account was selected
from the settings WebUI when the user attempts to enable sync. This leads
to the right histograms Signin.SigninStartedAccessPoint.{NewAccount|NotDefault|WithDefault}
when sync is enabled from settings.

This CL is a follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/986141

Bug: 819431
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I55c7390819ace71ae6cb937248d2d425a5a0396f
Reviewed-on: https://chromium-review.googlesource.com/999420
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarScott Chen <scottchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549510}
parent bf5b0e09
...@@ -194,7 +194,12 @@ Polymer({ ...@@ -194,7 +194,12 @@ Polymer({
/** @private */ /** @private */
onSyncButtonTap_: function() { onSyncButtonTap_: function() {
assert(this.shownAccount_); assert(this.shownAccount_);
this.syncBrowserProxy_.startSyncingWithEmail(this.shownAccount_.email); assert(this.storedAccounts_.length > 0);
const isDefaultPromoAccount =
(this.shownAccount_.email == this.storedAccounts_[0].email);
this.syncBrowserProxy_.startSyncingWithEmail(
this.shownAccount_.email, isDefaultPromoAccount);
}, },
/** @private */ /** @private */
...@@ -244,4 +249,4 @@ Polymer({ ...@@ -244,4 +249,4 @@ Polymer({
this.storedAccounts_ ? this.storedAccounts_[0] : null; this.storedAccounts_ ? this.storedAccounts_[0] : null;
} }
} }
}); });
\ No newline at end of file
...@@ -207,9 +207,12 @@ cr.define('settings', function() { ...@@ -207,9 +207,12 @@ cr.define('settings', function() {
/** /**
* Start syncing with an account, specified by its email. * Start syncing with an account, specified by its email.
* |isDefaultPromoAccount| is true if |email| is the email of the default
* account displayed in the promo.
* @param {string} email * @param {string} email
* @param {boolean} isDefaultPromoAccount
*/ */
startSyncingWithEmail(email) {} startSyncingWithEmail(email, isDefaultPromoAccount) {}
/** /**
* Opens the Google Activity Controls url in a new tab. * Opens the Google Activity Controls url in a new tab.
...@@ -297,8 +300,9 @@ cr.define('settings', function() { ...@@ -297,8 +300,9 @@ cr.define('settings', function() {
} }
/** @override */ /** @override */
startSyncingWithEmail(email) { startSyncingWithEmail(email, isDefaultPromoAccount) {
chrome.send('SyncSetupStartSyncingWithEmail', [email]); chrome.send(
'SyncSetupStartSyncingWithEmail', [email, isDefaultPromoAccount]);
} }
/** @override */ /** @override */
......
...@@ -146,7 +146,7 @@ void EnableSyncFromPromo( ...@@ -146,7 +146,7 @@ void EnableSyncFromPromo(
// It looks like on ChromeOS there are tests that expect that the Chrome // It looks like on ChromeOS there are tests that expect that the Chrome
// sign-in tab is presented even thought the user is signed in to Chrome // sign-in tab is presented even thought the user is signed in to Chrome
// (e.g. BookmarkBubbleSignInDelegateTest.*). However signing in to Chrome in // (e.g. BookmarkBubbleSignInDelegateTest.*). However signing in to Chrome in
// a refular profile is not supported on ChromeOS as the primary account is // a regular profile is not supported on ChromeOS as the primary account is
// set when the profile is created. // set when the profile is created.
// //
// TODO(msarda): Investigate whether this flow needs to be supported on // TODO(msarda): Investigate whether this flow needs to be supported on
......
...@@ -532,7 +532,9 @@ std::unique_ptr<base::ListValue> PeopleHandler::GetStoredAccountsList() { ...@@ -532,7 +532,9 @@ std::unique_ptr<base::ListValue> PeopleHandler::GetStoredAccountsList() {
void PeopleHandler::HandleStartSyncingWithEmail(const base::ListValue* args) { void PeopleHandler::HandleStartSyncingWithEmail(const base::ListValue* args) {
const base::Value* email; const base::Value* email;
const base::Value* is_default_promo_account;
CHECK(args->Get(0, &email)); CHECK(args->Get(0, &email));
CHECK(args->Get(1, &is_default_promo_account));
Browser* browser = Browser* browser =
chrome::FindBrowserWithWebContents(web_ui()->GetWebContents()); chrome::FindBrowserWithWebContents(web_ui()->GetWebContents());
...@@ -541,12 +543,9 @@ void PeopleHandler::HandleStartSyncingWithEmail(const base::ListValue* args) { ...@@ -541,12 +543,9 @@ void PeopleHandler::HandleStartSyncingWithEmail(const base::ListValue* args) {
AccountTrackerServiceFactory::GetForProfile(profile_); AccountTrackerServiceFactory::GetForProfile(profile_);
AccountInfo account = AccountInfo account =
account_tracker->FindAccountInfoByEmail(email->GetString()); account_tracker->FindAccountInfoByEmail(email->GetString());
// TODO(http://crbug.com/819431): Pass the right is_default_promo_account bit
// from the settings web_ui.
signin_ui_util::EnableSyncFromPromo( signin_ui_util::EnableSyncFromPromo(
browser, account, signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS, browser, account, signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS,
false /* is_default_promo_account */); is_default_promo_account->GetBool());
} }
#endif #endif
......
...@@ -150,8 +150,12 @@ cr.define('settings_sync_account_control', function() { ...@@ -150,8 +150,12 @@ cr.define('settings_sync_account_control', function() {
Polymer.dom.flush(); Polymer.dom.flush();
return browserProxy.whenCalled('startSyncingWithEmail') return browserProxy.whenCalled('startSyncingWithEmail')
.then(email => { .then((args) => {
const email = args[0];
const isDefaultPromoAccount = args[1];
assertEquals(email, 'foo@foo.com'); assertEquals(email, 'foo@foo.com');
assertEquals(isDefaultPromoAccount, true);
assertVisible(testElement.$$('paper-icon-button-light'), true); assertVisible(testElement.$$('paper-icon-button-light'), true);
assertTrue(testElement.$$('#sync-icon-container').hidden); assertTrue(testElement.$$('#sync-icon-container').hidden);
...@@ -178,8 +182,11 @@ cr.define('settings_sync_account_control', function() { ...@@ -178,8 +182,11 @@ cr.define('settings_sync_account_control', function() {
return browserProxy.whenCalled('startSyncingWithEmail'); return browserProxy.whenCalled('startSyncingWithEmail');
}) })
.then(email => { .then((args) => {
const email = args[0];
const isDefaultPromoAccount = args[1];
assertEquals(email, 'bar@bar.com'); assertEquals(email, 'bar@bar.com');
assertEquals(isDefaultPromoAccount, false);
// Tapping the last menu item will initiate sign-in. // Tapping the last menu item will initiate sign-in.
items[2].click(); items[2].click();
......
...@@ -53,8 +53,8 @@ class TestSyncBrowserProxy extends TestBrowserProxy { ...@@ -53,8 +53,8 @@ class TestSyncBrowserProxy extends TestBrowserProxy {
} }
/** @override */ /** @override */
startSyncingWithEmail(email) { startSyncingWithEmail(email, isDefaultPromoAccount) {
this.methodCalled('startSyncingWithEmail', email); this.methodCalled('startSyncingWithEmail', [email, isDefaultPromoAccount]);
} }
setImpressionCount(count) { setImpressionCount(count) {
...@@ -99,4 +99,4 @@ class TestSyncBrowserProxy extends TestBrowserProxy { ...@@ -99,4 +99,4 @@ class TestSyncBrowserProxy extends TestBrowserProxy {
this.methodCalled('setSyncEverything', syncEverything); this.methodCalled('setSyncEverything', syncEverything);
return Promise.resolve(settings.PageStatus.CONFIGURE); return Promise.resolve(settings.PageStatus.CONFIGURE);
} }
} }
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment