Adds new metric to distinguish between no address and no recent address.

Adds new metric to distinguish between no address and no recent address.

Logs NO_ZIP_CODE only if there was at least one candidate address.

BUG=713842
Review-Url: https://codereview.chromium.org/2861223004
Cr-Commit-Position: refs/heads/master@{#469830}
Committed: https://chromium.googlesource.com/chromium/src/+/fc136b6992c52386e8b22aef3550dbde14c1bafe

[csashi, 2017-05-05 21:59:34]

csashi@google.com changed reviewers:
+ jsaul@google.com, sebsg@chromium.org

[csashi, 2017-05-05 21:59:35]

Hi,
Please take a look. Thanks! -sashi.

[csashi, 2017-05-05 21:59:59]

Description was changed from

==========
Adds new metric to distinguish between no address and no recent address.

BUG=713842
==========

to

==========
Adds new metric to distinguish between no address and no recent address.

Logs NO_ZIP_CODE only if there was at least one candidate address.

BUG=713842
==========

[csashi, 2017-05-05 22:00:15]

The CQ bit was checked by csashi@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 22:01:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jared Saul, 2017-05-05 22:17:50]

lgtm

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager.cc:1287: // reporting on CVC
then address.
"Conform to the legacy order of reporting on CVC then address."

This isn't true anymore because we report on everything, right?  Can you you
remove this?

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager.cc:1339: :
AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ADDRESS;
nit: Could we change this to reflect it's new clearer meaning, like
"UPLOAD_NOT_OFFERED_NO_PROFILE_ADDRESSES"?

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_metrics.h (right):

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_metrics.h:111: // A CVC was detected
but no address was available.
"A CVC was detected but..." isn't strictly true because we report on everything
now, right?  Looks like most of these could benefit a change there.

[sebsg, 2017-05-05 22:18:51]

Nice! LGTM

[csashi, 2017-05-05 22:42:27]

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager.cc:1287: // reporting on CVC
then address.
On 2017/05/05 22:17:50, Jared Saul wrote:
> "Conform to the legacy order of reporting on CVC then address."
> 
> This isn't true anymore because we report on everything, right?  Can you you
> remove this?

Done.

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_manager.cc:1339: :
AutofillMetrics::UPLOAD_NOT_OFFERED_NO_ADDRESS;
On 2017/05/05 22:17:50, Jared Saul wrote:
> nit: Could we change this to reflect it's new clearer meaning, like
> "UPLOAD_NOT_OFFERED_NO_PROFILE_ADDRESSES"?

Done.

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
File components/autofill/core/browser/autofill_metrics.h (right):

https://codereview.chromium.org/2861223004/diff/1/components/autofill/core/br...
components/autofill/core/browser/autofill_metrics.h:111: // A CVC was detected
but no address was available.
On 2017/05/05 22:17:50, Jared Saul wrote:
> "A CVC was detected but..." isn't strictly true because we report on
everything
> now, right?  Looks like most of these could benefit a change there.

Done.

[csashi, 2017-05-05 22:42:35]

The CQ bit was checked by csashi@google.com

[csashi, 2017-05-05 22:42:35]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org,
jsaul@google.com
Link to the patchset: https://codereview.chromium.org/2861223004/#ps20001
(title: "Updates CardUploadDecisionMetric comments.")

[commit-bot: I haz the power, 2017-05-05 22:42:50]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-06 00:36:52]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494024155320710,
"parent_rev": "647aa09a706ee13efc535c94698e09559ce6d595", "commit_rev":
"fc136b6992c52386e8b22aef3550dbde14c1bafe"}

[commit-bot: I haz the power, 2017-05-06 00:37:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494024155320710,
"parent_rev": "647aa09a706ee13efc535c94698e09559ce6d595", "commit_rev":
"fc136b6992c52386e8b22aef3550dbde14c1bafe"}

[commit-bot: I haz the power, 2017-05-06 00:37:55]

Description was changed from

==========
Adds new metric to distinguish between no address and no recent address.

Logs NO_ZIP_CODE only if there was at least one candidate address.

BUG=713842
==========

to

==========
Adds new metric to distinguish between no address and no recent address.

Logs NO_ZIP_CODE only if there was at least one candidate address.

BUG=713842

Review-Url: https://codereview.chromium.org/2861223004
Cr-Commit-Position: refs/heads/master@{#469830}
Committed:
https://chromium.googlesource.com/chromium/src/+/fc136b6992c52386e8b22aef3550...
==========

[commit-bot: I haz the power, 2017-05-06 00:37:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fc136b6992c52386e8b22aef3550...