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

Issue 903290 link

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



Sign in to add a comment

Add SyncState suffix to some Autofill metrics

Project Member Reported by se...@chromium.org, Nov 8 
In order to better asses the impact of SyncTransport, it would be
useful to log some key Autofill metrics depending on the sync state
the user is in.

Those metrics are
Card saving metrics (shown, accepted, etc)
Suggestion CTR
Autofill disable rate.

The suffix to be added are
SignedOut
SignedIn
SignedInAndSyncTransport
SignedInAndSyncFeature

Comment 1 by jkrcal@chromium.org, Nov 19 

Seb, ping from Chrome Sync triager, any progress here? Still aiming at M72?

Comment 2 by se...@chromium.org, Nov 19 

Yes CL is almost complete :)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 27 

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

commit 31a8d2d345291f64509f6c622025b2d1e65c1afe
Author: Marc Treib <treib@chromium.org>
Date: Tue Nov 27 15:41:47 2018

SyncAuthManager: Expose static DetermineAccountToUse

This will be used by some autofill metrics.

Bug:  903290 
Change-Id: I40df75f93a3b25d1beb2ba77cb231a227b44c8ab
Reviewed-on: https://chromium-review.googlesource.com/c/1350984
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611116}
[modify] https://crrev.com/31a8d2d345291f64509f6c622025b2d1e65c1afe/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/31a8d2d345291f64509f6c622025b2d1e65c1afe/components/browser_sync/sync_auth_manager.h
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 27 

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

commit 2b0e8e60c2d02c4ac4a98422cb069784db7a0623
Author: Marc Treib <treib@chromium.org>
Date: Tue Nov 27 20:06:02 2018

Sync: Move DetermineAccountToUse into new sync_auth_util.h/cc

This is a followup to https://crrev.com/c/1350984.
In particular, this moves the code from /components/browser_sync/ into
/components/sync/, where autofill code can actually access it.

Bug:  903290 
Change-Id: Id7b90cf5c1344cb4404d9a672d057a67e74d35b9
Reviewed-on: https://chromium-review.googlesource.com/c/1352160
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611277}
[modify] https://crrev.com/2b0e8e60c2d02c4ac4a98422cb069784db7a0623/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/2b0e8e60c2d02c4ac4a98422cb069784db7a0623/components/browser_sync/sync_auth_manager.h
[modify] https://crrev.com/2b0e8e60c2d02c4ac4a98422cb069784db7a0623/components/sync/BUILD.gn
[add] https://crrev.com/2b0e8e60c2d02c4ac4a98422cb069784db7a0623/components/sync/driver/sync_auth_util.cc
[add] https://crrev.com/2b0e8e60c2d02c4ac4a98422cb069784db7a0623/components/sync/driver/sync_auth_util.h
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 7 

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

commit 119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3
Author: sebsg <sebsg@chromium.org>
Date: Fri Dec 07 01:03:47 2018

[AF] Add SyncState suffix to some key autofill metrics.

The suffixes describing the sync state are:
SignedOut
SignedIn
kSignedInAndWalletSyncTransportEnabled
SignedInAndSyncFeature

The metrics that will have those suffixes are
Autofill.FormEvents
Autofill.IsEnabled.PageLoad
Autofill.SaveCreditCardPrompt

The previous histograms will continue to be logged. This is only
additional logging.

Bug:  903290 
Change-Id: Ibc96a1db12408f13d40fe75ed9aec96af80eac96
Reviewed-on: https://chromium-review.googlesource.com/c/1326602
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614517}
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/chrome/browser/ui/autofill/save_card_bubble_controller_impl.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/chrome/browser/ui/autofill/save_card_bubble_controller_impl.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/chrome/browser/ui/autofill/save_card_bubble_controller_impl_unittest.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_external_delegate_unittest.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_manager.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_manager_unittest.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_metrics.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/personal_data_manager_unittest.cc
[add] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/sync_utils.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3/tools/metrics/histograms/histograms.xml

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

Labels: Merge-Request-72
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 8

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact 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
Project Member

Comment 8 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/+/7935b507722311ad4100ff176f1cc5f3cc355722

commit 7935b507722311ad4100ff176f1cc5f3cc355722
Author: sebsg <sebsg@chromium.org>
Date: Mon Dec 10 14:57:11 2018

Merge-72 [AF] Add SyncState suffix to some key autofill metrics.

The suffixes describing the sync state are:
SignedOut
SignedIn
kSignedInAndWalletSyncTransportEnabled
SignedInAndSyncFeature

The metrics that will have those suffixes are
Autofill.FormEvents
Autofill.IsEnabled.PageLoad
Autofill.SaveCreditCardPrompt

The previous histograms will continue to be logged. This is only
additional logging.

TBR=sebsg@chromium.org

(cherry picked from commit 119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3)

Bug:  903290 
Change-Id: Ibc96a1db12408f13d40fe75ed9aec96af80eac96
Reviewed-on: https://chromium-review.googlesource.com/c/1326602
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614517}
Reviewed-on: https://chromium-review.googlesource.com/c/1369630
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#196}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/chrome/browser/ui/autofill/save_card_bubble_controller_impl.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/chrome/browser/ui/autofill/save_card_bubble_controller_impl.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/chrome/browser/ui/autofill/save_card_bubble_controller_impl_unittest.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_external_delegate_unittest.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_manager.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_manager_unittest.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_metrics.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/personal_data_manager_unittest.cc
[add] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/sync_utils.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/7935b507722311ad4100ff176f1cc5f3cc355722/tools/metrics/histograms/histograms.xml

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

Status: Fixed (was: Started)
Project Member

Comment 10 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/+/7935b507722311ad4100ff176f1cc5f3cc355722

Commit: 7935b507722311ad4100ff176f1cc5f3cc355722
Author: sebsg@chromium.org
Commiter: sebsg@chromium.org
Date: 2018-12-10 14:57:11 +0000 UTC

Merge-72 [AF] Add SyncState suffix to some key autofill metrics.

The suffixes describing the sync state are:
SignedOut
SignedIn
kSignedInAndWalletSyncTransportEnabled
SignedInAndSyncFeature

The metrics that will have those suffixes are
Autofill.FormEvents
Autofill.IsEnabled.PageLoad
Autofill.SaveCreditCardPrompt

The previous histograms will continue to be logged. This is only
additional logging.

TBR=sebsg@chromium.org

(cherry picked from commit 119a2fe85a3ae98d60d9e5a5b0837c20cb21bfb3)

Bug:  903290 
Change-Id: Ibc96a1db12408f13d40fe75ed9aec96af80eac96
Reviewed-on: https://chromium-review.googlesource.com/c/1326602
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614517}
Reviewed-on: https://chromium-review.googlesource.com/c/1369630
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#196}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
►

Sign in to add a comment