iOS: Mark HTTP pages with credit card fields with an omnibox icon.

iOS: Mark HTTP pages with credit card fields with an omnibox icon.

BUG=648839
================================
TEST=Use an iPhone, not an iPad. First, enable the proper flag:
--------------------------------
1. Open the Settings app
2. Scroll to Chrome Beta/Dev/Canary and press
3. Scroll down to Experimental Settings and press
4. Scroll to EXTRA FLAGS (ONE PER LINE)
5. Toggle "Append Extra Flags" to ON
6. Set Flag1 to "--mark-non-secure-as=show-non-secure-passwords-cc-ui" (without the quotes)
--------------------------------
Test 3 URLs:
1) Visit https://badssl.com/input/credit-card/ and verify that the omnibox security has a green lock security indicator to the left of the URL.
2) Visit http://http-credit-card.badssl.com/ and verify that the omnibox security has a grey info icon ⓘ security indicator to the left of the URL.
3) Visit http://http.badssl.com/ and verify that the omnibox does *not* have a security indicator to the left of the URL.
--------------------------------
4) Set Flag1 (see above) to "--mark-non-secure-as=neutral" (without the quotes) and check that http://http-credit-card.badssl.com/ does *not* have a security indicator to the left of the URL.
================================

Review-Url: https://codereview.chromium.org/2669123003
Cr-Commit-Position: refs/heads/master@{#448193}
Committed: https://chromium.googlesource.com/chromium/src/+/8965c33248c51c71c273b5957344a974142a6914

[lgarron, 2017-02-03 03:18:39]

lgarron@chromium.org changed reviewers:
+ eugenebut@chromium.org, mathp@chromium.org

[lgarron, 2017-02-03 03:18:39]

mathp@: could you review autofill_driver_ios.mm?
eugenebut@: could you review the rest?

[lgarron, 2017-02-03 03:22:09]

Description was changed from

==========
iOS: Mark HTTP pages with credit card fields with an omnibox icon.

BUG=648839
==========

to

==========
iOS: Mark HTTP pages with credit card fields with an omnibox icon.

BUG=648839
================================
TEST=Use an iPhone, not an iPad. First, enable the proper flag:
--------------------------------
1. Open the Settings app
2. Scroll to Chrome Beta/Dev/Canary and press
3. Scroll down to Experimental Settings and press
4. Scroll to EXTRA FLAGS (ONE PER LINE)
5. Toggle "Append Extra Flags" to ON
6. Set Flag1 to "--mark-non-secure-as=show-non-secure-passwords-cc-ui" (without
the quotes)
--------------------------------
Test 3 URLs:
1) Visit https://badssl.com/input/credit-card/ and verify that the omnibox
security has a green lock security indicator to the left of the URL.
2) Visit http://http-credit-card.badssl.com/ and verify that the omnibox
security has a grey info icon ⓘ security indicator to the left of the URL.
3) Visit http://http.badssl.com/ and verify that the omnibox does *not* have a
security indicator to the left of the URL.
--------------------------------
4) Set Flag1 (see above) to "--mark-non-secure-as=neutral" (without the quotes)
and check that http://http-credit-card.badssl.com/ does *not* have a security
indicator to the left of the URL.
================================
==========

[lgarron, 2017-02-03 03:22:33]

On 2017/02/03 at 03:18:39, lgarron wrote:
> mathp@: could you review autofill_driver_ios.mm?
> eugenebut@: could you review the rest?

For reference, this CL is closely modeled after
https://codereview.chromium.org/2466143002
Screenshot at crbug.com/648839#c8

[lgarron, 2017-02-03 03:30:35]

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

[commit-bot: I haz the power, 2017-02-03 03:31:05]

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 03:46:20]

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

[commit-bot: I haz the power, 2017-02-03 03:46:20]

Dry run: This issue passed the CQ dry run.

[Eugene But (OOO till 7-30), 2017-02-03 19:24:16]

lgtm

https://codereview.chromium.org/2669123003/diff/50011/components/autofill/ios...
File components/autofill/ios/browser/autofill_driver_ios.mm (right):

https://codereview.chromium.org/2669123003/diff/50011/components/autofill/ios...
components/autofill/ios/browser/autofill_driver_ios.mm:9: #import
"ios/web/public/navigation_item.h"
You do need origin_util.h include, but do you need all others?

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
File ios/chrome/browser/autofill/autofill_controller_unittest.mm (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
ios/chrome/browser/autofill/autofill_controller_unittest.mm:599:
TEST_F(AutofillControllerTest, HTTPCreditCard) {
HttpCreditCard

Same for other tests

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
File ios/web/public/web_state/web_state.h (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
ios/web/public/web_state/web_state.h:216: // Tells the WebState that the current
page is an HTTP page
Do you want to use comment from web contents which looks more informative?
https://cs.chromium.org/chromium/src/content/public/browser/web_contents.h?ty...

If so, could you please update OnPasswordInputShownOnHttp comment as well.

[lgarron, 2017-02-04 00:28:35]

https://codereview.chromium.org/2669123003/diff/50011/components/autofill/ios...
File components/autofill/ios/browser/autofill_driver_ios.mm (right):

https://codereview.chromium.org/2669123003/diff/50011/components/autofill/ios...
components/autofill/ios/browser/autofill_driver_ios.mm:9: #import
"ios/web/public/navigation_item.h"
On 2017/02/03 at 19:24:16, Eugene But wrote:
> You do need origin_util.h include, but do you need all others?

Woops, refactoring error. Removed.

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
File ios/chrome/browser/autofill/autofill_controller_unittest.mm (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
ios/chrome/browser/autofill/autofill_controller_unittest.mm:599:
TEST_F(AutofillControllerTest, HTTPCreditCard) {
On 2017/02/03 at 19:24:16, Eugene But wrote:
> HttpCreditCard
> 
> Same for other tests

Mmkay. The password tests use the uppercase HTTP convention, though. ;-)

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
File ios/web/public/web_state/web_state.h (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
ios/web/public/web_state/web_state.h:216: // Tells the WebState that the current
page is an HTTP page
On 2017/02/03 at 19:24:16, Eugene But wrote:
> Do you want to use comment from web contents which looks more informative?
>
https://cs.chromium.org/chromium/src/content/public/browser/web_contents.h?ty...
> 
> If so, could you please update OnPasswordInputShownOnHttp comment as well.

I'd rather be a little more general here, in case the functionality needs to
change a little bit. Is that okay?

[Eugene But (OOO till 7-30), 2017-02-04 01:29:17]

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
File ios/chrome/browser/autofill/autofill_controller_unittest.mm (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/chrome/browser/auto...
ios/chrome/browser/autofill/autofill_controller_unittest.mm:599:
TEST_F(AutofillControllerTest, HTTPCreditCard) {
On 2017/02/04 00:28:35, lgarron wrote:
> On 2017/02/03 at 19:24:16, Eugene But wrote:
> > HttpCreditCard
> > 
> > Same for other tests
> 
> Mmkay. The password tests use the uppercase HTTP convention, though. ;-)
Oh, Alright. We quite inconsistent with following C++ Style Guide :)

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
File ios/web/public/web_state/web_state.h (right):

https://codereview.chromium.org/2669123003/diff/50011/ios/web/public/web_stat...
ios/web/public/web_state/web_state.h:216: // Tells the WebState that the current
page is an HTTP page
On 2017/02/04 00:28:35, lgarron wrote:
> On 2017/02/03 at 19:24:16, Eugene But wrote:
> > Do you want to use comment from web contents which looks more informative?
> >
>
https://cs.chromium.org/chromium/src/content/public/browser/web_contents.h?ty...
> > 
> > If so, could you please update OnPasswordInputShownOnHttp comment as well.
> 
> I'd rather be a little more general here, in case the functionality needs to
> change a little bit. Is that okay?
It's ok, but how is this code different from content? If this comment is better
then maybe comments in content should be updated? Generally this is up to you.

[Mathieu, 2017-02-04 02:19:19]

autofill_driver_ios lgtm

[lgarron, 2017-02-06 00:19:48]

The CQ bit was checked by lgarron@chromium.org

[lgarron, 2017-02-06 00:19:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org, mathp@chromium.org
Link to the patchset: https://codereview.chromium.org/2669123003/#ps110001
(title: "Update comments.")

[commit-bot: I haz the power, 2017-02-06 00:19:59]

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-06 00:33:25]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1486340388478510,
"parent_rev": "d7bb23484bdfdc5f4141b7bf9de7815e28d1f1b5", "commit_rev":
"8965c33248c51c71c273b5957344a974142a6914"}

[commit-bot: I haz the power, 2017-02-06 00:34:09]

Description was changed from

==========
iOS: Mark HTTP pages with credit card fields with an omnibox icon.

BUG=648839
================================
TEST=Use an iPhone, not an iPad. First, enable the proper flag:
--------------------------------
1. Open the Settings app
2. Scroll to Chrome Beta/Dev/Canary and press
3. Scroll down to Experimental Settings and press
4. Scroll to EXTRA FLAGS (ONE PER LINE)
5. Toggle "Append Extra Flags" to ON
6. Set Flag1 to "--mark-non-secure-as=show-non-secure-passwords-cc-ui" (without
the quotes)
--------------------------------
Test 3 URLs:
1) Visit https://badssl.com/input/credit-card/ and verify that the omnibox
security has a green lock security indicator to the left of the URL.
2) Visit http://http-credit-card.badssl.com/ and verify that the omnibox
security has a grey info icon ⓘ security indicator to the left of the URL.
3) Visit http://http.badssl.com/ and verify that the omnibox does *not* have a
security indicator to the left of the URL.
--------------------------------
4) Set Flag1 (see above) to "--mark-non-secure-as=neutral" (without the quotes)
and check that http://http-credit-card.badssl.com/ does *not* have a security
indicator to the left of the URL.
================================
==========

to

==========
iOS: Mark HTTP pages with credit card fields with an omnibox icon.

BUG=648839
================================
TEST=Use an iPhone, not an iPad. First, enable the proper flag:
--------------------------------
1. Open the Settings app
2. Scroll to Chrome Beta/Dev/Canary and press
3. Scroll down to Experimental Settings and press
4. Scroll to EXTRA FLAGS (ONE PER LINE)
5. Toggle "Append Extra Flags" to ON
6. Set Flag1 to "--mark-non-secure-as=show-non-secure-passwords-cc-ui" (without
the quotes)
--------------------------------
Test 3 URLs:
1) Visit https://badssl.com/input/credit-card/ and verify that the omnibox
security has a green lock security indicator to the left of the URL.
2) Visit http://http-credit-card.badssl.com/ and verify that the omnibox
security has a grey info icon ⓘ security indicator to the left of the URL.
3) Visit http://http.badssl.com/ and verify that the omnibox does *not* have a
security indicator to the left of the URL.
--------------------------------
4) Set Flag1 (see above) to "--mark-non-secure-as=neutral" (without the quotes)
and check that http://http-credit-card.badssl.com/ does *not* have a security
indicator to the left of the URL.
================================

Review-Url: https://codereview.chromium.org/2669123003
Cr-Commit-Position: refs/heads/master@{#448193}
Committed:
https://chromium.googlesource.com/chromium/src/+/8965c33248c51c71c273b5957344...
==========

[commit-bot: I haz the power, 2017-02-06 00:34:10]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/8965c33248c51c71c273b5957344...