New issue
Advanced search Search tips

Issue 847793 link

Starred by 1 user
Status: Fixed
Owner:
dvadym@chromium.org
Closed: Jul 17
Cc:
cfroussios@chromium.org
kolos@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocked on:
issue 840384
Blocking:
issue 838979



Sign in to add a comment

Implement clientside application of autofill filling predictions

Project Member Reported by cfroussios@chromium.org, May 30 2018 
The autofill server makes predictions on the type of info which should be filled into specific form fields. As part of issue 838979, the predictions may also a suggestion to overwrite a prefilled value provided the website.

This bug tracks the clientside implementation of overwriting the prefilled value, according to the server prediction.

Comment 1 by cfroussios@chromium.org, May 30 2018

Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, May 30 2018 

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

commit aa55d35c305eacda33ff0f4b7c3c425a4adbb26d
Author: Christos Froussios <cfroussios@chromium.org>
Date: Wed May 30 16:10:44 2018

[Password Manager] Add prefilled_placeholder flag in autofill server response

The autofill server will start making predictions on whether a form field
contains a prefilled placeholder in the value attribute of a username
field. Prefilled values are sometimes useful and therefore preserved.
However, when playing the role of a placeholder, they should be replaced
with a useful value as if they were a normal placeholder attribute.

This CL simply adds the flag in the proto, so that the source of the flag
is clear during the upcoming refactoring of password manager's filling.

Bug:  847793 
Change-Id: I401c1ed7328949930b6a6d7970088cf148501c96
Reviewed-on: https://chromium-review.googlesource.com/1078808
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562859}
[modify] https://crrev.com/aa55d35c305eacda33ff0f4b7c3c425a4adbb26d/components/autofill/core/browser/proto/server.proto

Comment 3 by cfroussios@chromium.org, Jul 5

Owner: dvadym@chromium.org
Vadym, please close this bug if the client is respecting server-side predictions and placeholder warnings.

Comment 4 by dvadym@chromium.org, Jul 5 

Implementation of filling placeholders with server-side predictions is in progress https://chromium-review.googlesource.com/c/chromium/src/+/1124474

The CL has full implementation, but no tests yet. I'll finish it next week.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 17 

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

commit 4c7b65f6307a2780848d31cb81ede88564377210
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue Jul 17 10:16:53 2018

Fill username field with prefilled values by server hints.

This CL is implementation of overriding of prefilled values in username
fields if the server believes that these values are placeholders.

The life of server prediction for filling prefilled values is the
following (it's called may_use_prefilled_placeholder in code):
1. The server sends it and it's propagated as part of FormStructure to
FormParser (it's implemented before this CL).
2. It's parsed in password_field_prediction.cc to PasswordFieldPrediction
3. FormParser puts it to PasswordForm.
4. In password_form_fill_data.cc it's propagated to PasswordFormFillData
5. It's sent to the renderer over MOJO
6. PasswordAutofillAgent uses it to override prefilled values if any.

The reason of putting processing may_use_prefilled_placeholder in
FormParser not in NewPasswordFormManager is that NewPasswordFormManager
should know nothing about form structure, it's FormParser responsibility
to process it.

Small deletion of dead code is done: parameter |set_selection| from
FillUserNameAndPassword, since it's never set to true.

Bug:  847793 , 831123
Change-Id: Id86d0992428e520718a6ab6e9f50e8064444d030
Reviewed-on: https://chromium-review.googlesource.com/1124474
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575597}
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/core/common/password_form.h
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/core/common/password_form_fill_data.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/core/common/password_form_fill_data.h
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/autofill/core/common/password_form_fill_data_unittest.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/password_manager/core/browser/form_parsing/password_field_prediction.cc
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/password_manager/core/browser/form_parsing/password_field_prediction.h
[modify] https://crrev.com/4c7b65f6307a2780848d31cb81ede88564377210/components/password_manager/core/browser/form_parsing/password_field_prediction_unittest.cc

Comment 6 by dvadym@chromium.org, Jul 17

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 17 

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

commit ec527be908f7e2d3528f65ddaab3b52fc6b51d4e
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Jul 17 12:47:22 2018

Revert "Fill username field with prefilled values by server hints."

This reverts commit 4c7b65f6307a2780848d31cb81ede88564377210.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 575597 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzRjN2I2NWY2MzA3YTI3ODA4NDhkMzFjYjgxZWRlODg1NjQzNzcyMTAM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-dbg/6868

Sample Failed Step: browser_tests

Original change's description:
> Fill username field with prefilled values by server hints.
> 
> This CL is implementation of overriding of prefilled values in username
> fields if the server believes that these values are placeholders.
> 
> The life of server prediction for filling prefilled values is the
> following (it's called may_use_prefilled_placeholder in code):
> 1. The server sends it and it's propagated as part of FormStructure to
> FormParser (it's implemented before this CL).
> 2. It's parsed in password_field_prediction.cc to PasswordFieldPrediction
> 3. FormParser puts it to PasswordForm.
> 4. In password_form_fill_data.cc it's propagated to PasswordFormFillData
> 5. It's sent to the renderer over MOJO
> 6. PasswordAutofillAgent uses it to override prefilled values if any.
> 
> The reason of putting processing may_use_prefilled_placeholder in
> FormParser not in NewPasswordFormManager is that NewPasswordFormManager
> should know nothing about form structure, it's FormParser responsibility
> to process it.
> 
> Small deletion of dead code is done: parameter |set_selection| from
> FillUserNameAndPassword, since it's never set to true.
> 
> Bug:  847793 , 831123
> Change-Id: Id86d0992428e520718a6ab6e9f50e8064444d030
> Reviewed-on: https://chromium-review.googlesource.com/1124474
> Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
> Reviewed-by: Ioana Pandele <ioanap@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Reviewed-by: Vaclav Brozek <vabr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575597}

Change-Id: Ibcea367414987035718f8010320921bc851dc406
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847793 , 831123
Reviewed-on: https://chromium-review.googlesource.com/1140213
Cr-Commit-Position: refs/heads/master@{#575616}
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/core/common/password_form.h
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/core/common/password_form_fill_data.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/core/common/password_form_fill_data.h
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/autofill/core/common/password_form_fill_data_unittest.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/password_manager/core/browser/form_parsing/password_field_prediction.cc
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/password_manager/core/browser/form_parsing/password_field_prediction.h
[modify] https://crrev.com/ec527be908f7e2d3528f65ddaab3b52fc6b51d4e/components/password_manager/core/browser/form_parsing/password_field_prediction_unittest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 18 

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

commit 75aa11b4ebdb5836a83938a867cf5c611dc2f07c
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Jul 18 13:33:58 2018

[Reland] Fill username field with prefilled values by server hints.

This CL is implementation of overriding of prefilled values in username
fields if the server believes that these values are placeholders.

The life of server prediction for filling prefilled values is the
following (it's called may_use_prefilled_placeholder in code):
1. The server sends it and it's propagated as part of FormStructure to
FormParser (it's implemented before this CL).
2. It's parsed in password_field_prediction.cc to PasswordFieldPrediction
3. FormParser puts it to PasswordForm.
4. In password_form_fill_data.cc it's propagated to PasswordFormFillData
5. It's sent to the renderer over MOJO
6. PasswordAutofillAgent uses it to override prefilled values if any.

The reason of putting processing may_use_prefilled_placeholder in
FormParser not in NewPasswordFormManager is that NewPasswordFormManager
should know nothing about form structure, it's FormParser responsibility
to process it.

Small deletion of dead code is done: parameter |set_selection| from
FillUserNameAndPassword, since it's never set to true.

Initial CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1124474

TBR=mkwst@chromium.org

Bug:  847793 , 831123
Change-Id: I5f9a98d755400b2ab9706d27578764a0b46b3f86
Reviewed-on: https://chromium-review.googlesource.com/1140321
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576022}
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/content/renderer/password_autofill_agent.h
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/core/common/password_form.h
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/core/common/password_form_fill_data.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/core/common/password_form_fill_data.h
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/autofill/core/common/password_form_fill_data_unittest.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/password_manager/core/browser/form_parsing/password_field_prediction.cc
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/password_manager/core/browser/form_parsing/password_field_prediction.h
[modify] https://crrev.com/75aa11b4ebdb5836a83938a867cf5c611dc2f07c/components/password_manager/core/browser/form_parsing/password_field_prediction_unittest.cc

Comment 9 by vabr@chromium.org, Nov 29

Cc: -vabr@chromium.org
►

Sign in to add a comment