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

Issue metadata

Status: Fixed
Owner:
Closed: Jan 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment
link

Issue 908905: [Wallet sync] Diffs for cards/addresses are not reported consistently across USS/Directory

Reported by jkrcal@google.com, Nov 27 Project Member

Issue description

We need to fix that in order to assess sanity of the launch.
 

Comment 1 by treib@chromium.org, Nov 28

Cc: treib@chromium.org

Comment 2 by bugdroid1@chromium.org, Nov 29

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

commit bd17882565dfad0db04fe4b23760ee5c54eb914a
Author: Jan Krcal <jkrcal@chromium.org>
Date: Thu Nov 29 10:03:00 2018

[AF] Unify diff reporting for wallet data sync changes

Previous metrics for wallet data sync were reported in quite different
situations. This CL unifies the reporting conditions so that this is
easily implementable for both USS and Directory: We newly report the
diffs only when both the existing and the new data sets contain a
PaymentsCustomerData entity. This boils down to situations when:
 - the user has synced before (thus excluding initial sync),
 - the user has a GPay account (diffs for other users are always empty),
 - we are not wiping the data (which is implemented in USS by storing an
   empty set).

Bug:  908905 
Change-Id: I38bc1c12e9407ddc08ff798f69f09ba85fd5848c
Reviewed-on: https://chromium-review.googlesource.com/c/1352410
Reviewed-by: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Florian Uunk <feuunk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612123}
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/components/autofill/core/browser/webdata/autofill_wallet_sync_bridge.cc
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/components/autofill/core/browser/webdata/autofill_wallet_sync_bridge.h
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/components/autofill/core/browser/webdata/autofill_wallet_sync_bridge_unittest.cc
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.h
[modify] https://crrev.com/bd17882565dfad0db04fe4b23760ee5c54eb914a/tools/metrics/histograms/histograms.xml

Comment 3 by bugdroid1@chromium.org, Dec 4

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b3c9df03fd40c8725129a35f1819c30bb666719

commit 5b3c9df03fd40c8725129a35f1819c30bb666719
Author: Jan Krcal <jkrcal@chromium.org>
Date: Tue Dec 04 07:26:37 2018

[AF] Add missing affected-histogram tags for autofill wallet sync UMA

This CL adds tags that were omitted in CL 1352410

Bug:  908905 
Change-Id: Ifd5c676174472d66caee4bfd2125d1b24eb39c1b
Reviewed-on: https://chromium-review.googlesource.com/c/1358435
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613458}
[modify] https://crrev.com/5b3c9df03fd40c8725129a35f1819c30bb666719/tools/metrics/histograms/histograms.xml

Comment 4 by bugdroid1@chromium.org, Dec 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bf4776d959863e0d5a261dc33d22ed6e2201e35

commit 8bf4776d959863e0d5a261dc33d22ed6e2201e35
Author: Jan Krcal <jkrcal@chromium.org>
Date: Wed Dec 12 15:22:20 2018

[AF Wallet] Do not distinguish types of server cards in Compare()

This CL changes the implementation of CreditCard::Compare() so that we
do not distinguish masked from unmasked server cards. This distinction
was not needed by any of the call sites and it is undesired for wallet
data sync.

Namely, we do not want to write to database new server cards if the only
difference is that some cards in the DB are unmasked whereas all cards
from sync protos are (by definition) masked. This DB write does not
change anything in the DB but it is unnecessary and it also pollutes UMA
metrics.

Bug:  908905 
Change-Id: I187e8f717c7595220545febc22e479ea6806baf9
Reviewed-on: https://chromium-review.googlesource.com/c/1373449
Reviewed-by: Florian Uunk <feuunk@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615892}
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/chrome/browser/sync/test/integration/wallet_helper.cc
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/chrome/browser/sync/test/integration/wallet_helper.h
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/components/autofill/core/browser/credit_card.cc
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/components/autofill/core/browser/credit_card.h
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/components/autofill/core/browser/credit_card_unittest.cc
[modify] https://crrev.com/8bf4776d959863e0d5a261dc33d22ed6e2201e35/components/autofill/core/browser/personal_data_manager_unittest.cc

Comment 5 by bugdroid1@chromium.org, Dec 15

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

commit fb6b132969c51f1954492f21ed99800dd0083ac7
Author: Jan Krcal <jkrcal@chromium.org>
Date: Sat Dec 15 16:33:43 2018

[AF] Fix reporting to Autofill.Wallet* diff histograms in Directory

This CL fixes how diff metrics are reported from wallet syncable
service. Unlike the USS implementation, it reported diff metrics on
every startup, leading to inconsistencies in number of samples (and also
to inconsistencies in the distribution of the values).

This CL makes sure we do not report on startup
(in MergeDataAndStartSyncing()) but only in real updates
(ProcessSyncChanges()).

Bug:  908905 
Change-Id: I813cc9629febdecc3f10929a73345539e12eb842
Reviewed-on: https://chromium-review.googlesource.com/c/1373451
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616981}
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/chrome/browser/sync/test/integration/single_client_wallet_sync_test.cc
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/components/autofill/core/browser/webdata/autofill_wallet_sync_bridge.cc
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/components/autofill/core/browser/webdata/autofill_wallet_sync_bridge_unittest.cc
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/components/autofill/core/browser/webdata/autofill_wallet_syncable_service.h
[modify] https://crrev.com/fb6b132969c51f1954492f21ed99800dd0083ac7/tools/metrics/histograms/histograms.xml

Comment 6 by jkrcal@chromium.org, Jan 15

Status: Fixed (was: Started)

Sign in to add a comment