Narrow Bisect:
https://chromium.googlesource.com/chromium/src/+log/5418e9a45cc7e18e81cf6e17aa93d8c340eda329..7ec3781845fc302ff276f1bcd3f95d1499ba4494?pretty=fuller&n=100

Suspect: r471421

Hi there,

have you already started this ticket, if not, I would like to have a go at it.

Cheers

no, I haven't looked at this myself, however, I'm passing this along for further triage

I'm gonna have a go at this myself...

Hello,

I'm attaching a video that better demonstrates the nature of this problem.

I really want to work on this issue. However, I won't go any further because the actual problem lies on feature specs.

The implementation of  crbug.com/589533  indicates that if a form has certain changes (being hidden, for instance), even though no XHR transaction took place, that form should be considered as a successful login, and this is what is happening in here. The new login form in the Google page changes the visibility of the form, to show a new form to another user, and this is causing PasswordAutofillAgent to assume that the form was successful.

I thought of ways that we could narrow down this assumption, but I couldn't come up with an idea. So, I'm not sure how this requirement can be implemented. Having said that, I'm glad to help in any way to get to bottom of it.

Deleted: issue_submit.mov 5.8 MB

issue_submit.mov 5.8 MB Download

Sorry for late reply.

I was looking at code and thinking a little bit. It's pretty tricky case how to detect here whether submission was successful. One option would be to call GetPasswordManagerDriver()->PasswordFormSubmitted instead of GetPasswordManagerDriver()->InPageNavigation in case when the form is disappeared, i.e. DOM_MUTATION_AFTER_XHR happened in all other cases to call GetPasswordManagerDriver()->InPageNavigation (https://cs.chromium.org/chromium/src/components/autofill/content/renderer/password_autofill_agent.cc?type=cs&q=GetPasswordManagerDriver%5C(%5C)-%3EInPageNavigation&sq=package:chromium&l=1176).

The difference is that PasswordFormSubmitted doesn't say anything about a success of the submission, and it will be verified later, InPageNavigation says that the submission is successful.

claudiom...@  sure you're welcome, I would be glad to make a code review. If after my description you still want to fix this, please confirm it and go ahead. The fix itself is just a couple of lines, but it's also needed to test it (see password_autofill_agent_browsertest.cc). It would be ideal to get this fixed till Oct 10. If you have any questions please ask on this bug, I'll try to respond quickly.

 Issue 771520  has been merged into this issue.

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

commit 28d0c4aaba2aedddacd384df63f111f0938cbf42
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Aug 29 14:51:16 2018

Fix false saving positives on accounts.google.com.

In the following case false positive on accounts.google.com happens:

1.Suppose that the user has 2 Gaia credentials (with usernames and passwords
u1/p1 and u2/p2). Let u1 be saved in Password Manager.

2.The user go to accounts.google.com, u1/p1 are autofilled by CPM (a username
field is visible, a password field is invisible).

3.The user is typing u2 in the username field and is clicking next button.

4.At that moment the page removes password form from the DOM and Password
Manager incorrectly thinks that it was successful submission with u2/p1

Video is on  bug 764663  (actual_bubble).

This CL fixes this by ignoring accounts.google.com forms with submitted
type DOM_MUTATION_AFTER_XHR, which means that the form disappeared from the
DOM, without any visible submission.

Similar issues might be on different sites, this CL fixes only
accounts.google.com, because
1.A general solution is unlikely without more complex changes and server-side
support.
2.accounts.google.com is crucial for Chrome, in particular because it serves
the Chrome sign-in page.


Bug:  758155 ,  764663 

Change-Id: I44f9878673bf8f419c49f63220335f6c0f26a6e4
Reviewed-on: https://chromium-review.googlesource.com/1194011
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587115}
[modify] https://crrev.com/28d0c4aaba2aedddacd384df63f111f0938cbf42/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/28d0c4aaba2aedddacd384df63f111f0938cbf42/components/password_manager/core/browser/password_manager_unittest.cc

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

commit 64009bd0adf132b24e4075a1295ea676449af0ac
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Aug 30 08:27:21 2018

Revert "Fix false saving positives on accounts.google.com."

This reverts commit 28d0c4aaba2aedddacd384df63f111f0938cbf42.

Reason for revert: this CL breaks saving passwords on accounts.google.com with second factor enabled.

Original change's description:
> Fix false saving positives on accounts.google.com.
> 
> In the following case false positive on accounts.google.com happens:
> 
> 1.Suppose that the user has 2 Gaia credentials (with usernames and passwords
> u1/p1 and u2/p2). Let u1 be saved in Password Manager.
> 
> 2.The user go to accounts.google.com, u1/p1 are autofilled by CPM (a username
> field is visible, a password field is invisible).
> 
> 3.The user is typing u2 in the username field and is clicking next button.
> 
> 4.At that moment the page removes password form from the DOM and Password
> Manager incorrectly thinks that it was successful submission with u2/p1
> 
> Video is on  bug 764663  (actual_bubble).
> 
> This CL fixes this by ignoring accounts.google.com forms with submitted
> type DOM_MUTATION_AFTER_XHR, which means that the form disappeared from the
> DOM, without any visible submission.
> 
> Similar issues might be on different sites, this CL fixes only
> accounts.google.com, because
> 1.A general solution is unlikely without more complex changes and server-side
> support.
> 2.accounts.google.com is crucial for Chrome, in particular because it serves
> the Chrome sign-in page.
> 
> 
> Bug:  758155 ,  764663 
> 
> Change-Id: I44f9878673bf8f419c49f63220335f6c0f26a6e4
> Reviewed-on: https://chromium-review.googlesource.com/1194011
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587115}

TBR=vabr@chromium.org,dvadym@chromium.org

Change-Id: Idd261e61ca9c7653a1fa72cb175db4afe5ee91fb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  758155 ,  764663 
Reviewed-on: https://chromium-review.googlesource.com/1195511
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587482}
[modify] https://crrev.com/64009bd0adf132b24e4075a1295ea676449af0ac/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/64009bd0adf132b24e4075a1295ea676449af0ac/components/password_manager/core/browser/password_manager_unittest.cc

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

commit 02f40d918c55c943f87d42cb805fd667f7e2fc7d
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Sep 05 17:15:48 2018

Fix false saving positives on accounts.google.com.

In the following case false positive on accounts.google.com happens:

1.Suppose that the user has 2 Gaia credentials (with usernames and passwords
u1/p1 and u2/p2). Let u1 be saved in Password Manager.

2.The user go to accounts.google.com, u1/p1 are autofilled by CPM (a username
field is visible, a password field is invisible).

3.The user is typing u2 in the username field and is clicking next button.

4.At that moment the page removes password form from the DOM and Password
Manager incorrectly thinks that it was successful submission with u2/p1

Video is on  bug 764663  (actual_bubble).

This CL fixes this by ignoring for saving accounts.google.com forms without
a visible password field.

Similar issues might be on different sites, this CL fixes only
accounts.google.com, because
1.A general solution is unlikely without more complex changes and server-side
support.
2.accounts.google.com is crucial for Chrome, in particular because it serves
the Chrome sign-in page.


Bug:  758155 ,  764663 , 880876

Change-Id: I57bbe94ea0a6d101c6a5a33cf354b42f0d0353fc
Reviewed-on: https://chromium-review.googlesource.com/1206476
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588907}
[modify] https://crrev.com/02f40d918c55c943f87d42cb805fd667f7e2fc7d/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/02f40d918c55c943f87d42cb805fd667f7e2fc7d/components/password_manager/core/browser/password_manager_unittest.cc

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

commit c40ebd5dbd53b4acdb954d1c989ca4ed4f03953e
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Sep 12 10:59:55 2018

Fix false saving positives on accounts.google.com.

In the following case false positive on accounts.google.com happens:

1.Suppose that the user has 2 Gaia credentials (with usernames and passwords
u1/p1 and u2/p2). Let u1 be saved in Password Manager.

2.The user go to accounts.google.com, u1/p1 are autofilled by CPM (a username
field is visible, a password field is invisible).

3.The user is typing u2 in the username field and is clicking next button.

4.At that moment the page removes password form from the DOM and Password
Manager incorrectly thinks that it was successful submission with u2/p1

Video is on  bug 764663  (actual_bubble).

This CL fixes this by ignoring for saving accounts.google.com forms without
a visible password field.

Similar issues might be on different sites, this CL fixes only
accounts.google.com, because
1.A general solution is unlikely without more complex changes and server-side
support.
2.accounts.google.com is crucial for Chrome, in particular because it serves
the Chrome sign-in page.


Bug:  758155 ,  764663 , 880876

TBR=dvadym@chromium.org

(cherry picked from commit 02f40d918c55c943f87d42cb805fd667f7e2fc7d)

Change-Id: I57bbe94ea0a6d101c6a5a33cf354b42f0d0353fc
Reviewed-on: https://chromium-review.googlesource.com/1206476
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588907}
Reviewed-on: https://chromium-review.googlesource.com/1221255
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#318}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/c40ebd5dbd53b4acdb954d1c989ca4ed4f03953e/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/c40ebd5dbd53b4acdb954d1c989ca4ed4f03953e/components/password_manager/core/browser/password_manager_unittest.cc