Skip to content

Commit

Permalink
Decouple Autofill and Payments sync toggles on Desktop.
Browse files Browse the repository at this point in the history
The "Payment methods" toggle gets automatically disabled/enabled
when "Addresses and more" is disabled/enabled. This CL removes
this logic, making the toggles independent.

Bug: 1435431
Change-Id: Ifffbac0edc43655003df71a2639e087489d37f1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4941915
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Commit-Queue: Dmitry Vykochko <vykochko@google.com>
Cr-Commit-Position: refs/heads/main@{#1211345}
  • Loading branch information
DVykochko authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent e799086 commit f3567e9
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ <h2 class="cr-title-text flex">$i18n{syncData}</h2>
<cr-policy-indicator indicator-type="userPolicy"
hidden="[[!syncPrefs.autofillManaged]]">
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.autofillSynced}}"
<cr-toggle id="autofillCheckbox"
checked="{{syncPrefs.autofillSynced}}"
on-change="onAutofillDataTypeChanged_"
disabled="[[disableTypeCheckBox_(
syncPrefs.syncAllDataTypes,syncPrefs.autofillManaged)]]"
Expand All @@ -249,7 +250,8 @@ <h2 class="cr-title-text flex">$i18n{syncData}</h2>
<cr-policy-indicator indicator-type="userPolicy"
hidden="[[!syncPrefs.paymentsManaged]]">
</cr-policy-indicator>
<cr-toggle checked="{{syncPrefs.paymentsSynced}}"
<cr-toggle id="paymentsCheckbox"
checked="{{syncPrefs.paymentsSynced}}"
on-change="onSingleSyncDataTypeChanged_"
disabled="[[disablePaymentsCheckbox_(
syncPrefs.syncAllDataTypes, syncPrefs.autofillSynced,
Expand Down
34 changes: 25 additions & 9 deletions chrome/browser/resources/settings/people_page/sync_controls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ import {OpenWindowProxyImpl} from 'chrome://resources/js/open_window_proxy.js';

// </if>

// <if expr="is_chromeos">
import {loadTimeData} from '../i18n_setup.js';
// </if>

import {Route, Router} from '../router.js';

import {getTemplate} from './sync_controls.html.js';
Expand All @@ -39,6 +36,9 @@ enum RadioButtonNames {
CUSTOMIZE_SYNC = 'customize-sync',
}

const SYNC_DECOUPLE_ADDRESS_PAYMENT_SETTINGS_FEATURE: string =
'syncDecoupleAddressPaymentSettings';

/**
* @fileoverview
* 'settings-sync-controls' contains all sync data type controls.
Expand Down Expand Up @@ -138,7 +138,10 @@ export class SettingsSyncControlsElement extends

// If autofill is not registered or synced, force Payments integration off.
// TODO(crbug.com/1435431): Remove this coupling.
if (!this.syncPrefs.autofillRegistered || !this.syncPrefs.autofillSynced) {
if (!loadTimeData.getBoolean(
SYNC_DECOUPLE_ADDRESS_PAYMENT_SETTINGS_FEATURE) &&
(!this.syncPrefs.autofillRegistered ||
!this.syncPrefs.autofillSynced)) {
this.set('syncPrefs.paymentsSynced', false);
}
}
Expand Down Expand Up @@ -211,24 +214,37 @@ export class SettingsSyncControlsElement extends
* Handler for when the autofill data type checkbox is changed.
*/
private onAutofillDataTypeChanged_() {
// TODO(crbug.com/1435431): Remove this coupling.
this.set('syncPrefs.paymentsSynced', this.syncPrefs!.autofillSynced);
if (!loadTimeData.getBoolean(
SYNC_DECOUPLE_ADDRESS_PAYMENT_SETTINGS_FEATURE)) {
// TODO(crbug.com/1435431): Remove this coupling.
this.set('syncPrefs.paymentsSynced', this.syncPrefs!.autofillSynced);
}

this.onSingleSyncDataTypeChanged_();
}

// TODO(crbug.com/1435431): Remove this coupling.
private shouldPaymentsCheckboxBeHidden_(
paymentsRegistered: boolean, autofillRegistered: boolean): boolean {
return !paymentsRegistered || !autofillRegistered;
if (loadTimeData.getBoolean(
SYNC_DECOUPLE_ADDRESS_PAYMENT_SETTINGS_FEATURE)) {
return !paymentsRegistered;
} else {
return !paymentsRegistered || !autofillRegistered;
}
}

// TODO(crbug.com/1435431): Remove this coupling.
private disablePaymentsCheckbox_(
syncAllDataTypes: boolean, autofillSynced: boolean,
autofillManaged: boolean, paymentsManaged: boolean): boolean {
return syncAllDataTypes || !autofillSynced || autofillManaged ||
paymentsManaged;
if (loadTimeData.getBoolean(
SYNC_DECOUPLE_ADDRESS_PAYMENT_SETTINGS_FEATURE)) {
return this.disableTypeCheckBox_(syncAllDataTypes, paymentsManaged);
} else {
return this.disableTypeCheckBox_(syncAllDataTypes, paymentsManaged) ||
!autofillSynced || autofillManaged;
}
}

private disableTypeCheckBox_(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,10 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,

html_source->AddString("plusAddressManagementUrl",
plus_addresses::kPlusAddressManagementUrl.Get());

html_source->AddBoolean("syncDecoupleAddressPaymentSettings",
base::FeatureList::IsEnabled(
syncer::kSyncDecoupleAddressPaymentSettings));
}

void AddSignOutDialogStrings(content::WebUIDataSource* html_source,
Expand Down
93 changes: 90 additions & 3 deletions chrome/test/data/webui/settings/people_page_sync_controls_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ import {isVisible} from 'chrome://webui-test/test_util.js';

import {getSyncAllPrefs, getSyncAllPrefsManaged, setupRouterWithSyncRoutes, SyncRoutes} from './sync_test_util.js';
import {TestSyncBrowserProxy} from './test_sync_browser_proxy.js';

// <if expr="chromeos_lacros">
import {loadTimeData} from 'chrome://settings/settings.js';
// </if>

// clang-format on

Expand Down Expand Up @@ -326,3 +323,93 @@ suite('SyncControlsManagedTest', async function() {
browserProxy.resetResolver('setSyncDatatypes');
});
});

suite('AutofillAndPaymentsToggles', async function() {
let autofillCheckbox: CrToggleElement;
let paymentsCheckbox: CrToggleElement;

setup(async function() {
setupRouterWithSyncRoutes();
const browserProxy = new TestSyncBrowserProxy();
SyncBrowserProxyImpl.setInstance(browserProxy);

document.body.innerHTML = window.trustedTypes!.emptyHTML;
const syncControls = document.createElement('settings-sync-controls');
document.body.appendChild(syncControls);

webUIListenerCallback('sync-prefs-changed', getSyncAllPrefs());
flush();

await waitBeforeNextRender(syncControls);
const customizeSync: CrRadioButtonElement =
syncControls.shadowRoot!.querySelector(
'cr-radio-button[name="customize-sync"]')!;
autofillCheckbox =
syncControls.shadowRoot!.querySelector('#autofillCheckbox')!;
paymentsCheckbox =
syncControls.shadowRoot!.querySelector('#paymentsCheckbox')!;
assertTrue(!!customizeSync);
assertTrue(!!autofillCheckbox);
assertTrue(!!paymentsCheckbox);

customizeSync.click();
flush();
assertTrue(customizeSync.checked);
assertTrue(autofillCheckbox.checked);
assertTrue(paymentsCheckbox.checked);
});

test('CoupledAutofillPaymentsToggles', async function() {
loadTimeData.overrideValues({
syncDecoupleAddressPaymentSettings: false,
});

// Disable Autofill sync.
autofillCheckbox.click();
flush();
assertFalse(autofillCheckbox.checked);
assertFalse(paymentsCheckbox.checked);
assertTrue(paymentsCheckbox.disabled);

// Enable Autofill sync.
autofillCheckbox.click();
flush();
assertTrue(autofillCheckbox.checked);
assertTrue(paymentsCheckbox.checked);
assertFalse(paymentsCheckbox.disabled);

// Disable Payment methods sync.
paymentsCheckbox.click();
flush();
assertTrue(autofillCheckbox.checked);
assertFalse(paymentsCheckbox.checked);
assertFalse(paymentsCheckbox.disabled);
});

test('DecoupledAutofillPaymentsToggles', async function() {
loadTimeData.overrideValues({
syncDecoupleAddressPaymentSettings: true,
});

// Disable Autofill sync.
autofillCheckbox.click();
flush();
assertFalse(autofillCheckbox.checked);
assertTrue(paymentsCheckbox.checked);
assertFalse(paymentsCheckbox.disabled);

// Disable Payment methods sync.
paymentsCheckbox.click();
flush();
assertFalse(autofillCheckbox.checked);
assertFalse(paymentsCheckbox.checked);
assertFalse(paymentsCheckbox.disabled);

// Enable Autofill sync.
autofillCheckbox.click();
flush();
assertTrue(autofillCheckbox.checked);
assertFalse(paymentsCheckbox.checked);
assertFalse(paymentsCheckbox.disabled);
});
});

0 comments on commit f3567e9

Please sign in to comment.