New issue
Advanced search Search tips

Issue 905982 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Server predictions are ignored when they are cached

Project Member Reported by dvadym@chromium.org, Nov 16

Issue description

Now a cache is used for Autofill server predictions. Predictions are cached for 1 week. If form server predictions are cached, then PasswordManager receives
them earlier than learns about the corresponding form in DOM. And predictions are lost since NewPasswordFormManager is created later. 

So now only the first time in 1 week when the user goes to a page with sign-in form it leads to filling with server help.

The predictions might be saved in PasswordManager and as soon as NewPasswordFormManager they might be passed to it.



 
Fixing this bug should fix filling on 5-7% of sites. Among them are 
1.baidu.com
2.postbank.de
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 16

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

commit 59684986c7a2094f3ee285fee1281e059ba8b6d4
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Fri Nov 16 13:17:44 2018

Fix server predictions race conditions in NewPasswordFormManager.

If server predicitons are cached, then often PasswordManager receives
them earlier than learns about a corresponding form in DOM. So
NewPasswordFormManager is created later. And predictions are lost.
This CL fixes this, by caching predictions in PasswordManager.

Bug: 905982, 831123


Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
Reviewed-on: https://chromium-review.googlesource.com/c/1338101
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608750}
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/59684986c7a2094f3ee285fee1281e059ba8b6d4/components/password_manager/core/browser/password_manager_unittest.cc

Labels: RegressedIn-71 Merge-Request-71
This is a pretty simple change that fixes Password Manager on a lot of sites.
Project Member

Comment 4 by sheriffbot@chromium.org, Nov 19

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: Less than 11 days to go before AppStore submit on M71
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #3 and per offline chat with dvadym@. Please merge now. 
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 19

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa381092b25edb73ad4b5b8713d7e99ee32eafae

commit aa381092b25edb73ad4b5b8713d7e99ee32eafae
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Nov 19 18:50:44 2018

Fix server predictions race conditions in NewPasswordFormManager.

If server predicitons are cached, then often PasswordManager receives
them earlier than learns about a corresponding form in DOM. So
NewPasswordFormManager is created later. And predictions are lost.
This CL fixes this, by caching predictions in PasswordManager.

Bug: 905982, 831123

TBR=dvadym@chromium.org

(cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)

Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
Reviewed-on: https://chromium-review.googlesource.com/c/1338101
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#608750}
Reviewed-on: https://chromium-review.googlesource.com/c/1342917
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#757}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/aa381092b25edb73ad4b5b8713d7e99ee32eafae/components/password_manager/core/browser/password_manager_unittest.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/aa381092b25edb73ad4b5b8713d7e99ee32eafae

Commit: aa381092b25edb73ad4b5b8713d7e99ee32eafae
Author: dvadym@chromium.org
Commiter: dvadym@chromium.org
Date: 2018-11-19 18:50:44 +0000 UTC

Fix server predictions race conditions in NewPasswordFormManager.

If server predicitons are cached, then often PasswordManager receives
them earlier than learns about a corresponding form in DOM. So
NewPasswordFormManager is created later. And predictions are lost.
This CL fixes this, by caching predictions in PasswordManager.

Bug: 905982, 831123

TBR=dvadym@chromium.org

(cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)

Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
Reviewed-on: https://chromium-review.googlesource.com/c/1338101
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#608750}
Reviewed-on: https://chromium-review.googlesource.com/c/1342917
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#757}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ca601e62d07e3d573b8adb41eec3cd8817fff16f

Commit: ca601e62d07e3d573b8adb41eec3cd8817fff16f
Author: govind@chromium.org
Commiter: govind@chromium.org
Date: 2018-11-20 04:58:48 +0000 UTC

Revert "Fix server predictions race conditions in NewPasswordFormManager."

This reverts commit aa381092b25edb73ad4b5b8713d7e99ee32eafae.

Reason for revert: Breaks Android M71 build, see bug 906953

Original change's description:
> Fix server predictions race conditions in NewPasswordFormManager.
> 
> If server predicitons are cached, then often PasswordManager receives
> them earlier than learns about a corresponding form in DOM. So
> NewPasswordFormManager is created later. And predictions are lost.
> This CL fixes this, by caching predictions in PasswordManager.
> 
> Bug: 905982, 831123
> 
> TBR=dvadym@chromium.org
> 
> (cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)
> 
> Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1338101
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#608750}
> Reviewed-on: https://chromium-review.googlesource.com/c/1342917
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#757}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=dvadym@chromium.org

Change-Id: I894df318b37c01a8a326f5cd1060f0e9bcce3e9b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 905982, 831123
Reviewed-on: https://chromium-review.googlesource.com/c/1343535
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#770}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 20

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

commit ca601e62d07e3d573b8adb41eec3cd8817fff16f
Author: Krishna Govind <govind@chromium.org>
Date: Tue Nov 20 04:58:48 2018

Revert "Fix server predictions race conditions in NewPasswordFormManager."

This reverts commit aa381092b25edb73ad4b5b8713d7e99ee32eafae.

Reason for revert: Breaks Android M71 build, see bug 906953

Original change's description:
> Fix server predictions race conditions in NewPasswordFormManager.
> 
> If server predicitons are cached, then often PasswordManager receives
> them earlier than learns about a corresponding form in DOM. So
> NewPasswordFormManager is created later. And predictions are lost.
> This CL fixes this, by caching predictions in PasswordManager.
> 
> Bug: 905982, 831123
> 
> TBR=dvadym@chromium.org
> 
> (cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)
> 
> Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1338101
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#608750}
> Reviewed-on: https://chromium-review.googlesource.com/c/1342917
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#757}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

TBR=dvadym@chromium.org

Change-Id: I894df318b37c01a8a326f5cd1060f0e9bcce3e9b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 905982, 831123
Reviewed-on: https://chromium-review.googlesource.com/c/1343535
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#770}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/ca601e62d07e3d573b8adb41eec3cd8817fff16f/components/password_manager/core/browser/password_manager_unittest.cc

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

Commit: c1ece4a3cba2e69060db15812e2ece78fe681c85
Author: dvadym@chromium.org
Commiter: dvadym@chromium.org
Date: 2018-11-20 13:07:57 +0000 UTC

Reland "Fix server predictions race conditions in NewPasswordFormManager."

This is a reland of aa381092b25edb73ad4b5b8713d7e99ee32eafae

Original change's description:
> Fix server predictions race conditions in NewPasswordFormManager.
> 
> If server predicitons are cached, then often PasswordManager receives
> them earlier than learns about a corresponding form in DOM. So
> NewPasswordFormManager is created later. And predictions are lost.
> This CL fixes this, by caching predictions in PasswordManager.
> 
> Bug: 905982, 831123
> 
> TBR=dvadym@chromium.org
> 
> (cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)
> 
> Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1338101
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#608750}
> Reviewed-on: https://chromium-review.googlesource.com/c/1342917
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#757}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Bug: 905982, 831123
Change-Id: I95ebcac7bdf65e05a50b07e3a6764eb2aac8cfbc
Reviewed-on: https://chromium-review.googlesource.com/c/1343114
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#773}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 20

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

commit c1ece4a3cba2e69060db15812e2ece78fe681c85
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue Nov 20 13:07:57 2018

Reland "Fix server predictions race conditions in NewPasswordFormManager."

This is a reland of aa381092b25edb73ad4b5b8713d7e99ee32eafae

Original change's description:
> Fix server predictions race conditions in NewPasswordFormManager.
> 
> If server predicitons are cached, then often PasswordManager receives
> them earlier than learns about a corresponding form in DOM. So
> NewPasswordFormManager is created later. And predictions are lost.
> This CL fixes this, by caching predictions in PasswordManager.
> 
> Bug: 905982, 831123
> 
> TBR=dvadym@chromium.org
> 
> (cherry picked from commit 59684986c7a2094f3ee285fee1281e059ba8b6d4)
> 
> Change-Id: I833fe675c3c621edbae31d5b449e274c7d5a33e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1338101
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#608750}
> Reviewed-on: https://chromium-review.googlesource.com/c/1342917
> Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
> Cr-Commit-Position: refs/branch-heads/3578@{#757}
> Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Bug: 905982, 831123
Change-Id: I95ebcac7bdf65e05a50b07e3a6764eb2aac8cfbc
Reviewed-on: https://chromium-review.googlesource.com/c/1343114
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#773}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/c1ece4a3cba2e69060db15812e2ece78fe681c85/components/password_manager/core/browser/password_manager_unittest.cc

Cc: -vabr@chromium.org
vabr going hobby only -> reducing involvement.
Please contact me directly in urgent matters.

Sign in to add a comment