Skip to content

GitLab

T
tangled
Commit 8419c0a4 authored by David Roger's avatar David Roger Committed by Commit Bot
Browse files
Options

[settings] Cleanup shouldShowProfile_ and related code 

This CL removes a reference to diceEnabled in shouldShowProfile_.
Additionally it removes dead code: on desktop this branch is only used
when signin is disallowed and the user is signed out.

The tests related to the signout button are converted to use the Dice
flow.

Bug: 891781
Change-Id: I227bdbce52c38c24920433d6b2c8d21a4c6f3bd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015247
Commit-Queue: David Roger <droger@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735370}
parent 463b9cdd
Hide whitespace changes
Inline Side-by-side
Showing with 166 additions and 223 deletions
chrome/app/settings_chromium_strings.grdp
View file @ 8419c0a4
......@@ -107,9 +107,6 @@
</message>
<!-- Sync Page -->
<message name="IDS_SETTINGS_SYNC_SIGNIN" desc="The label that appears on the sync button in the options dialog when sync has not been set up by the user.">
Sign in to Chromium
</message>
<message name="IDS_SETTINGS_SYNC_DATA_ENCRYPTED_TEXT" desc="Text alerting the user that synced data is encrypted.">
For added security, Chromium will encrypt your data.
</message>
......
chrome/app/settings_google_chrome_strings.grdp
View file @ 8419c0a4
......@@ -107,9 +107,6 @@
</message>
<!-- Sync Page -->
<message name="IDS_SETTINGS_SYNC_SIGNIN" desc="The label that appears on the sync button in the options dialog when sync has not been set up by the user.">
Sign in to Chrome
</message>
<message name="IDS_SETTINGS_SYNC_DATA_ENCRYPTED_TEXT" desc="Text alerting the user that synced data is encrypted.">
For added security, Google Chrome will encrypt your data
</message>
......
chrome/browser/resources/settings/people_page/people_page.html
View file @ 8419c0a4
......@@ -114,7 +114,7 @@
"$i18n{peopleSignInPromptSecondaryWithNoAccount}">
</settings-sync-account-control>
</template>
<template is="dom-if" if="[[shouldShowProfile_(
<template is="dom-if" if="[[!shouldShowSyncAccountControl_(
syncStatus.syncSystemEnabled, syncStatus.signinAllowed)]]" restamp>
<div id="picture-subpage-trigger" class="settings-box first two-line">
<template is="dom-if" if="[[syncStatus]]">
......@@ -129,9 +129,13 @@
actionable$="[[isProfileActionable_]]">
<div class="flex text-elide settings-box-text">
<span id="profile-name">[[profileName_]]</span>
<!-- When the user is signed-in, the settings-sync-account-control is always
shown on non-ChromeOS platforms -->
<if expr="chromeos">
<div class="secondary" hidden="[[!syncStatus.signedIn]]">
[[syncStatus.signedInUsername]]
</div>
</if>
</div>
<if expr="not chromeos">
<cr-icon-button class="subpage-arrow"
......@@ -148,26 +152,9 @@
aria-describedby="profile-name"></cr-icon-button>
</if>
</div>
<!-- Chrome OS is always signed-in and does not show a sign-out button. -->
<if expr="not chromeos">
<template is="dom-if" if="[[showSignin_(syncStatus)]]">
<div class="separator"></div>
<cr-button class="action-button" on-click="onSigninTap_"
disabled="[[syncStatus.firstSetupInProgress]]">
$i18n{syncSignin}
</cr-button>
</template>
<template is="dom-if" if="[[syncStatus.signedIn]]">
<div class="separator"></div>
<cr-button id="disconnectButton" on-click="onDisconnectTap_"
disabled="[[syncStatus.firstSetupInProgress]]">
$i18n{syncDisconnect}
</cr-button>
</template>
</if>
</template>
</div>
</template> <!-- if="[[shouldShowProfile_()]]" -->
</template> <!-- if="[[!shouldShowSyncAccountControl_()]]" -->
<!-- Chrome OS uses settings-sync-account-control for sync promos. -->
<if expr="not chromeos">
......
chrome/browser/resources/settings/people_page/people_page.js
View file @ 8419c0a4
......@@ -275,8 +275,8 @@ Polymer({
// Sign-in impressions should be recorded only if the sign-in promo is
// shown. They should be recorder only once, the first time
// |this.syncStatus| is set.
const shouldRecordSigninImpression =
!this.syncStatus && syncStatus && this.showSignin_(syncStatus);
const shouldRecordSigninImpression = !this.syncStatus && syncStatus &&
!!syncStatus.signinAllowed && !syncStatus.signedIn;
this.syncStatus = syncStatus;
......@@ -315,20 +315,9 @@ Polymer({
// </if>
},
/** @private */
onSigninTap_() {
this.syncBrowserProxy_.startSignIn();
},
/** @private */
onDisconnectDialogClosed_(e) {
this.showSignoutDialog_ = false;
// <if expr="not chromeos">
if (!this.diceEnabled_) {
// If DICE-enabled, this button won't exist here.
cr.ui.focusWithoutInk(assert(this.$$('#disconnectButton')));
}
// </if>
if (settings.Router.getInstance().getCurrentRoute() ==
settings.routes.SIGN_OUT) {
......@@ -336,11 +325,6 @@ Polymer({
}
},
/** @private */
onDisconnectTap_() {
settings.Router.getInstance().navigateTo(settings.routes.SIGN_OUT);
},
/** @private */
onSyncTap_() {
// Users can go to sync subpage regardless of sync status.
......@@ -387,22 +371,6 @@ Polymer({
!!this.syncStatus.signinAllowed;
},
/**
* @return {boolean} Whether to show the profile row and associated controls.
* @private
*/
shouldShowProfile_() {
// Closure compiler doesn't understand <if> so use a variable.
let show = false;
// <if expr="chromeos">
show = !this.shouldShowSyncAccountControl_();
// </if>
// <if expr="not chromeos">
show = !this.diceEnabled_;
// </if>
return show;
},
/**
* @param {string} iconUrl
* @return {string} A CSS image-set for multiple scale factors.
......@@ -411,13 +379,4 @@ Polymer({
getIconImageSet_(iconUrl) {
return cr.icon.getImage(iconUrl);
},
/**
* @param {!settings.SyncStatus} syncStatus
* @return {boolean} Whether to show the "Sign in to Chrome" button.
* @private
*/
showSignin_(syncStatus) {
return !!syncStatus.signinAllowed && !syncStatus.signedIn;
},
});
chrome/browser/ui/webui/settings/settings_localized_strings_provider.cc
View file @ 8419c0a4
......@@ -1960,7 +1960,6 @@ void AddPeopleStrings(content::WebUIDataSource* html_source, Profile* profile) {
{"syncOverview", IDS_SETTINGS_SYNC_OVERVIEW},
{"syncDisabled", IDS_PROFILES_DICE_SYNC_DISABLED_TITLE},
{"syncDisabledByAdministrator", IDS_SIGNED_IN_WITH_SYNC_DISABLED_BY_POLICY},
{"syncSignin", IDS_SETTINGS_SYNC_SIGNIN},
{"syncDisconnect", IDS_SETTINGS_PEOPLE_SIGN_OUT},
{"syncDisconnectTitle", IDS_SETTINGS_SYNC_DISCONNECT_TITLE},
{"syncDisconnectDeleteProfile",
......
chrome/test/data/webui/settings/cr_settings_browsertest.js
View file @ 8419c0a4
......@@ -455,6 +455,7 @@ CrSettingsPeoplePageTest.prototype = {
/** @override */
extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
'../test_browser_proxy.js',
'../test_util.js',
'sync_test_util.js',
'test_profile_info_browser_proxy.js',
'test_sync_browser_proxy.js',
......
chrome/test/data/webui/settings/people_page_test.js
View file @ 8419c0a4
......@@ -112,19 +112,146 @@ cr.define('settings_people_page', function() {
test('NoManageProfileRow', function() {
assertFalse(!!peoplePage.$$('#edit-profile'));
});
});
test('GetProfileInfo', function() {
let disconnectButton = null;
return syncBrowserProxy.whenCalled('getSyncStatus')
.then(function() {
Polymer.dom.flush();
disconnectButton = peoplePage.$$('#disconnectButton');
assertTrue(!!disconnectButton);
assertFalse(!!peoplePage.$$('settings-signout-dialog'));
suite('DiceUITest', function() {
suiteSetup(function() {
// Force UIs to think DICE is enabled for this profile.
loadTimeData.overrideValues({
diceEnabled: true,
});
});
disconnectButton.click();
Polymer.dom.flush();
})
setup(function() {
syncBrowserProxy = new TestSyncBrowserProxy();
settings.SyncBrowserProxyImpl.instance_ = syncBrowserProxy;
profileInfoBrowserProxy = new TestProfileInfoBrowserProxy();
settings.ProfileInfoBrowserProxyImpl.instance_ =
profileInfoBrowserProxy;
PolymerTest.clearBody();
peoplePage = document.createElement('settings-people-page');
peoplePage.pageVisibility = settings.pageVisibility;
document.body.appendChild(peoplePage);
Polymer.dom.flush();
});
teardown(function() {
peoplePage.remove();
});
test('ShowCorrectRows', function() {
return syncBrowserProxy.whenCalled('getSyncStatus').then(function() {
// The correct /manageProfile link row is shown.
assertTrue(!!peoplePage.$$('#edit-profile'));
assertFalse(!!peoplePage.$$('#picture-subpage-trigger'));
// Sync-overview row should not exist when diceEnabled is true, even
// if syncStatus values would've warranted the row otherwise.
sync_test_util.simulateSyncStatus({
signedIn: false,
signinAllowed: true,
syncSystemEnabled: true,
});
assertFalse(!!peoplePage.$$('#sync-overview'));
// The control element should exist when policy allows.
const accountControl = peoplePage.$$('settings-sync-account-control');
assertTrue(
window.getComputedStyle(accountControl)['display'] != 'none');
// Control element doesn't exist when policy forbids sync or sign-in.
sync_test_util.simulateSyncStatus({
signinAllowed: false,
syncSystemEnabled: true,
});
assertEquals(
'none', window.getComputedStyle(accountControl)['display']);
sync_test_util.simulateSyncStatus({
signinAllowed: true,
syncSystemEnabled: false,
});
assertEquals(
'none', window.getComputedStyle(accountControl)['display']);
const manageGoogleAccount = peoplePage.$$('#manage-google-account');
// Do not show Google Account when stored accounts or sync status
// could not be retrieved.
sync_test_util.simulateStoredAccounts(undefined);
sync_test_util.simulateSyncStatus(undefined);
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts([]);
sync_test_util.simulateSyncStatus(undefined);
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts(undefined);
sync_test_util.simulateSyncStatus({});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts([]);
sync_test_util.simulateSyncStatus({});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
// A stored account with sync off but no error should result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: false,
hasError: false,
});
assertTrue(
window.getComputedStyle(manageGoogleAccount)['display'] !=
'none');
// A stored account with sync off and error should not result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: false,
hasError: true,
});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
// A stored account with sync on but no error should result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: true,
hasError: false,
});
assertTrue(
window.getComputedStyle(manageGoogleAccount)['display'] !=
'none');
// A stored account with sync on but with error should not result in
// the Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: true,
hasError: true,
});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
});
});
test('SignOutNavigationNormalProfile', function() {
// Navigate to chrome://settings/signOut
settings.Router.getInstance().navigateTo(settings.routes.SIGN_OUT);
return new Promise(function(resolve) {
peoplePage.async(resolve);
})
.then(function() {
const signoutDialog = peoplePage.$$('settings-signout-dialog');
assertTrue(signoutDialog.$$('#dialog').open);
......@@ -151,14 +278,27 @@ cr.define('settings_people_page', function() {
})
.then(function(deleteProfile) {
assertFalse(deleteProfile);
});
});
test('SignOutDialogManagedProfile', function() {
let accountControl = null;
return syncBrowserProxy.whenCalled('getSyncStatus')
.then(function() {
sync_test_util.simulateSyncStatus({
signedIn: true,
domain: 'example.com',
signinAllowed: true,
syncSystemEnabled: true,
});
assertFalse(!!peoplePage.$$('#dialog'));
disconnectButton.click();
accountControl = peoplePage.$$('settings-sync-account-control');
return test_util.waitBeforeNextRender(accountControl);
})
.then(function() {
const turnOffButton = accountControl.$$('#turn-off');
turnOffButton.click();
Polymer.dom.flush();
return new Promise(function(resolve) {
......@@ -194,17 +334,12 @@ cr.define('settings_people_page', function() {
});
test('getProfileStatsCount', function() {
return syncBrowserProxy.whenCalled('getSyncStatus')
.then(function() {
Polymer.dom.flush();
// Open the disconnect dialog.
disconnectButton = peoplePage.$$('#disconnectButton');
assertTrue(!!disconnectButton);
disconnectButton.click();
// Navigate to chrome://settings/signOut
settings.Router.getInstance().navigateTo(settings.routes.SIGN_OUT);
return profileInfoBrowserProxy.whenCalled('getProfileStatsCount');
})
return new Promise(function(resolve) {
peoplePage.async(resolve);
})
.then(function() {
Polymer.dom.flush();
const signoutDialog = peoplePage.$$('settings-signout-dialog');
......@@ -306,138 +441,6 @@ cr.define('settings_people_page', function() {
});
});
});
suite('DiceUITest', function() {
suiteSetup(function() {
// Force UIs to think DICE is enabled for this profile.
loadTimeData.overrideValues({
diceEnabled: true,
});
});
setup(function() {
syncBrowserProxy = new TestSyncBrowserProxy();
settings.SyncBrowserProxyImpl.instance_ = syncBrowserProxy;
profileInfoBrowserProxy = new TestProfileInfoBrowserProxy();
settings.ProfileInfoBrowserProxyImpl.instance_ =
profileInfoBrowserProxy;
PolymerTest.clearBody();
peoplePage = document.createElement('settings-people-page');
peoplePage.pageVisibility = settings.pageVisibility;
document.body.appendChild(peoplePage);
Polymer.dom.flush();
});
teardown(function() {
peoplePage.remove();
});
test('ShowCorrectRows', function() {
return syncBrowserProxy.whenCalled('getSyncStatus').then(function() {
// The correct /manageProfile link row is shown.
assertTrue(!!peoplePage.$$('#edit-profile'));
assertFalse(!!peoplePage.$$('#picture-subpage-trigger'));
// Sync-overview row should not exist when diceEnabled is true, even
// if syncStatus values would've warranted the row otherwise.
sync_test_util.simulateSyncStatus({
signedIn: false,
signinAllowed: true,
syncSystemEnabled: true,
});
assertFalse(!!peoplePage.$$('#sync-overview'));
// The control element should exist when policy allows.
const accountControl = peoplePage.$$('settings-sync-account-control');
assertTrue(
window.getComputedStyle(accountControl)['display'] != 'none');
// Control element doesn't exist when policy forbids sync or sign-in.
sync_test_util.simulateSyncStatus({
signinAllowed: false,
syncSystemEnabled: true,
});
assertEquals(
'none', window.getComputedStyle(accountControl)['display']);
sync_test_util.simulateSyncStatus({
signinAllowed: true,
syncSystemEnabled: false,
});
assertEquals(
'none', window.getComputedStyle(accountControl)['display']);
const manageGoogleAccount = peoplePage.$$('#manage-google-account');
// Do not show Google Account when stored accounts or sync status
// could not be retrieved.
sync_test_util.simulateStoredAccounts(undefined);
sync_test_util.simulateSyncStatus(undefined);
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts([]);
sync_test_util.simulateSyncStatus(undefined);
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts(undefined);
sync_test_util.simulateSyncStatus({});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
sync_test_util.simulateStoredAccounts([]);
sync_test_util.simulateSyncStatus({});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
// A stored account with sync off but no error should result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: false,
hasError: false,
});
assertTrue(
window.getComputedStyle(manageGoogleAccount)['display'] !=
'none');
// A stored account with sync off and error should not result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: false,
hasError: true,
});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
// A stored account with sync on but no error should result in the
// Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: true,
hasError: false,
});
assertTrue(
window.getComputedStyle(manageGoogleAccount)['display'] !=
'none');
// A stored account with sync on but with error should not result in
// the Google Account being shown.
sync_test_util.simulateStoredAccounts([{email: 'foo@foo.com'}]);
sync_test_util.simulateSyncStatus({
signedIn: true,
hasError: true,
});
assertEquals(
'none', window.getComputedStyle(manageGoogleAccount)['display']);
});
});
});
}
suite('SyncSettings', function() {
......
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