Do not show Scan or Sign In options when credit card form is non-secure

Do not show Scan or Sign In options when credit card form is non-secure

Credit card autofill logic may offer "Scan credit card" and "Sign in to
Chrome to use cards from your Google account" options when filling out
a credit card form.

These options should not be shown when the form is not secure. We
instead notify users that non-secure forms will not autofill.

BUG=688461
Review-Url: https://codereview.chromium.org/2676513007
Cr-Commit-Position: refs/heads/master@{#448870}
Committed: https://chromium.googlesource.com/chromium/src/+/a0cab13ed385f0c524f29e279b2874677872bcdf

[elawrence, 2017-02-03 21:12:10]

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

[commit-bot: I haz the power, 2017-02-03 21:12:36]

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-02-03 21:53:38]

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

[commit-bot: I haz the power, 2017-02-03 21:53:39]

Dry run: This issue passed the CQ dry run.

[elawrence, 2017-02-03 22:10:28]

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

[elawrence, 2017-02-03 22:10:29]

PTAL?

[Mathieu, 2017-02-06 18:16:41]

Sorry for the delay. Thanks for the cleanup/fix

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

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:536: bool
AutofillManager::IsFormNonSecure(const FormData& form) const {
short unit test? AutofillClient functionality will be easy to mock out with
TestAutofillClient.

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:539: if
(form.action.is_valid() && form.action.SchemeIs("http"))
return !client_->IsContextSecure(form.origin) || (form.action.is_valid() &&
form.action.SchemeIs("http"));

[Mathieu, 2017-02-06 18:21:26]

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

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:325: bool
AutofillManager::ShouldShowCreditCardSigninPromo(
also see if the unit test for this needs to be updated?

[elawrence, 2017-02-07 20:12:28]

Patchset #3 (id:40001) has been deleted

[elawrence, 2017-02-07 20:16:48]

Addressed feedback and added tests. Please have another look?

thanks!

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

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:325: bool
AutofillManager::ShouldShowCreditCardSigninPromo(
On 2017/02/06 18:21:26, Mathieu Perreault wrote:
> also see if the unit test for this needs to be updated?

I added two new tests that check the new restriction.

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:536: bool
AutofillManager::IsFormNonSecure(const FormData& form) const {
On 2017/02/06 18:16:41, Mathieu Perreault wrote:
> short unit test? AutofillClient functionality will be easy to mock out with
> TestAutofillClient.

My new Unit tests exercise the IsFormNonSecure function which seemed safer than
"Friend"ing into the class to get access to this private member. Please let me
know if that's not okay.

https://codereview.chromium.org/2676513007/diff/20001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:539: if
(form.action.is_valid() && form.action.SchemeIs("http"))
On 2017/02/06 18:16:41, Mathieu Perreault wrote:
> return !client_->IsContextSecure(form.origin) || (form.action.is_valid() &&
> form.action.SchemeIs("http"));

Done.

[Mathieu, 2017-02-08 00:53:12]

Thanks for adding the tests! lgtm

[elawrence, 2017-02-08 01:37:50]

The CQ bit was checked by elawrence@chromium.org

[commit-bot: I haz the power, 2017-02-08 01:38:52]

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-02-08 02:41:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486517870023760,
"parent_rev": "db07be4db9751369102b24cd8fe772200982f3e7", "commit_rev":
"a0cab13ed385f0c524f29e279b2874677872bcdf"}

[commit-bot: I haz the power, 2017-02-08 02:41:45]

Description was changed from

==========
Do not show Scan or Sign In options when credit card form is non-secure

Credit card autofill logic may offer "Scan credit card" and "Sign in to
Chrome to use cards from your Google account" options when filling out
a credit card form.

These options should not be shown when the form is not secure. We
instead notify users that non-secure forms will not autofill.

BUG=688461
==========

to

==========
Do not show Scan or Sign In options when credit card form is non-secure

Credit card autofill logic may offer "Scan credit card" and "Sign in to
Chrome to use cards from your Google account" options when filling out
a credit card form.

These options should not be shown when the form is not secure. We
instead notify users that non-secure forms will not autofill.

BUG=688461

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

[commit-bot: I haz the power, 2017-02-08 02:41:47]

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