We could probably use IsPublicSuffixDomainMatch() instead of comparing security origins on equality. I am waiting for the security team to green light this proposal.

Please let me know if I can work on this.

Emily, can you give us security approval for my proposal? If this LGTY, please assign to Nikhil.

My preference would be to explore UI solutions like the key-icon-in-form-field. Is that option on the table? Would this be a temporary stopgap measure?

Do you have any specific threat scenario in mind? The key-icon-in-form-field would probably still feel broken / not particularly assistive.

Talking about a stopgap measure: I think that a key-icon-in-form-field solution would be significantly more work then a one-line change.

Note that the proposal is to only fill passwords into iframes with matching eTLD+1.

No, I don't have any specific attack scenario. I'm just thinking that I would really like to invest in a UI solution that can cover all sorts of situations where it might not be safe or desirable to autofill -- or at least experiment with it, to try to quantify/prove the claim that it would still feel broken.

So, consider this a reluctant security lgtm. :) Assigning back to battre because I'm not sure who Nikhil is (comment #3).

We will look into the key-icon-in-form-field idea as part of Project Maya.

Nikhil, I cannot assign the issue to you, but please work on this.

Please let me know if you face issues or if I should mark the bug as available for other reasons. Thanks.

Hi Battre

Stuck with 2 problems.

1)I tried to check the bug but since I do not have the credentials for login and also creating needs SSN etc which I do not have was not able to reproduce the issue.

2)I still went ahead by changing the code and  used password_manager::IsPublicSuffixDomainMatch in password_autofill_agent.cc but it got compiled but failed in presubmit error check.

** Presubmit ERRORS **
You added one or more #includes that violate checkdeps rules.
  components/autofill/content/renderer/password_autofill_agent.cc
      Illegal include: "components/password_manager/core/browser/psl_matching_helper.h"

Please let me your opinion for above.

@1: In a recent build of Chrome, you can do the following:

1) Navigate to https://www.schwab.com/
2) Type "test"/"test" into Login ID and Password.
3) Click on the lock icon on the right of the omnibar
4) Save the password.
5) Refresh the page and observe the reproduction of the behavior.

@2) You can temporarily copy IsPublicSuffixDomainMatch() and GetRegistryControlledDomain() into password_autofill_agent.cc while you are doing the development and writing unittests.

I am CCing vabr@ to see whether he as a proposal what to do about 2.

Thanks for helping me out for (1) and (2) .
I submitted a working patch https://chromium-review.googlesource.com/c/chromium/src/+/704435 . It is fixing the issue,its just a WIP patch till we finalize the 
solution.

For  Unit test on this , can you please give any example or any pointer or any existing test scenario for this.

Please look at the `git blame` output for the code you modify. This will point you to the CL where the logic was introduced to check the matching of domains of iframes. That CL contains browser tests.

For IsPublicSuffixDomainMatch, one fix would include moving the function somewhere to the core/common of either the autofill or password_manager component, and then using some rules specific to password_autofill_agent.cc in the "specific_include_rules" section in components/autofill/content/renderer/DEPS. But that would complicate both the helper functions layout (more scattered) and potentially also the dependency graph of the two components (which now satisfies the restriction that autofill never depends on password_manager).

There are two sources of this layering problem: (1) Most of the password_autofill_agent.cc logic should be in the password_manager component, and (2) it should be done in browser code. This should be fixed once we finish the refactoring in  bug 564578 .

So my suggestion is indeed to duplicate the code of IsPublicSuffixDomainMatch inside the anonymous namespace in password_autofill_agent.cc and add a comment like this:

// TODO( crbug.com/564578 ): This duplicates code from components/password_manager/core/browser/psl_matching_helper.h. The logic using this code should ultimately end up in components/password_manager/core/browser, at which point it can use the original code directly.

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

commit ece4e7e9c06c8dc95e82bc801264931a09c75887
Author: nikhil <nikhil.sahni@samsung.com>
Date: Tue Oct 17 09:04:40 2017

Relax autofilling passwords in iframes to PSL matching.

Auto fill should work in cases where iframe domain is different
but have same public suffix and the next lower subdomain.

For example, on the homepage of schwab.com, the login form is hosted
in an iframe on a different domain.
(main page: https://www.schwab.com/, iframe: https://lms.schwab.com).
This patch relaxes auto filling only when eTLD+1 is the same for both
the iframe and the main frame.

BUG= 770181 

Change-Id: Idd44aa5a31aaf8ea159e1f7149043ace657be6b0
Reviewed-on: https://chromium-review.googlesource.com/704435
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#509331}
[modify] https://crrev.com/ece4e7e9c06c8dc95e82bc801264931a09c75887/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/ece4e7e9c06c8dc95e82bc801264931a09c75887/components/autofill/content/renderer/password_autofill_agent.cc

Issue 776468 has been merged into this issue.