[Autofill] Log whether queried credit card form context is secure

[Autofill] Log whether queried credit card form context is secure

BUG=627493
TEST=AutofillMetricsTest

Committed: https://crrev.com/f7cef869aca6de2a7b14986eb045a20ef7363cda
Cr-Commit-Position: refs/heads/master@{#404971}

[Mathieu, 2016-07-12 16:02:45]

Patchset #1 (id:1) has been deleted

[Mathieu, 2016-07-12 16:03:12]

mathp@chromium.org changed reviewers:
+ isherman@chromium.org

[Mathieu, 2016-07-12 16:03:13]

Hi Ilya, please take a look?

[Ilya Sherman, 2016-07-12 21:33:04]

LGTM, thanks.

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

https://codereview.chromium.org/2144653002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1843: //
|form_structure| will be owned by |autofill_manager_|.
nit: Please omit the ownership comment, since it doesn't seem to apply here.

https://codereview.chromium.org/2144653002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1849:
autofill_client_.set_is_context_secure(true);
nit: I'd swap the order of the above two lines (and ditto below), so that you're
testing the narrowest possible window for when the metric is set.

[Mathieu, 2016-07-13 01:32:57]

Thanks, will submit

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

https://codereview.chromium.org/2144653002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1843: //
|form_structure| will be owned by |autofill_manager_|.
On 2016/07/12 21:33:04, Ilya Sherman wrote:
> nit: Please omit the ownership comment, since it doesn't seem to apply here.

Done.

https://codereview.chromium.org/2144653002/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_metrics_unittest.cc:1849:
autofill_client_.set_is_context_secure(true);
On 2016/07/12 21:33:04, Ilya Sherman wrote:
> nit: I'd swap the order of the above two lines (and ditto below), so that
you're
> testing the narrowest possible window for when the metric is set.

Done.

[Mathieu, 2016-07-13 01:33:01]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-07-13 01:33:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2144653002/#ps40001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-07-13 01:37:01]

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, 2016-07-13 03:40:29]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-13 03:40:38]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 03:42:12]

Description was changed from

==========
[Autofill] Log whether queried credit card form context is secure

BUG=627493
TEST=AutofillMetricsTest
==========

to

==========
[Autofill] Log whether queried credit card form context is secure

BUG=627493
TEST=AutofillMetricsTest

Committed: https://crrev.com/f7cef869aca6de2a7b14986eb045a20ef7363cda
Cr-Commit-Position: refs/heads/master@{#404971}
==========

[commit-bot: I haz the power, 2016-07-13 03:42:14]

Patchset 2 (id:??) landed as
https://crrev.com/f7cef869aca6de2a7b14986eb045a20ef7363cda
Cr-Commit-Position: refs/heads/master@{#404971}