Log observations of explicit UPI-VPA autocomplete hints.

Log observations of explicit UPI-VPA autocomplete hints.

BUG=702220
Review-Url: https://codereview.chromium.org/2773433002
Cr-Commit-Position: refs/heads/master@{#460819}
Committed: https://chromium.googlesource.com/chromium/src/+/921b599d1775ae5ea19a51a4df7bbe70241beb1b

[Roger McFarlane (Chromium), 2017-03-22 18:55:25]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[Roger McFarlane (Chromium), 2017-03-22 18:56:09]

Description was changed from

==========
Log observations of explicit UPI-VPA autocomplete hints.

BUG=702220
R=jwd@chromium.org, mathp@chromium.org
==========

to

==========
Log observations of explicit UPI-VPA autocomplete hints.

BUG=702220
==========

[commit-bot: I haz the power, 2017-03-22 18:56:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Roger McFarlane (Chromium), 2017-03-22 18:57:58]

The CQ bit was unchecked by rogerm@chromium.org

[Roger McFarlane (Chromium), 2017-03-22 18:58:23]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[Roger McFarlane (Chromium), 2017-03-22 18:58:33]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-03-22 18:59:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Roger McFarlane (Chromium), 2017-03-22 19:01:40]

jwd and mathp, PTAL

[commit-bot: I haz the power, 2017-03-22 20:38:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 20:38:12]

Dry run: This issue passed the CQ dry run.

[Mathieu, 2017-03-23 11:26:22]

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/autofill_metrics.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics.cc:701:
UMA_HISTOGRAM_BOOLEAN("Autofill.ExplicitUpiVpaHint", true);
We could use something like Autofill.DeveloperEngagement.UpiVpaHint so that
anyone looking up the *other* autocomplete attribute metric
"Autofill.DeveloperEngagement" will see this one as well, close by

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/autofill_metrics_unittest.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1436:
TEST_F(AutofillMetricsTest, DeveloperEngagement) {
Please mirror your test on this one.

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:272: // TODO(rogerm): Add
UPI_VIRTUAL_PAYMENT_ADDRESS as an autofill type.
TODO(crbug.com/xxxxxx) is the norm now (more tractable)

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:272: // TODO(rogerm): Add
UPI_VIRTUAL_PAYMENT_ADDRESS as an autofill type.
nit: it would be HTML_TYPE_UPI...

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure_unittest.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure_unittest.cc:439:
base::HistogramTester histograms;
there is no histogram testing in this file (histograms are all tested in
autofill_metrics_unittest.cc), so let's remove it

https://codereview.chromium.org/2773433002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2773433002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:3579: +    Counts the number of times
the user encounters a form field that is
I like the "DeveloperEngagement" description:

For each form field detected, logs whether the field has an author-specified
type hint ("autocomplete") indicating UPA-VPI. 

Or some variation of that.

BTW, is that the question we want answered, or do we prefer the % of _forms_
that have a UPI-VPA hint?

[Roger McFarlane (Chromium), 2017-03-28 18:55:45]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 18:56:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Roger McFarlane (Chromium), 2017-03-28 19:08:24]

Thanks. Another look?

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/autofill_metrics.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics.cc:701:
UMA_HISTOGRAM_BOOLEAN("Autofill.ExplicitUpiVpaHint", true);
On 2017/03/23 11:26:22, Mathieu wrote:
> We could use something like Autofill.DeveloperEngagement.UpiVpaHint so that
> anyone looking up the *other* autocomplete attribute metric
> "Autofill.DeveloperEngagement" will see this one as well, close by

Added as an Autofill.DeveloperEngagement histogram enum.

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/autofill_metrics_unittest.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1436:
TEST_F(AutofillMetricsTest, DeveloperEngagement) {
On 2017/03/23 11:26:22, Mathieu wrote:
> Please mirror your test on this one.

Done.

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:272: // TODO(rogerm): Add
UPI_VIRTUAL_PAYMENT_ADDRESS as an autofill type.
On 2017/03/23 11:26:22, Mathieu wrote:
> TODO(crbug.com/xxxxxx) is the norm now (more tractable)

Done.

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:272: // TODO(rogerm): Add
UPI_VIRTUAL_PAYMENT_ADDRESS as an autofill type.
On 2017/03/23 11:26:22, Mathieu wrote:
> nit: it would be HTML_TYPE_UPI...

Done.

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
File components/autofill/core/browser/form_structure_unittest.cc (right):

https://codereview.chromium.org/2773433002/diff/20001/components/autofill/cor...
components/autofill/core/browser/form_structure_unittest.cc:439:
base::HistogramTester histograms;
On 2017/03/23 11:26:22, Mathieu wrote:
> there is no histogram testing in this file (histograms are all tested in
> autofill_metrics_unittest.cc), so let's remove it

Done.

https://codereview.chromium.org/2773433002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2773433002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:3579: +    Counts the number of times
the user encounters a form field that is
On 2017/03/23 11:26:22, Mathieu wrote:
> I like the "DeveloperEngagement" description:
> 
> For each form field detected, logs whether the field has an author-specified
> type hint ("autocomplete") indicating UPA-VPI. 
> 
> Or some variation of that.
> 
> BTW, is that the question we want answered, or do we prefer the % of _forms_
> that have a UPI-VPA hint? 

merged this into the developer engagement metric and updated it to count forms
having a hint, not fields having a hint.

[Mathieu, 2017-03-28 19:14:12]

lgtm with nit

https://codereview.chromium.org/2773433002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/2773433002/diff/40001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:373: if
(has_author_specified_upi_vpa_hint()) {
why not has_author_specified_upi_vpa_hint_

[Roger McFarlane (Chromium), 2017-03-28 19:37:20]

Thanks.

jwd@... ping?

https://codereview.chromium.org/2773433002/diff/40001/components/autofill/cor...
File components/autofill/core/browser/form_structure.cc (right):

https://codereview.chromium.org/2773433002/diff/40001/components/autofill/cor...
components/autofill/core/browser/form_structure.cc:373: if
(has_author_specified_upi_vpa_hint()) {
On 2017/03/28 19:14:12, Mathieu wrote:
> why not has_author_specified_upi_vpa_hint_

Done.

[Roger McFarlane (Chromium), 2017-03-28 19:37:35]

The CQ bit was checked by rogerm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 19:38:03]

Dry run: 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-03-28 20:55:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 20:55:58]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jwd, 2017-03-29 15:45:44]

lgtm

[Roger McFarlane (Chromium), 2017-03-30 15:25:37]

The CQ bit was checked by rogerm@chromium.org

[Roger McFarlane (Chromium), 2017-03-30 15:25:37]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org
Link to the patchset: https://codereview.chromium.org/2773433002/#ps60001
(title: "fix a nit")

[commit-bot: I haz the power, 2017-03-30 15:26:18]

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-03-30 17:54:22]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490887537173200,
"parent_rev": "b31c887a2d14761b078f329629f1d3469106b891", "commit_rev":
"921b599d1775ae5ea19a51a4df7bbe70241beb1b"}

[commit-bot: I haz the power, 2017-03-30 17:55:04]

Description was changed from

==========
Log observations of explicit UPI-VPA autocomplete hints.

BUG=702220
==========

to

==========
Log observations of explicit UPI-VPA autocomplete hints.

BUG=702220

Review-Url: https://codereview.chromium.org/2773433002
Cr-Commit-Position: refs/heads/master@{#460819}
Committed:
https://chromium.googlesource.com/chromium/src/+/921b599d1775ae5ea19a51a4df7b...
==========

[commit-bot: I haz the power, 2017-03-30 17:55:05]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/921b599d1775ae5ea19a51a4df7b...