Prepare the infrastructure for password overrides.

Prepare the infrastructure for password overrides.

Currently use the autofill field categories "USERNAME" and "USERNAME_AND_EMAIL" in our password form parsing heuristics. Use the same pipeline to pass the predictions of "PASSWORD" and "ACCOUNT_CREATION_PASSWORD" as well.

This is mostly refactoring. The new hints on password fields are not yet used in this CL.

BUG=492604
Committed: https://crrev.com/8593e1698184b2ec3146b90212bad54e38b03217
Cr-Commit-Position: refs/heads/master@{#331968}

[msramek, 2015-05-27 12:55:58]

msramek@chromium.org changed reviewers:
+ vabr@chromium.org

[msramek, 2015-05-27 12:55:59]

Hi Václav,

could you have a look at this boilerplate? :)

Thanks,
Martin

[vabr (Chromium), 2015-05-27 13:31:51]

Hi Martin,

This LGTM.

Cheers,
Vaclav

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
File components/autofill/core/common/password_form_field_predictions.h (right):

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:15:
PREDICTION_USERNAME,
Would it make sense to use ServerFieldType instead?

[msramek, 2015-05-27 14:38:35]

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
File components/autofill/core/common/password_form_field_predictions.h (right):

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:15:
PREDICTION_USERNAME,
On 2015/05/27 13:31:51, vabr (Chromium) wrote:
> Would it make sense to use ServerFieldType instead?

I wanted to give GetPasswordForm() a more explicit input, but it could work
directly with ServerFieldType too. I don't have such a strong opinion on this.

However, it cannot be done directly, as ServerFieldType is in browser/, and this
needs to stay in common/ (these messages are sent from renderer to browser).

[msramek, 2015-05-27 14:40:09]

msramek@chromium.org changed reviewers:
+ isherman@chromium.org, mkwst@chromium.org

[msramek, 2015-05-27 14:40:10]

+Ilya, Mike

PTAL as well.

Thanks,
Martin

[Ilya Sherman, 2015-05-27 18:21:50]

LGTM.  I only looked at the non-password files, plus the new file.  Let me know
if you wanted me to look at anything else as well.

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
File components/autofill/core/common/password_form_field_predictions.h (right):

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:22:
std::map<PasswordFormFieldPredictionType, FormFieldData>;
nit: I'd prefer to avoid this typedef/using stmt entirely.  However, if you do
keep it, please name the type "PasswordFormFieldPredictionMap".  "PredictionMap"
indicates a map, whereas a simply plural "Predictions", by convention, indicates
a vector or other type of list.

[vabr (Chromium), 2015-05-28 07:53:32]

(Still LGTM.)
Cheers,
Vaclav

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
File components/autofill/core/common/password_form_field_predictions.h (right):

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:15:
PREDICTION_USERNAME,
On 2015/05/27 14:38:35, msramek wrote:
> On 2015/05/27 13:31:51, vabr (Chromium) wrote:
> > Would it make sense to use ServerFieldType instead?
> 
> I wanted to give GetPasswordForm() a more explicit input, but it could work
> directly with ServerFieldType too. I don't have such a strong opinion on this.
> 
> However, it cannot be done directly, as ServerFieldType is in browser/, and
this
> needs to stay in common/ (these messages are sent from renderer to browser).

Acknowledged, can stay as it is. Would you consider putting in a code comment
explaining that this does not use ServerFieldType, because that type is
currently in browser/ instead of common/?

[Mike West, 2015-05-28 11:57:18]

IPC LGTM.

[msramek, 2015-05-28 19:36:48]

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
File components/autofill/core/common/password_form_field_predictions.h (right):

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:15:
PREDICTION_USERNAME,
On 2015/05/28 07:53:32, vabr (Chromium) wrote:
> On 2015/05/27 14:38:35, msramek wrote:
> > On 2015/05/27 13:31:51, vabr (Chromium) wrote:
> > > Would it make sense to use ServerFieldType instead?
> > 
> > I wanted to give GetPasswordForm() a more explicit input, but it could work
> > directly with ServerFieldType too. I don't have such a strong opinion on
this.
> > 
> > However, it cannot be done directly, as ServerFieldType is in browser/, and
> this
> > needs to stay in common/ (these messages are sent from renderer to browser).
> 
> Acknowledged, can stay as it is. Would you consider putting in a code comment
> explaining that this does not use ServerFieldType, because that type is
> currently in browser/ instead of common/?

Done.

https://codereview.chromium.org/1153023004/diff/1/components/autofill/core/co...
components/autofill/core/common/password_form_field_predictions.h:22:
std::map<PasswordFormFieldPredictionType, FormFieldData>;
On 2015/05/27 18:21:50, Ilya Sherman wrote:
> nit: I'd prefer to avoid this typedef/using stmt entirely.  However, if you do
> keep it, please name the type "PasswordFormFieldPredictionMap". 
"PredictionMap"
> indicates a map, whereas a simply plural "Predictions", by convention,
indicates
> a vector or other type of list.

I'd prefer not to add a file for so little content either, but currently this
seems like the most reasonable approach. I'll keep it in mind; eventually, we
might want to give even more autofill information to password manager and then
we'll definitely want to reuse ServerFieldType.

Renamed to Map.

[msramek, 2015-05-28 19:36:54]

The CQ bit was checked by msramek@chromium.org

[msramek, 2015-05-28 19:36:54]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org,
mkwst@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1153023004/#ps20001
(title: "Renamed to Map, added comment.")

[commit-bot: I haz the power, 2015-05-28 19:37:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1153023004/20001

[commit-bot: I haz the power, 2015-05-28 19:53:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-28 19:53:10]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vabr (Chromium), 2015-05-28 19:58:01]

lgtm

[msramek, 2015-05-29 12:31:19]

I forgot about the test here, which didn't compile after the message name was
changed.

I changed the wording to represent the fact that we can test both the username
and password predictions together; however, not right now, as we don't apply the
password predictions yet. (This is blocked on some discussions about what
exactly should we return from Autofill.)

[vabr (Chromium), 2015-05-29 12:33:23]

Still LGTM, thanks for the explanation.
Cheers,
Vaclav

[msramek, 2015-05-29 12:39:45]

The CQ bit was checked by msramek@chromium.org

[msramek, 2015-05-29 12:39:46]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1153023004/#ps40001
(title: "Fixed the test.")

[commit-bot: I haz the power, 2015-05-29 12:39:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1153023004/40001

[commit-bot: I haz the power, 2015-05-29 14:56:46]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-29 14:57:38]

Patchset 3 (id:??) landed as
https://crrev.com/8593e1698184b2ec3146b90212bad54e38b03217
Cr-Commit-Position: refs/heads/master@{#331968}