New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 871191 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 20
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-20
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Saved password entry gets deleted after sign into chrome.

Reported by pranjali...@etouch.net, Aug 6

Issue description

Chrome version : 70.0.3514.0 (Official Build) e359e11e00b7a42e1b5f7fee1f89b38a48c2dc60-refs/branch-heads/3514@{#1}(32/64-bit) 

OS :Win(7,8,8.1,10) ,Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14)  and Linux(14.04 LTS)  OS

Steps to reproduce:
1. Launch chrome ,sign into gmail account with valid credentials and save password.
2. Repeat step 2 (such that two password entries should be seen in 'chrome://settings/passwords').
3. Now sign into chrome with first username/password(Kindly  fill the username and password manually and do not select from autofill suggestions.)
4. Now navigate to 'chrome://settings/passwords' and observe.

Actual Result: Saved entry of gmail with which sign in to chrome is done that entry gets deleted automatically from chrome://settings/password
Expected Result:  Saved password entry should not be deleted after sign into chrome.

This is a regression issue broken in ‘M-69’ and below is bisect info.
Good build: 69.0.3481.0
Bad build: 69.0.3482.0

You are probably looking for a change made after 572584 (known good), but no later than 572585 (first known bad).
CHANGELOG URL:

The script might not always return single CL as suspect as some perf builds might get missing due to failure.

  https://chromium.googlesource.com/chromium/src/+log/e3b5b9701623cfcd2b8524cd2aab4bf700434eac..7a9340678c5ceb4c06abc270beea92aa0c999957

Suspect: https://chromium.googlesource.com/chromium/src/+/7a9340678c5ceb4c06abc270beea92aa0c999957

@dvadym: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank You

 
Actual_result.mp4
1.6 MB View Download
Expected_result.mp4
1.6 MB View Download
Cc: pbomm...@chromium.org manoranj...@chromium.org
Labels: ReleaseBlock-Stable
Adding release blocker label for this issue.Please reduce priority or remove if not the case.

Thank You!
M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
Status: Started (was: Assigned)
Cc: linds...@chromium.org
M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.


I've found a solution for this problem, the CL is almost ready https://chromium-review.googlesource.com/c/chromium/src/+/1174714 and it will be landed on Thursday
Thank you, pls request a merge to M69 once CL listed at #6 is baked/verified in canary and safe to merge. 
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 16

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

commit 015b85bca4c129d883485862c87ee18ca928d2d8
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Aug 16 14:09:40 2018

[Password Manager] Do not fill gaia reath and chrome sign-in forms.

These forms should not be filled. This CL implements that nor
PasswordFormManager nor NewPasswordFormManager is created for such forms
so as result they are not filled.

Bug:871191

Change-Id: I509bb6d1ca500ac2bbbea265712001f61bec3c53
Reviewed-on: https://chromium-review.googlesource.com/1174714
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583637}
[modify] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/chrome/browser/password_manager/password_manager_browsertest.cc
[add] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/chrome/test/data/password/gaia_reath_form.html
[modify] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/components/password_manager/core/browser/test_password_store.cc
[modify] https://crrev.com/015b85bca4c129d883485862c87ee18ca928d2d8/components/password_manager/core/browser/test_password_store.h

Update :We have few test failures on trunk builders due to this crash and here is the tracking bug for the same  issue#875052 
NextAction: 2018-08-17
Pls request a merge to M69 branch once CL is baked/verified in canary and test failure reported at #9 is addressed. 
The NextAction date has arrived: 2018-08-17
Test failure from comment #9 is fixed. I'll ask for a merge as soon as it is verified.
NextAction: 2018-08-20
Pls update bug with canary result on Monday morning including fix for test failure. If all looks good and safe to merge, pls request a merge to M69.
Labels: Merge-Request-69
 Issue 875052  fixed.
Project Member

Comment 15 by sheriffbot@chromium.org, Aug 20

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
The NextAction date has arrived: 2018-08-20
Update:
Rechecked the above issue on Win(7,8,8.1,10) ,Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14)  and Linux(14.04 LTS)  OS using latest canary build #70.0.3528.0 and issue is fix.
Please refer attached screencast for reference.

Thank you... 
Canary Behaviour.mp4
1.6 MB View Download
Labels: TE-Verified-M70 TE-Verified-70.0.3528.0
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comments #14 and #18. Please merge now and mark bug as fixed after the merge if nothing else is pending.

Do we also need a merge for  Issue 875052   to M69? 
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 20

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38b53f808f510a6bcac4b2e8f98e2865a989cc87

commit 38b53f808f510a6bcac4b2e8f98e2865a989cc87
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Aug 20 15:38:50 2018

[Password Manager] Do not fill gaia reath and chrome sign-in forms.

These forms should not be filled. This CL implements that nor
PasswordFormManager nor NewPasswordFormManager is created for such forms
so as result they are not filled.

Bug:871191

TBR=dvadym@chromium.org

(cherry picked from commit 015b85bca4c129d883485862c87ee18ca928d2d8)

Change-Id: I509bb6d1ca500ac2bbbea265712001f61bec3c53
Reviewed-on: https://chromium-review.googlesource.com/1174714
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#583637}
Reviewed-on: https://chromium-review.googlesource.com/1181341
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#710}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/chrome/browser/password_manager/password_manager_browsertest.cc
[add] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/chrome/test/data/password/gaia_reath_form.html
[modify] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/components/password_manager/core/browser/test_password_store.cc
[modify] https://crrev.com/38b53f808f510a6bcac4b2e8f98e2865a989cc87/components/password_manager/core/browser/test_password_store.h

Status: Fixed (was: Started)
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 28

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

commit c6ccb877f3fb45516a98384ae016f0e1831bc376
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue Aug 28 21:56:22 2018

Wipe out WipeStoreCopyIfOutdated.

Password Manager ignores saving of Chrome sync credentials. But if
accounts.google.com credentials were saved before signing in in Chrome,
they persist. Which may lead to the problem, that after changing
password, saved sync credentials become obsolete. For this reason it
was introduced wiping such credentials if Password Manager found
submission with different password. The problem that it assumes that
Password Manager always correctly detects success of password form
submission on accounts.google.com. That's not true anymore. As result
valid credentials might be removed. In the current setup probability
of this is low, but with other changes it might become much bigger, for
example  https://crbug.com/871191  is example. So it's the time to remove
wiping.

Bug:  871191 , 875768,  458279 
Change-Id: I24c096f6202bf21a89d091932b22984eed8efe0a
Reviewed-on: https://chromium-review.googlesource.com/1183308
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586871}
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/form_saver.h
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/form_saver_impl.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/form_saver_impl.h
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/form_saver_impl_unittest.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/password_form_manager.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/password_form_manager.h
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/password_manager_unittest.cc
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/components/password_manager/core/browser/stub_form_saver.h
[modify] https://crrev.com/c6ccb877f3fb45516a98384ae016f0e1831bc376/tools/metrics/histograms/histograms.xml

Labels: TE-Verified-69.0.3497.72 TE-Verified-M69
Tested the issue on Windows-10,Mac 10.13.6 & Debian Rodete using chrome latest Beta M69-69.0.3497.72 by following steps mentioned in the original comment. Observed that saved password details are displaying as expected. Hence adding TE-Verified label.

Please find the screen cast(Win) for reference.

Thank you!
871191.mp4
4.1 MB View Download

Sign in to add a comment