elawrence, do you think you could take a look at this one next week? I'm not sure exactly how to implement this, so maybe you can dig in a little bit. For passwords, the "Login not secure" suggestion is added at [1]. We early-return a little above that if suggestions.empty() -- maybe we need some logic to not do that early-return if the http warning is going to be added? You could play around to see if that would work.

The corresponding spot for credit cards is [2].

[1] https://cs.chromium.org/chromium/src/components/password_manager/core/browser/password_autofill_manager.cc?q=password_autofill_manager.cc&sq=package:chromium&l=231

[2] https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofill_manager.cc?sq=package:chromium&rcl=1481890797&l=612

Unfortunately, the simple approach of #5 will not work for the password notice. 

In the event that the user hasn't previously stored a password for the site, the target function PasswordAutofillManager::OnShowPasswordSuggestions isn't called because PasswordAutofillAgent::FillPasswordForm isn't called because PasswordManager::AutoFill isn't called because  PasswordFormManager::ProcessFrameInternal bails out when best_matches_ is empty() for a given form. Removing that early return requires many code changes downstream as multiple function calls need to consider the new scenario where best_matches, federated_matches, and preferred_match have not been populated. 

+lshang who has thought about this a little bit

Re #9: I can't find/follow the code path you're describing. My understanding is that ProcessFrameInternal is responsible for autofilling passwords on page load and as of https://codereview.chromium.org/2604453003/ it shows the suggestion popup with the warning when it does so. So the ProcessFrameInternal -> PasswordManager::Autofill -> PasswordAutofillAgent::FillPasswordForm path doesn't have to worry about showing the warning when there are no suggestions; we only show the warning in this code path when the form is autofilled on page load, which implies that there *are* suggestions. (Also, I don't see that PasswordAutofillManager::OnShowPasswordSuggestions gets called at all in that code path... except in the ShowInitialPasswordAccountSuggestions code path, which is for the off-by-default fill-on-account-select experiment. Maybe that's what you're looking at?)

In comment #5 I was looking at the code path where PasswordAutofillManager::OnShowPasswordSuggestions is called via PasswordAutofillAgent::ShowSuggestions [1] which is triggered from e.g. clicking or typing in the input field. In that code path I don't see any other early returns, though I definitely haven't actually verified that to be sure.

[1] https://cs.chromium.org/chromium/src/components/autofill/content/renderer/autofill_agent.cc?dr=CSs&sq=package:chromium&rcl=1483575682&l=668

In my tracing, when typing in a password form with no stored data, PasswordAutofillManager::OnShowPasswordSuggestions won't get called because it early returned in PasswordAutofillAgent::ShowSuggestions[1], when web_input_to_password_info_ is empty, and that map didn't get filled in PasswordAutofillAgent::FillPasswordForm. So #5 is not enough for this case.

Also +mathp for more sights.

[1] https://cs.chromium.org/chromium/src/components/autofill/content/renderer/password_autofill_agent.cc?sq=package:chromium&rcl=1483662145&l=847

I poked around at this a little bit, here's an outline of what seemed to work in my quick-and-dirty testing. Eric, how does the below approach sound to you?

LOGINS:
In PasswordAutofillAgent::ShowSuggestions, we want to show the Not Secure warning even if FindPasswordInfoForElement [1] returns false. So I propose that in that spot we call autofill_agent_->ShowNotSecureWarning(element) if element.isPasswordField() is true or HasAutocompleteAttributeValue(element, "username"). We can check the main frame URL at that point with:

content::IsOriginSecure(url::Origin(render_frame()->GetRenderView()->GetMainRenderFrame()->GetWebFrame()->getSecurityOrigin()).GetURL())

That check might not work from within iframes, especially not out-of-process iframes; I haven't tested it. Getting this to work without iframes (or without OOPIFs) would be a good start.

This will also not really detect usernames, which are detected with a more complicated heuristic in password_form_conversion_utils.cc. We could try tackling that as a follow-up.

[1] https://cs.chromium.org/chromium/src/components/autofill/content/renderer/password_autofill_agent.cc?q=PasswordAutofillAgent::ShowS&sq=package:chromium&l=845

CREDIT CARDS:
AutofillManager::OnQueryFormFieldAutofill seems to get called for credit card fields regardless of whether there is stored autofill data or not, so all we have to do there is pull the Form-Not-Secure out of the !suggestions.empty() conditional. i.e. pull out |type|, |is_filling_credit_card| and |is_context_secure| from [2] up to line 564, and pull the CreateHttpWarningMessageSuggestionItem down to line 652 so that it gets inserted into |suggestions| unconditionally of suggestions.empty().

[2] https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofill_manager.cc?q=OnQueryF&sq=package:chromium&l=568

Thanks, Emily!

https://codereview.chromium.org/2640903006/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68da9592ad01a360f95d866a6b124be8bf766337

commit 68da9592ad01a360f95d866a6b124be8bf766337
Author: elawrence <elawrence@chromium.org>
Date: Wed Jan 25 20:21:03 2017

Show FormNotSecure warnings on sensitive inputs in non-secure contexts

Ensure that we show the on-field warnings for sensitive input fields,
even if the user does not have any stored autofill data for the fields.

BUG= 672663 

Review-Url: https://codereview.chromium.org/2640903006
Cr-Commit-Position: refs/heads/master@{#446104}

[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/chrome/browser/autofill/autofill_interactive_uitest.cc
[add] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/chrome/test/data/autofill/autofill_password_form.html
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/content/renderer/BUILD.gn
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/content/renderer/DEPS
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/68da9592ad01a360f95d866a6b124be8bf766337/components/autofill/core/browser/autofill_manager_unittest.cc

Requesting a merge to M57 for the commit in comment 15

There's a glitch in this fix to be cleaned up in a followup CL.

-if (!is_context_secure && is_http_warning_enabled)

+if (!is_context_secure && is_filling_credit_card && is_http_warning_enabled)

Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

If possible, pls merge your change to M57 branch 2987 before 5:00 PM PT today (Thursday, 01/26) so we can pick it for tomorrow's Dev release.

https://codereview.chromium.org/2660473002/ fixes the problem identified in comment #18. I'm not sure if it makes sense to merge the prior CL to Dev without this fix.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7563d11df963b1807dc96bc530a6a870fb1db222

commit 7563d11df963b1807dc96bc530a6a870fb1db222
Author: elawrence <elawrence@chromium.org>
Date: Thu Jan 26 22:15:49 2017

Ensure field type is Payment before showing Payment Not Secure Warning

The Form Not Secure warning must verify that the form field belongs to
a payment form before showing the "Payment Not Secure" warning.

BUG= 672663 

Review-Url: https://codereview.chromium.org/2660473002
Cr-Commit-Position: refs/heads/master@{#446468}

[modify] https://crrev.com/7563d11df963b1807dc96bc530a6a870fb1db222/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/7563d11df963b1807dc96bc530a6a870fb1db222/components/autofill/core/browser/autofill_manager_unittest.cc

Pls merge your change to M57 branch 2987 before 5:00 PM PT Monday (01/30) so we can pick it up for next week Last M57  Dev release. Thank you.

Merge should block on  Issue 672663  and Issue 685982. We should not merge to M57 without those fixes coming along in the same release.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The fix for Issue 685982 has landed; after it bakes for Canary and is eligible for merge, we can merge all three necessary CLs to M57:

  68da9592ad01a360f95d866a6b124be8bf766337
  7563d11df963b1807dc96bc530a6a870fb1db222 
  aaf3b71d6e6d9c1ce38fd741c0dbfdf28085fdf0

Optionally, we should consider merging the fix for  Issue 685769  at the same time (it's not a blocker, but it's a definite nice-to-have).

Thank you  elawrence@. Please request a merge for Issue 685982 and  Issue 685769  if canary data looks good. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61c2fc567134019d5af000eb9606169f9fe395b5

commit 61c2fc567134019d5af000eb9606169f9fe395b5
Author: Emily Stark <estark@google.com>
Date: Thu Feb 02 18:38:58 2017

Show FormNotSecure warnings on sensitive inputs in non-secure contexts

Ensure that we show the on-field warnings for sensitive input fields,
even if the user does not have any stored autofill data for the fields.

BUG= 672663 

Review-Url: https://codereview.chromium.org/2640903006
Cr-Commit-Position: refs/heads/master@{#446104}
(cherry picked from commit 68da9592ad01a360f95d866a6b124be8bf766337)

Review-Url: https://codereview.chromium.org/2676653004 .
Cr-Commit-Position: refs/branch-heads/2987@{#270}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/chrome/browser/autofill/autofill_interactive_uitest.cc
[add] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/chrome/test/data/autofill/autofill_password_form.html
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/content/renderer/BUILD.gn
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/content/renderer/DEPS
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/61c2fc567134019d5af000eb9606169f9fe395b5/components/autofill/core/browser/autofill_manager_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/595def03304d70b51fddb4ccbd75f56339d4928c

commit 595def03304d70b51fddb4ccbd75f56339d4928c
Author: Emily Stark <estark@google.com>
Date: Thu Feb 02 18:40:25 2017

Ensure field type is Payment before showing Payment Not Secure Warning

The Form Not Secure warning must verify that the form field belongs to
a payment form before showing the "Payment Not Secure" warning.

BUG= 672663 

Review-Url: https://codereview.chromium.org/2660473002
Cr-Commit-Position: refs/heads/master@{#446468}
(cherry picked from commit 7563d11df963b1807dc96bc530a6a870fb1db222)

Review-Url: https://codereview.chromium.org/2674743002 .
Cr-Commit-Position: refs/branch-heads/2987@{#271}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/595def03304d70b51fddb4ccbd75f56339d4928c/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/595def03304d70b51fddb4ccbd75f56339d4928c/components/autofill/core/browser/autofill_manager_unittest.cc