Move Autofill Payments integration checkbox to the sync settings page.

Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting has changed. Previously it was changed by events from the personal data manager and the sync service. This was because it depended on the state of those services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends *only* on whether Autofill sync is enabled. (It no longer depends on the rollout flag governed by the sync service nor on whether the user currently has server cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill sync setting, the availability of the integration setting can simply be changed in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than visibility to avoid a dialog size change and relayout whenever the user clicks the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism was used to read/write the integration setting. Because the sync settings dialog uses an ok/cancel design, we can't immediately write the value of the checkbox to the setting. Instead, it's manually set along with the rest of the sync settings after the user clicks ok.

BUG=535784
Committed: https://crrev.com/b4ddb6b439ded116b61d77dc6ed0f0e8a3bad4b9
Cr-Commit-Position: refs/heads/master@{#369857}

[Justin Donnelly, 2016-01-14 22:05:45]

Description was changed from

==========
Move Autofill Payments integration checkbox to the sync settings page.

BUG=535784
==========

to

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on mock from zkoch: https://screenshot.googleplex.com/59eiPNOvZAS.png

BUG=535784
==========

[Justin Donnelly, 2016-01-14 22:05:53]

Patchset #1 (id:1) has been deleted

[Justin Donnelly, 2016-01-14 22:07:53]

jdonnelly@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2016-01-15 01:28:19]

dbeam@chromium.org changed reviewers:
+ brettw@chromium.org, estade@chromium.org

[Dan Beam, 2016-01-15 01:28:20]

i'm generally unsure where this UI should go and kinda hazy on how these 3
systems interact (sync, payments, autofill).

i nominate brettw@ or estade@ in my stead (sorry!)

[Justin Donnelly, 2016-01-15 14:09:58]

Brett has agreed to review this.

[Justin Donnelly, 2016-01-15 14:10:12]

jdonnelly@chromium.org changed reviewers:
- dbeam@chromium.org, estade@chromium.org

[Justin Donnelly, 2016-01-15 14:30:20]

Description was changed from

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on mock from zkoch: https://screenshot.googleplex.com/59eiPNOvZAS.png

BUG=535784
==========

to

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock
available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting
has changed. Previously it was changed by events from the personal data manager
and the sync service. This was because it depended on the state of those
services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends
*only* on whether Autofill sync is enabled. (It no longer depends on the rollout
flag governed by the sync service nor on whether the user currently has server
cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill
sync setting, the availability of the integration setting can simply be changed
in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than
visibility to avoid a dialog size change and relayout whenever the user clicks
the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism
was used to read/write the integration setting. Because the sync settings dialog
uses an ok/cancel design, we can't immediately write the value of the checkbox
to the setting. Instead, it's manually set along with the rest of the sync
settings after the user clicks ok.

BUG=535784
==========

[please use gerrit instead, 2016-01-15 17:05:32]

https://codereview.chromium.org/1578173005/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/sync_setup_overlay.js (right):

https://codereview.chromium.org/1578173005/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/sync_setup_overlay.js:856: * Toggles the
visibility of the Payments integration checkbox.
Would this wording be better?

s/visibility/enabled-disabled state/

[Justin Donnelly, 2016-01-15 17:14:50]

Patchset #3 (id:60001) has been deleted

[Justin Donnelly, 2016-01-15 17:15:36]

https://codereview.chromium.org/1578173005/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/sync_setup_overlay.js (right):

https://codereview.chromium.org/1578173005/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/sync_setup_overlay.js:856: * Toggles the
visibility of the Payments integration checkbox.
On 2016/01/15 17:05:32, Rouslan wrote:
> Would this wording be better?
> 
> s/visibility/enabled-disabled state/

Oops, yeah, I copied this from the autofill settings code and didn't update this
comment to reflect the change in behavior.

Since this function is only called from one place now I've just inlined the code
and removed the function.

[brettw, 2016-01-15 20:30:41]

With upstreaming, I think moving this to sync makes sense.

https://codereview.chromium.org/1578173005/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc (right):

https://codereview.chromium.org/1578173005/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc:65: enum
EncryptAllConfig {
The existing system seems to be explicitly trying to avoid using booleans for
the setup here. It makes the setup call sites much more readable. Can you add an
enum for the payments integration too like the encryption option, and replace
the boolean with it?

[brettw, 2016-01-15 20:30:58]

LGTM with previous suggestions ^^^

[Justin Donnelly, 2016-01-15 21:16:51]

Thanks for the review.

https://codereview.chromium.org/1578173005/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc (right):

https://codereview.chromium.org/1578173005/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/options/sync_setup_handler_unittest.cc:65: enum
EncryptAllConfig {
On 2016/01/15 20:30:41, brettw wrote:
> The existing system seems to be explicitly trying to avoid using booleans for
> the setup here. It makes the setup call sites much more readable. Can you add
an
> enum for the payments integration too like the encryption option, and replace
> the boolean with it?

Good point. Done.

[Justin Donnelly, 2016-01-15 21:17:01]

The CQ bit was checked by jdonnelly@chromium.org

[Justin Donnelly, 2016-01-15 21:17:01]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1578173005/#ps120001
(title: "Replace bool with enum in tests")

[commit-bot: I haz the power, 2016-01-15 21:18:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1578173005/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1578173005/120001

[commit-bot: I haz the power, 2016-01-15 22:05:33]

Description was changed from

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock
available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting
has changed. Previously it was changed by events from the personal data manager
and the sync service. This was because it depended on the state of those
services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends
*only* on whether Autofill sync is enabled. (It no longer depends on the rollout
flag governed by the sync service nor on whether the user currently has server
cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill
sync setting, the availability of the integration setting can simply be changed
in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than
visibility to avoid a dialog size change and relayout whenever the user clicks
the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism
was used to read/write the integration setting. Because the sync settings dialog
uses an ok/cancel design, we can't immediately write the value of the checkbox
to the setting. Instead, it's manually set along with the rest of the sync
settings after the user clicks ok.

BUG=535784
==========

to

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock
available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting
has changed. Previously it was changed by events from the personal data manager
and the sync service. This was because it depended on the state of those
services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends
*only* on whether Autofill sync is enabled. (It no longer depends on the rollout
flag governed by the sync service nor on whether the user currently has server
cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill
sync setting, the availability of the integration setting can simply be changed
in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than
visibility to avoid a dialog size change and relayout whenever the user clicks
the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism
was used to read/write the integration setting. Because the sync settings dialog
uses an ok/cancel design, we can't immediately write the value of the checkbox
to the setting. Instead, it's manually set along with the rest of the sync
settings after the user clicks ok.

BUG=535784
==========

[commit-bot: I haz the power, 2016-01-15 22:05:34]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-01-15 22:06:36]

Description was changed from

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock
available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting
has changed. Previously it was changed by events from the personal data manager
and the sync service. This was because it depended on the state of those
services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends
*only* on whether Autofill sync is enabled. (It no longer depends on the rollout
flag governed by the sync service nor on whether the user currently has server
cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill
sync setting, the availability of the integration setting can simply be changed
in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than
visibility to avoid a dialog size change and relayout whenever the user clicks
the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism
was used to read/write the integration setting. Because the sync settings dialog
uses an ok/cancel design, we can't immediately write the value of the checkbox
to the setting. Instead, it's manually set along with the rest of the sync
settings after the user clicks ok.

BUG=535784
==========

to

==========
Move Autofill Payments integration checkbox to the sync settings page.

Based on results of discussions between privacy, pm, and UI review. Mock
available at: https://screenshot.googleplex.com/59eiPNOvZAS.png

Notes on the implementation:

The mechanism for handling the availability of the payments integration setting
has changed. Previously it was changed by events from the personal data manager
and the sync service. This was because it depended on the state of those
services and because the setting was on a different page than the sync settings.

With the new upload feature, the availability of the integration setting depends
*only* on whether Autofill sync is enabled. (It no longer depends on the rollout
flag governed by the sync service nor on whether the user currently has server
cards in the personal data manager.)

Because of this and since the setting is on the same page/dialog as the Autofill
sync setting, the availability of the integration setting can simply be changed
in a click handler on the Autofill sync setting.

"Availability" is also now implemented via disabling (greying out) rather than
visibility to avoid a dialog size change and relayout whenever the user clicks
the Autofill sync setting.

One final detail: on the Autofill settings dialog, the magic "pref=" mechanism
was used to read/write the integration setting. Because the sync settings dialog
uses an ok/cancel design, we can't immediately write the value of the checkbox
to the setting. Instead, it's manually set along with the rest of the sync
settings after the user clicks ok.

BUG=535784

Committed: https://crrev.com/b4ddb6b439ded116b61d77dc6ed0f0e8a3bad4b9
Cr-Commit-Position: refs/heads/master@{#369857}
==========

[commit-bot: I haz the power, 2016-01-15 22:06:37]

Patchset 5 (id:??) landed as
https://crrev.com/b4ddb6b439ded116b61d77dc6ed0f0e8a3bad4b9
Cr-Commit-Position: refs/heads/master@{#369857}