vabr: I was wondering if you had any advice how to implement this. We want the suggestions popup with the "Login not secure" warning in it to show up when a password form is autofilled on page load. (See the Password tab at go/fns-ui-spec for the specs.)

I was looking at PasswordAutofillAgent::FillPasswordForm and thinking that maybe we could trigger the ShowSuggestions() popup from there. (Or maybe trigger something like ShowSuggestions() that asks the driver to show the popup with just the "Login not secure" warning in it, not all the suggestions.) Does that sound reasonable?

Thanks!

This sounds similar to what jww@ started to do for fill-on-account-select (FOAS), the two differences being that in the HttpBad case:
* there is both the autofill and the pop-up (no autofill in FOAS), and
* the filled suggestion should not be shown (all are shown normally in FOAS).

You might have a look at what the FOAS code (guarded by password_manager::features::kFillOnAccountSelect) does to get the pop-up on load. It should be possible to keep autofilling with that, if desired.

I'm not sure what to do with leaving out the fillable suggestions. The mocks also are not clear about what is the condition of when the fillable suggestions should be shown, and what to do when there are more than one fillable suggestion. You can likely customise PasswordAutofillManager::OnShowPasswordSuggestions to do what you want there.


On a more general note, perhaps we should ensure that FOAS and HttpBad are aligned. In particular, if HttpBad wants pop-ups on load, perhaps we can combine it with FOAS and just have the pop-ups without the autofill on HTTP. That should be good for security, and hardly a hit for UX. What do you think?

Thanks! I'll play around with the FOAS code.

emilyschechter: vabr's suggesting that, because we want "Login not secure" dropdowns on page load, we might want to harmonize with fill-on-account-select and basically just do FOAS on HTTP (i.e. show the popup with "Login not secure" and the suggestions on page load, but not do the autofill). That would be ideal from a security perspective, but I seem to recall that UI review didn't want to stop autofilling on HTTP (yet, at least). Am I remembering that correctly?

c#7: Yes, you're correct. I agree FOAS is better, unfortunately we can't do that quite yet. I would think of it as a "next future step" that hopefully we can start doing soon. So I think it would prob be worth it to be comfy with what that code would look like, but it shouldn't be in the initial version.

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

commit 50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7
Author: estark <estark@chromium.org>
Date: Wed Jan 04 01:52:40 2017

Show Form-Not-Secure warning on page load

Currently, the Form-Not-Secure warning shows up in the autofill dropdown
when the autofill suggestions are shown, but it should also show up on page
load when a password form is autofilled.

This CL adds a ShowNotSecureWarning method that roughly mirrors the
Fill-On-Account-Select flow. When a password form is autofilled on page load,
and when the PasswordFormFillData indicates that the warning should be shown,
the renderer calls ShowNotSecureWarning() to tell the browser to show the
autofill popup with the form-not-secure warning (and no autofill suggestions).

BUG= 672668 
TEST=Save a password on http://rsolomakhin.github.io/autofill/ in the
Name/Password form. Load the page again and observe that the field
is autofilled with a "Login not secure" autofill dropdown.

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

[add] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/chrome/renderer/autofill/DEPS
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/chrome/renderer/autofill/fake_content_password_manager_driver.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/chrome/renderer/autofill/fake_content_password_manager_driver.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/common/autofill_driver.mojom
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/renderer/autofill_agent.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/renderer/autofill_agent.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/content/renderer/renderer_save_password_progress_logger_unittest.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/core/common/password_form_fill_data.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/autofill/core/common/password_form_fill_data.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/content/browser/content_password_manager_driver.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/core/browser/password_autofill_manager.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/core/browser/password_autofill_manager.h
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/core/browser/password_autofill_manager_unittest.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/50592c0e5e6c21d5b11f3a5f1515d9afcd5fe1a7/components/password_manager/core/browser/password_manager.cc

Tested the issue on windows 7, Mac 10.12.2, Linux Ubuntu 14.04 using chrome Dev version-57.0.2976.5 with the steps mentioned in comment #0.Observed 'Not secure' page loaded & password autofilled successfully when user selects name from name field.

Please find the attached screen cast for the same.

Adding TE-Verified labels.

Deleted: 672668.mp4 980 KB

	672668.mp4 980 KB View Download

Upon further review, we've decided to disable the "Show on page load" portion of this change; that's tracked in  Issue 685213 .

WontFix per comment 14