New issue
Advanced search Search tips

Issue 839029 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Bank name experiment histogram doesn't respect masked cards correctly

Project Member Reported by jsaul@google.com, May 2 2018

Issue description

The metric that detects if a server suggestion was filled when a bank name card was available in the Autofill suggestions only included FULL server cards, not MASKED server cards:
https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofill_metrics.cc?l=1469-1472&rcl=9af650571273676119bde851aab4e8d7d73ece49

This means that metric is missing ~20% of its data.  It's yet unclear if this would change its proportions, but we should relaunch to be safe.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 2 2018

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

commit 97e89f6879839baac6c1ef876b4aceaa1937747a
Author: Jared Saul <jsaul@google.com>
Date: Wed May 02 21:04:32 2018

[Autofill] Fix bank name metric for suggestions filled for MASKED server cards

Bug:  839029 
Change-Id: I11bb3eb81a3e2ac04bd8f99332756e8529b94f7b
Reviewed-on: https://chromium-review.googlesource.com/1040535
Commit-Queue: Jared Saul <jsaul@google.com>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555519}
[modify] https://crrev.com/97e89f6879839baac6c1ef876b4aceaa1937747a/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/97e89f6879839baac6c1ef876b4aceaa1937747a/components/autofill/core/browser/autofill_metrics_unittest.cc

Comment 2 by jsaul@google.com, May 2 2018

Labels: Merge-Request-67 M-67
Requesting merge of https://chromium-review.googlesource.com/c/chromium/src/+/1040535 into M67.  This is a very small, 4-line metrics change that we want to remedy as early as possible.  Thanks!
Project Member

Comment 3 by sheriffbot@chromium.org, May 2 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Less than 23 days to go before AppStore submit on M67
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 4 by jsaul@google.com, May 2 2018

(Clarification: The important part is 4 lines.  I also added ~115 lines of unit tests but I assume those aren't a merge issue.)
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comments #2 and #4. Pls merge ASAP. Thank you.
Project Member

Comment 6 by bugdroid1@chromium.org, May 3 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4fc6c03f7f29f6de7e60f8159a358be20d56f7b2

commit 4fc6c03f7f29f6de7e60f8159a358be20d56f7b2
Author: Jared Saul <jsaul@google.com>
Date: Thu May 03 18:03:51 2018

[Autofill] Fix bank name metric for suggestions filled for MASKED server cards

Bug:  839029 
Change-Id: I11bb3eb81a3e2ac04bd8f99332756e8529b94f7b
Reviewed-on: https://chromium-review.googlesource.com/1040535
Commit-Queue: Jared Saul <jsaul@google.com>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555519}(cherry picked from commit 97e89f6879839baac6c1ef876b4aceaa1937747a)
Reviewed-on: https://chromium-review.googlesource.com/1042766
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/branch-heads/3396@{#459}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/4fc6c03f7f29f6de7e60f8159a358be20d56f7b2/components/autofill/core/browser/autofill_metrics.cc
[modify] https://crrev.com/4fc6c03f7f29f6de7e60f8159a358be20d56f7b2/components/autofill/core/browser/autofill_metrics_unittest.cc

Comment 7 by jsaul@google.com, May 3 2018

Status: Fixed (was: Assigned)
Thanks!  Merge successful.

Sign in to add a comment