New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 907929 link

Starred by 2 users
Status: Fixed
Owner:
se...@chromium.org
Closed: Dec 10
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Add a wallet sync transport opt-in pref.

Project Member Reported by se...@chromium.org, Nov 22 
This will be used to record whether a user has opted in to seeing their
Google Account cards in the Autofill dropdown.

This should be recorded in a Profile Pref.

Since the intention is to have the opt-in be per-account & per-device, we need to store them as a dictionary where the key is a hash of the account id and the key is the opt-in status.

The dictionary should not contain info about accounts that are not opted in.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/affe4c454358628862b3dfd85e97e87eefa4e8f7

commit affe4c454358628862b3dfd85e97e87eefa4e8f7
Author: sebsg <sebsg@chromium.org>
Date: Fri Nov 23 16:03:58 2018

[AF] Add a wallet sync transport opt-in pref.

This will be used to record whether a user has opted in to seeing their
Google Account cards in the Autofill dropdown.

The dictionary should not contain info about accounts that are not opted
in.

Bug:  907929 
Change-Id: Icc663b228792c58b00c02b204249fba9f557481b
Reviewed-on: https://chromium-review.googlesource.com/c/1348792
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610621}
[modify] https://crrev.com/affe4c454358628862b3dfd85e97e87eefa4e8f7/components/autofill/core/common/autofill_prefs.cc
[modify] https://crrev.com/affe4c454358628862b3dfd85e97e87eefa4e8f7/components/autofill/core/common/autofill_prefs.h
[modify] https://crrev.com/affe4c454358628862b3dfd85e97e87eefa4e8f7/components/autofill/core/common/autofill_prefs_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 26 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b665f08c483f80994e679c4af7d9fa350af99b1

commit 6b665f08c483f80994e679c4af7d9fa350af99b1
Author: sebsg <sebsg@chromium.org>
Date: Mon Nov 26 22:35:10 2018

[AF] Clear wallet sync transport opt-in pref when user clears cookies.

Clear the entire opt-in dictionary if a user clears cookies

Bug:  907929 
Change-Id: I5ec68e09a8763be66b95109174b6281abe54d050
Reviewed-on: https://chromium-review.googlesource.com/c/1348793
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610964}
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/chrome/browser/autofill/personal_data_manager_factory.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/autofill_manager_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/credit_card_save_manager_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/form_data_importer_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/common/autofill_prefs.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/common/autofill_prefs.h
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/autofill/core/common/autofill_prefs_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/components/browser_sync/profile_sync_service_autofill_unittest.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/ios/chrome/browser/autofill/personal_data_manager_factory.cc
[modify] https://crrev.com/6b665f08c483f80994e679c4af7d9fa350af99b1/ios/web_view/internal/autofill/web_view_personal_data_manager_factory.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 6 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e93500b9d63f42bfac14872be6bec7560b8479a

commit 3e93500b9d63f42bfac14872be6bec7560b8479a
Author: sebsg <sebsg@chromium.org>
Date: Thu Dec 06 17:25:39 2018

[AF] Update ShouldSuggestServerCards to check  opt in in sync-transport.

For users that are in sync transport mode, server cards should only be
suggested if the user has opted-in to seeing them.

Bug:  907929 
Change-Id: I45d855fa6072a1ecb85a4c35244280b0ec8cb4fb
Reviewed-on: https://chromium-review.googlesource.com/c/1365086
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614398}
[modify] https://crrev.com/3e93500b9d63f42bfac14872be6bec7560b8479a/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/3e93500b9d63f42bfac14872be6bec7560b8479a/components/autofill/core/browser/personal_data_manager_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca14b09d545fe715968a9de0867a340ff38c5385

commit ca14b09d545fe715968a9de0867a340ff38c5385
Author: sebsg <sebsg@chromium.org>
Date: Fri Dec 07 16:56:15 2018

[AF] Add feature to always show server cards in sync transport mode.

Bug:  907929 
Change-Id: I5327254e20e4e6bbbfd0d8fc35d24e1dd5814917
Reviewed-on: https://chromium-review.googlesource.com/c/1366374
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614726}
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/chrome/browser/about_flags.cc
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/chrome/browser/flag-metadata.json
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/components/autofill/core/common/autofill_features.cc
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/components/autofill/core/common/autofill_features.h
[modify] https://crrev.com/ca14b09d545fe715968a9de0867a340ff38c5385/tools/metrics/histograms/enums.xml

Comment 6 by se...@chromium.org, Dec 7

Labels: Merge-Request-72
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 7

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by srinivassista@google.com, Dec 7 

sebsg@ can you please confirm once the CL is in canary and test results in canary look fine. Also, How risky is this change  and how critical is it to take the change into 72.0 ? can this wait for 73.0?

Comment 9 by se...@chromium.org, Dec 7 

Hi, I forgot to mention that this is all behind a feature, and part of the Wallet Sync Transport (also behind a feature) which is M72 critical.

Thanks!

Comment 10 by srinivassista@google.com, Dec 7 

thanks sebsg@, there are multiple CL's listed here , Comment 4 and 5, can you confirm if you are asking merge approval for both or one?

Comment 11 by se...@chromium.org, Dec 7 

It would be for 3, 4 and 5. Thanks!

Comment 12 by abdulsyed@chromium.org, Dec 10 

Are these all behind a flag? Specifically for launching a new feature (#5), feature freeze date is 2 weeks before branch. Can you please confirm how big this feature is and why are we needing this to merge this the week of beta promotion? My recommendation is to wait until M73.

Comment 13 by se...@chromium.org, Dec 10 

There are all gated on butter, for which we already have approval and etc. This was a UX requirement that was added recently. 

The new "feature" that was added is to allow experimenting with or without this new UI for Butter. But either cases is gated on Butter anyway.

Comment 14 by abdulsyed@chromium.org, Dec 10

Labels: -Merge-Review-72 Merge-Approved-72
Ok thanks for more context. Approving merge for M72.
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 10

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb

commit a6bb06a99e3d8d4913a9f0591b4c0c84889241fb
Author: sebsg <sebsg@chromium.org>
Date: Mon Dec 10 18:09:21 2018

Merge-72 [AF]Use accnt id hash as key for wallet sync-transp opt-in pref

TBR=sebsg@chromium.org

(cherry picked from commit aa3d28f3e904b8efd36b851a46f3795577f09eb9)

Bug:  907929 
Change-Id: I6cc9d7a908010c204fc8060802c7ee453363ffa3
Reviewed-on: https://chromium-review.googlesource.com/c/1349455
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612769}
Reviewed-on: https://chromium-review.googlesource.com/c/1369640
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#208}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb/components/autofill/core/common/BUILD.gn
[add] https://crrev.com/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb/components/autofill/core/common/DEPS
[modify] https://crrev.com/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb/components/autofill/core/common/autofill_prefs.cc
[modify] https://crrev.com/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb/components/autofill/core/common/autofill_prefs_unittest.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbc11d6fb687e3094f5477382e4ddee494155a19

commit cbc11d6fb687e3094f5477382e4ddee494155a19
Author: sebsg <sebsg@chromium.org>
Date: Mon Dec 10 18:13:40 2018

Merge-72 [AF]ShouldSuggestServerCards check opt-in sync-transport.

For users that are in sync transport mode, server cards should only be
suggested if the user has opted-in to seeing them.

TBR=sebsg@chromium.org

(cherry picked from commit 3e93500b9d63f42bfac14872be6bec7560b8479a)

Bug:  907929 
Change-Id: I45d855fa6072a1ecb85a4c35244280b0ec8cb4fb
Reviewed-on: https://chromium-review.googlesource.com/c/1365086
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614398}
Reviewed-on: https://chromium-review.googlesource.com/c/1370309
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#210}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/cbc11d6fb687e3094f5477382e4ddee494155a19/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/cbc11d6fb687e3094f5477382e4ddee494155a19/components/autofill/core/browser/personal_data_manager_unittest.cc
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a426998b416b9b84d55226c57e4ab9d8b64c1847

commit a426998b416b9b84d55226c57e4ab9d8b64c1847
Author: sebsg <sebsg@chromium.org>
Date: Mon Dec 10 18:16:19 2018

Merge [AF]Add feature to always show server cards in sync transport mode

TBR=sebsg@chromium.org

(cherry picked from commit ca14b09d545fe715968a9de0867a340ff38c5385)

Bug:  907929 
Change-Id: I5327254e20e4e6bbbfd0d8fc35d24e1dd5814917
Reviewed-on: https://chromium-review.googlesource.com/c/1366374
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614726}
Reviewed-on: https://chromium-review.googlesource.com/c/1370310
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#212}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/chrome/browser/about_flags.cc
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/chrome/browser/flag-metadata.json
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/components/autofill/core/common/autofill_features.cc
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/components/autofill/core/common/autofill_features.h
[modify] https://crrev.com/a426998b416b9b84d55226c57e4ab9d8b64c1847/tools/metrics/histograms/enums.xml

Comment 18 by se...@chromium.org, Dec 10

Status: Fixed (was: Started)
Merged the 3 CLs, thanks!
Project Member

Comment 19 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/a426998b416b9b84d55226c57e4ab9d8b64c1847

Commit: a426998b416b9b84d55226c57e4ab9d8b64c1847
Author: sebsg@chromium.org
Commiter: sebsg@chromium.org
Date: 2018-12-10 18:16:19 +0000 UTC

Merge [AF]Add feature to always show server cards in sync transport mode

TBR=sebsg@chromium.org

(cherry picked from commit ca14b09d545fe715968a9de0867a340ff38c5385)

Bug:  907929 
Change-Id: I5327254e20e4e6bbbfd0d8fc35d24e1dd5814917
Reviewed-on: https://chromium-review.googlesource.com/c/1366374
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614726}
Reviewed-on: https://chromium-review.googlesource.com/c/1370310
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#212}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Project Member

Comment 20 by cr-audit...@appspot.gserviceaccount.com, Dec 19 

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/a6bb06a99e3d8d4913a9f0591b4c0c84889241fb

Commit: a6bb06a99e3d8d4913a9f0591b4c0c84889241fb
Author: sebsg@chromium.org
Commiter: sebsg@chromium.org
Date: 2018-12-10 18:09:21 +0000 UTC

Merge-72 [AF]Use accnt id hash as key for wallet sync-transp opt-in pref

TBR=sebsg@chromium.org

(cherry picked from commit aa3d28f3e904b8efd36b851a46f3795577f09eb9)

Bug:  907929 
Change-Id: I6cc9d7a908010c204fc8060802c7ee453363ffa3
Reviewed-on: https://chromium-review.googlesource.com/c/1349455
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612769}
Reviewed-on: https://chromium-review.googlesource.com/c/1369640
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#208}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Project Member

Comment 21 by cr-audit...@appspot.gserviceaccount.com, Dec 19 

The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/cbc11d6fb687e3094f5477382e4ddee494155a19

Commit: cbc11d6fb687e3094f5477382e4ddee494155a19
Author: sebsg@chromium.org
Commiter: sebsg@chromium.org
Date: 2018-12-10 18:13:40 +0000 UTC

Merge-72 [AF]ShouldSuggestServerCards check opt-in sync-transport.

For users that are in sync transport mode, server cards should only be
suggested if the user has opted-in to seeing them.

TBR=sebsg@chromium.org

(cherry picked from commit 3e93500b9d63f42bfac14872be6bec7560b8479a)

Bug:  907929 
Change-Id: I45d855fa6072a1ecb85a4c35244280b0ec8cb4fb
Reviewed-on: https://chromium-review.googlesource.com/c/1365086
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614398}
Reviewed-on: https://chromium-review.googlesource.com/c/1370309
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#210}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
►

Sign in to add a comment