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

Issue 845426 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
hobby only
Closed: Nov 6
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 854130



Sign in to add a comment

Parse FormData in browser

Project Member Reported by vabr@chromium.org, May 22 2018

Issue description

This is the refactoring bringing parsing of FormData into PasswordForm to browser.

Design doc: https://docs.google.com/document/d/1KxWnt3-Pykz4ut4P1IHxNhn6Ef64v3VMyGobUWyLnDg/edit?usp=sharing

Bigger context of refactoring PasswordFormManager: https://docs.google.com/document/d/1fgh8u4cUP45XGEydGD9MTF8z-fpM3M6hFkn4foc0Zh0/edit?usp=sharing

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 23 2018

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

commit 183570c8b65c10953856c25b305e83ef4ae4b41a
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed May 23 13:03:16 2018

Enable password manager parsing unittests

By accident, the unit_tests target of
components/password_manager/core/browser/form_parsing/BUILD.gn was not
included in components_unittests. As a result, those tests never ran.

This CL adds the missing dependency, so that, in particular, the
IOSFormParserTest gets run as part of components_unittests.

Bug:  845426 
Change-Id: I49b5ea80a90b1bb33cab103b8cea51c7bdc9f988
Reviewed-on: https://chromium-review.googlesource.com/1070153
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561049}
[modify] https://crrev.com/183570c8b65c10953856c25b305e83ef4ae4b41a/components/password_manager/core/browser/BUILD.gn

Comment 2 by vabr@chromium.org, May 23 2018

A benchmark to support a particular design choice for filtering vectors: https://crrev.com/c/1070189

Comment 3 by vabr@chromium.org, May 29 2018

Description: Show this description
Project Member

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

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

commit 4703e86794f4fe564f5c7918576aa439a3c36c46
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed May 30 17:28:55 2018

Introduce browser-side PasswordForm parser

PasswordForm objects are obtained by parsing FormData. Except for iOS,
this parsing happens in the renderer.

This CL adds a FormData -> PasswordForm parser on the browser side. It
is used by the NewPasswordFormManager and will eventually replace the
renderer-side parser.

The parser is not completed yet, but provides some basic functionality
and includes unit tests and integration into fuzzers.

Bug:  845426 
Change-Id: Idfe8eedbb9ba04751cf3f4fd004aff5499fe44c7
Reviewed-on: https://chromium-review.googlesource.com/1069355
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562888}
[modify] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/BUILD.gn
[add] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/form_parser.cc
[add] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/form_parser.h
[add] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
[modify] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/fuzzer/BUILD.gn
[add] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/fuzzer/form_parser_fuzzer_generic.cc
[add] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/form_parsing/fuzzer/form_parser_proto_fuzzer_generic.cc
[modify] https://crrev.com/4703e86794f4fe564f5c7918576aa439a3c36c46/components/password_manager/core/browser/new_password_form_manager.cc

Project Member

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

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

commit 08608724870b92d5d488776c9ea2010c99625c13
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed May 30 20:50:59 2018

Use std::make_reverse_iterator in password_manager

This CL adds the first use of std::make_reverse_iterator, inside the
password_manager component at a place where it shortens an awkwardly
long for-loop header.

It also adds std::make_reverse_iterator to the allowed C++14 features.

Bug:  845426 
Change-Id: Ief6983c167b3c85a7d52a25555e8157d4d0f0a1c
Reviewed-on: https://chromium-review.googlesource.com/1078758
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562987}
[modify] https://crrev.com/08608724870b92d5d488776c9ea2010c99625c13/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/08608724870b92d5d488776c9ea2010c99625c13/styleguide/c++/c++11.html

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2018

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

commit 883eb5c7165179620bf596031cc1cf29b7e597f5
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Jun 01 16:59:19 2018

Fix password fuzzer targets to end in "_fuzzer"

Two fuzzer targets in the password_manager component had names not
ending in "_fuzzer". This might confuse fuzzing infrastructure, so this
CL renames those targets. It also renames the corresponding .cc files to
match the target name, so that it does not look confusing and inviting
for a "fix" in the wrong direction.

Bug:  845426 
Change-Id: I94cb95204ee4026937a94e08776b737bc316f7b7
Reviewed-on: https://chromium-review.googlesource.com/1082535
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563687}
[modify] https://crrev.com/883eb5c7165179620bf596031cc1cf29b7e597f5/components/password_manager/core/browser/form_parsing/fuzzer/BUILD.gn
[rename] https://crrev.com/883eb5c7165179620bf596031cc1cf29b7e597f5/components/password_manager/core/browser/form_parsing/fuzzer/form_parser_generic_fuzzer.cc
[rename] https://crrev.com/883eb5c7165179620bf596031cc1cf29b7e597f5/components/password_manager/core/browser/form_parsing/fuzzer/form_parser_proto_generic_fuzzer.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 4 2018

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

commit a60a9339003b5fe452d959437c5cc1ed0b831b70
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon Jun 04 14:57:28 2018

Password form parser: readonly support

This CL teaches the new FormData->PasswordForm parser the meaning of
readonly fields:

* No difference for usernames.
* Readonly passwords are to be ignored unless there are explicit hints
  for them:
    - autocomplete mark-up
    - past typing or autofill in that field
    - (server-side data, which is not handled in this CL yet)

Bug:  845426 
Change-Id: Idfa8d8316cbdead5aaf97390ef1f18c2f77066dd
Reviewed-on: https://chromium-review.googlesource.com/1082537
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564092}
[modify] https://crrev.com/a60a9339003b5fe452d959437c5cc1ed0b831b70/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/a60a9339003b5fe452d959437c5cc1ed0b831b70/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 6 2018

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

commit 7beaf449ab922efbfae9a074f653daf0966d2ec5
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jun 06 09:24:53 2018

Clean-up password manager's form_parser.*

This CL replaces a redundant type mention with 'auto' in form_parser.cc
makes the use of trailing ',' consistent and removes unnecessary {} from
form_parser_unittest.cc.

Bug:  845426 
Change-Id: I292af1e436d9b7e98880b3392f0232d88a81e84b
Reviewed-on: https://chromium-review.googlesource.com/1087267
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564834}
[modify] https://crrev.com/7beaf449ab922efbfae9a074f653daf0966d2ec5/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/7beaf449ab922efbfae9a074f653daf0966d2ec5/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Comment 9 by vabr@chromium.org, Jun 6 2018

Description: Show this description
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 12 2018

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

commit 107af572d76570c8f3b946a8908af423922fc614
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Jun 12 12:27:38 2018

Teach form_parser use server hints

This CL extends the browser-side FormData parser in password manager to
use server predictions to determine roles of input fields.

This support is incomplete and does not handle well partial predictions
yet, e.g., if only the USERNAME is predicted. It also does not handle
the "NOT_PASSWORD" prediction. Those will get addressed in separate CLs.

Bug:  845426 
Change-Id: If8e444fb865c2c8358b122fed875386a9aa05ec8
Reviewed-on: https://chromium-review.googlesource.com/1087066
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566388}
[modify] https://crrev.com/107af572d76570c8f3b946a8908af423922fc614/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/107af572d76570c8f3b946a8908af423922fc614/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
[modify] https://crrev.com/107af572d76570c8f3b946a8908af423922fc614/components/password_manager/core/browser/form_parsing/password_field_prediction.cc
[modify] https://crrev.com/107af572d76570c8f3b946a8908af423922fc614/components/password_manager/core/browser/form_parsing/password_field_prediction.h
[modify] https://crrev.com/107af572d76570c8f3b946a8908af423922fc614/components/password_manager/core/browser/form_parsing/password_field_prediction_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 13 2018

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

commit ed731074b350ddb4f9f06bccd625a32e04d38380
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jun 13 13:21:11 2018

Add ProcessedField in FormData parser

When parsing FormData, a few bits of information derivable from a
FormData instance deserve to be cached:
(1) Whether the field is a password field or an unmasked field.
(2) What is the parsing result of its autocomplete attribute.
(3) (In near future,) whether the the field has been typed into.

Currently, (1) is always recomputed on demand and (2) is held in an
external cache. (3) is not needed yet, but will be, and more cases may
come.

Therefore, this CL adds an internal ProcessedField structure to the
parser, which allows wrapping the FormData together with those derived
data. That spares the need to recompute the derived data, and also
stores it at a natural place, avoiding the need to do caching and
operate with std::map.

The CL also migrates the AutocompleteFlag enum values from MACRO_STYLE
to kConstantStyle. The latter seems preferred by the current style
guide.

Bug:  845426 
Change-Id: I19c84e7f6e8dcc5535faf392523a71ca6f72e23d
Reviewed-on: https://chromium-review.googlesource.com/1098920
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566814}
[modify] https://crrev.com/ed731074b350ddb4f9f06bccd625a32e04d38380/components/password_manager/core/browser/form_parsing/form_parser.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 14 2018

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

commit 6a946e627c7e3c81fa87f41de00867d9ed7a0d63
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Jun 14 12:45:57 2018

Teach form parser consider interactability

The FormData -> PasswordForm password needs to distinguish three levels
of interactability of fields: fields already interacted with, fields
likely to be interacted with, and those which are unlikely.

This is what the old parser in password_form_conversion_utils.cc called
filtering levels.

The interactability is useful when identifying whether an obtained
result is potentially useful. Also, the structural analysis only
should consider the fields with best interactability.

This CL introduces the concept of interactability and restricts the
structural analysis ("base heuristics") to fields with best
interactability.

The CL does not introduce interactability-based penalties yet, because
the confidence rating is not implemented yet. (See details in the design
doc linked from the bug description.)

Bug:  845426 
Change-Id: Ib0dd2f9837c537e4d64432a35b7ba2f5f8e72403
Reviewed-on: https://chromium-review.googlesource.com/1099060
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567232}
[modify] https://crrev.com/6a946e627c7e3c81fa87f41de00867d9ed7a0d63/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/6a946e627c7e3c81fa87f41de00867d9ed7a0d63/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 15 2018

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

commit dd61f83e8978d5bb26f829b354af28a063749d79
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Jun 15 17:34:49 2018

FormData parser generates all_possible_passwords

This CL teaches the new FormData->PasswordForm parser to generate
PasswordForm::all_possible_passwords.

Bug:  845426 
Change-Id: Ic75a3ade7b5ed62eaf4462ea5d00cc053c12c512
Reviewed-on: https://chromium-review.googlesource.com/1100883
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567713}
[modify] https://crrev.com/dd61f83e8978d5bb26f829b354af28a063749d79/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/dd61f83e8978d5bb26f829b354af28a063749d79/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 15 2018

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

commit 6fd466d379736f66ee5e1ce64d45b472d712e2f8
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri Jun 15 19:20:53 2018

Add basic fields to PasswordForm in new parser

This CL lets the new FormData -> PasswordForm parser initialize some
fields which always have the same value in this context:
scheme, preferred, blacklisted_by_user, type

Bug:  845426 
Change-Id: I66f83bbc4f2031c4eb349786694110a4a3bbd89e
Reviewed-on: https://chromium-review.googlesource.com/1100885
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567761}
[modify] https://crrev.com/6fd466d379736f66ee5e1ce64d45b472d712e2f8/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/6fd466d379736f66ee5e1ce64d45b472d712e2f8/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Comment 15 by vabr@chromium.org, Jun 19 2018

Blockedon: 854123
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 19 2018

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

commit 07a7b5a5d98f4be9d61bc5b2628b1eb012417c91
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Jun 19 10:32:55 2018

Username predictions into FormData

Username predictions list text field from a form in a descending
likelihood that they are usernames. The predictions are obtained by
running a classifier (locally) on the DOM tree containing the form. They
are represented as a vector of unique renderer ids of the identified
fields.

This CL makes the username predictions part of FormData and teaches the
new FormData -> PasswordForm parser to use them.

FormData serves both address autofill and password autofill, yet
username predictions are only relevant to the latter. In this sense,
password autofill is polluting the struct for address autofill. However,
given the close relationship between the predictions and the actual
FormFieldData they refer to, and given the not-so-high overhead of
passing around an empty vector (compared to the overall size of
FormData), encapsulating the predictions with FormData was chosen as the
approach to go with.

Bug:  845426 
Change-Id: I92ca2eb8d2a24d08541e4878a0732092a473c74f
Reviewed-on: https://chromium-review.googlesource.com/1101027
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568393}
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/content/common/autofill_types_struct_traits_unittest.cc
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/core/common/form_data.cc
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/autofill/core/common/form_data.h
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/password_manager/core/browser/form_parsing/BUILD.gn
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/07a7b5a5d98f4be9d61bc5b2628b1eb012417c91/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Comment 17 by vabr@chromium.org, Jun 19 2018

Blockedon: 854130

Comment 18 by vabr@chromium.org, Jun 19 2018

Blockedon: 854147

Comment 19 by vabr@chromium.org, Jun 25 2018

Blockedon: 855987
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 27 2018

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

commit beabf6b716772d369f4cdc286af5f3793d0b1e2c
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jun 27 14:38:02 2018

FormData parser: no confirmation field without new password

This CL teached the FormData -> PasswordForm parser to prevent the
conclusion that there is a confirmation password field on a form which
has no new password field. This state does not correspond to reality.

Therefore this CL adds a DCHECK to verify this and also a sanitization
step for processing the server data.

Bug:  845426 
Change-Id: I6ffffb31766f9271f0f8021f3aa249d1ee163730
Reviewed-on: https://chromium-review.googlesource.com/1116920
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570764}
[modify] https://crrev.com/beabf6b716772d369f4cdc286af5f3793d0b1e2c/components/password_manager/core/browser/form_parsing/form_parser.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 2

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

commit b0a6421113c7d8f30aea9eabef72cbf1a9ecf494
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon Jul 02 17:45:19 2018

FormData parser tries to find missing username

When parsing FormData into PasswordForm, the new parser tries to keep
results from different types of analysis separate: if autocomplete
attributes or server hints only provide the password fields, the
parser currently does not try to figure out username with structural
analysis.

The server data currently lack username quite often, and also
autocomplete mark-up often lacks the username. As a result, the new
parser fails to find the username in many cases when the old parser
could.

Once server data improve, this issue will get much smaller (the
autocomplete data will also get replaced by server hints). However, it
is unlikely that this happens soon enough. Therefore, to keep the
effect of the change of parsers in M69 small, this CL adds the ability
to merge username from structural analysis / HTML classifier with the
results from server hints or autocomplete attributes.

The CL also renames ParseResult to SignificantFields, because both
"parse" and "result" are somewhat overloaded in this file. The CL
further does a few similar minor changes to naming, comments and
code structure, in an attempt to increase readability

Bug:  845426 
Change-Id: Ie5a6297a2f7eb1a0d89421ea91a93bdd5a7112b5
Reviewed-on: https://chromium-review.googlesource.com/1117183
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571937}
[modify] https://crrev.com/b0a6421113c7d8f30aea9eabef72cbf1a9ecf494/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/b0a6421113c7d8f30aea9eabef72cbf1a9ecf494/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Blockedon: -854123
Blockedon: -854147
Blockedon: -855987
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 25

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

commit 061a338c852c9666e6d18b2c2d765e77da8c0df9
Author: Vaclav Brozek <vabr@chromium.org>
Date: Wed Jul 25 11:40:02 2018

Add reporting UsernameDetectionMethod from new parser

The old FormData -> PasswordForm parser reported to the
PasswordManager.UsernameDetectionMethod histogram, which method was used
to detect the username in the form.

This CL teaches the new parser to do the same.

The supporting enum is copied from the old one and modernised. There is
not point in attempting to share that code given that the old parser is
going away soon.

Bug:  845426 
Change-Id: I7737cb8dc598c11b42c0e7e0f929afed5e830d7c
Reviewed-on: https://chromium-review.googlesource.com/1149866
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577841}
[modify] https://crrev.com/061a338c852c9666e6d18b2c2d765e77da8c0df9/components/password_manager/core/browser/form_parsing/BUILD.gn
[modify] https://crrev.com/061a338c852c9666e6d18b2c2d765e77da8c0df9/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/061a338c852c9666e6d18b2c2d765e77da8c0df9/components/password_manager/core/browser/form_parsing/form_parser.h
[modify] https://crrev.com/061a338c852c9666e6d18b2c2d765e77da8c0df9/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 26

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

commit 06ea4ca77289790a77db9b87b65d4cd6bd6b3d12
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Jul 26 20:45:52 2018

Remove FormFieldData::is_default

...and also the related *password_value_is_default fields in
PasswordForm. These are only set and never read.

Bug:  845426 
Change-Id: I87e686d2cfb588cb94a50e8a6395a7b19b7b82d5
Reviewed-on: https://chromium-review.googlesource.com/1149878
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578432}
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/common/autofill_types.mojom
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/common/autofill_types_struct_traits.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/common/autofill_types_struct_traits.h
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/common/autofill_types_struct_traits_unittest.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/renderer/form_autofill_util.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/renderer/form_autofill_util_browsertest.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/core/common/form_field_data.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/core/common/form_field_data.h
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/core/common/password_form.cc
[modify] https://crrev.com/06ea4ca77289790a77db9b87b65d4cd6bd6b3d12/components/autofill/core/common/password_form.h

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 29

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

commit 012429431bff6114dc070718ce7872d683336666
Author: Vaclav Brozek <vabr@chromium.org>
Date: Sat Sep 29 09:47:31 2018

Add UKM for comparing password form parsing for saving

When the user submits a form and Chrome understands that as a
successful submission, it parses the submitted data into a
PasswordForm ready to be saved into the PasswordStore.

Currently, two codepaths exists to do so. The old one involves a
PasswordFormManager and an old FormData parser, the new one involves a
NewPasswordFormManager and a new FormData parser.

This CL adds a metric comparing the result of both codepaths.

The privacy review of password manager UKM metrics was done on
https://crbug.com/728707.

Bug:  845426 
Change-Id: Ie8c11d4ff6e0a17029ec05104a560fe764597f1e
Reviewed-on: https://chromium-review.googlesource.com/1243126
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595312}
[modify] https://crrev.com/012429431bff6114dc070718ce7872d683336666/components/password_manager/core/browser/password_form_metrics_recorder.cc
[modify] https://crrev.com/012429431bff6114dc070718ce7872d683336666/components/password_manager/core/browser/password_form_metrics_recorder.h
[modify] https://crrev.com/012429431bff6114dc070718ce7872d683336666/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/012429431bff6114dc070718ce7872d683336666/components/password_manager/core/browser/password_manager_unittest.cc
[modify] https://crrev.com/012429431bff6114dc070718ce7872d683336666/tools/metrics/ukm/ukm.xml

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 1

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

commit dcd6161a3eb50da48140842959a061f657237d05
Author: Vaclav Brozek <vabr@chromium.org>
Date: Mon Oct 01 19:02:27 2018

Remove conditional build dep for form_parsing

In components/password_manager/core/browser/BUILD.gn:unit_test,
the .../form_parsing:unit_tests dependency is added twice:
once unconditionally and once for iOS.
The latter is older but now obviously unneeded.
So this CL removes the latter.

Bug:  845426 

Change-Id: Iba83449d74cf041369dbced5506d1055f7686e7a
Reviewed-on: https://chromium-review.googlesource.com/1252482
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595492}
[modify] https://crrev.com/dcd6161a3eb50da48140842959a061f657237d05/components/password_manager/core/browser/BUILD.gn

Status: Fixed (was: Started)
The parser has been there for some time now.
Particular issues are tracked in separate bugs.

Sign in to add a comment