[Password Generation] Use canonical version of action in for form matching in PasswordGenerationAgen

[Password Generation] Use canonical version of action in for form matching in PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show generation popup because form matching failed (e.g. if one action is canonical, another isn't).

This fix is temporary solution. This CL (https://codereview.chromium.org/2318533002) will use signature based matching.

BUG=582434
Committed: https://crrev.com/3dc6a6b24366b58e4b5f6c29d278c07c902e2b5f
Cr-Commit-Position: refs/heads/master@{#416932}

[kolos1, 2016-09-07 10:46:49]

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

[kolos1, 2016-09-07 12:42:04]

Description was changed from

==========
[Password Generation] Use canonical version of action in autofill::FormData

Even if server find suitable form for generation, Chrome will not show
generation popop because form macheing failed (e.g. if one action is canonical,
another isn't).

BUG=582434
==========

to

==========
[Password Generation] Use canonical version of action in for form matching in
PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show
generation popup because form matching failed (e.g. if one action is canonical,
another isn't).

This fix is temporary solution. This CL
(https://codereview.chromium.org/2318533002) will use signature based matching. 

BUG=582434
==========

[kolos1, 2016-09-07 12:45:06]

Patchset #2 (id:20001) has been deleted

[kolos1, 2016-09-07 12:46:36]

Hi Vaclav,

Please review new version that we discussed in person. Thank you.

Regards,
Maxim

[vabr (Chromium), 2016-09-07 13:10:08]

LGTM, just nits.

Thank you, Maxim.
Vaclav

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
components/autofill/content/renderer/form_autofill_util.cc:1191: // We want to
keep the path but strip any authentication data, as well as
nit: The old comment is vague and out of date (usage of "we", reference to a
form while the argument is just a URL). Please drop it completely, the code is
clear about what it does, and you added a better comment at the declaration.

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
File components/autofill/content/renderer/password_generation_agent.cc (right):

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
components/autofill/content/renderer/password_generation_agent.cc:34: using
autofill::form_util::StripAuthAndParams;
nit: Move this below "namespace autofill" and drop the "autofill::".

[kolos1, 2016-09-07 13:19:00]

Thanks for quick response. 

I made the changes addressed to your comments.

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
components/autofill/content/renderer/form_autofill_util.cc:1191: // We want to
keep the path but strip any authentication data, as well as
On 2016/09/07 13:10:08, vabr (Chromium) wrote:
> nit: The old comment is vague and out of date (usage of "we", reference to a
> form while the argument is just a URL). Please drop it completely, the code is
> clear about what it does, and you added a better comment at the declaration.

Done.

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
File components/autofill/content/renderer/password_generation_agent.cc (right):

https://codereview.chromium.org/2320563002/diff/40001/components/autofill/con...
components/autofill/content/renderer/password_generation_agent.cc:34: using
autofill::form_util::StripAuthAndParams;
On 2016/09/07 13:10:08, vabr (Chromium) wrote:
> nit: Move this below "namespace autofill" and drop the "autofill::".

Done.

[vabr (Chromium), 2016-09-07 13:32:28]

lgtm

[kolos1, 2016-09-07 13:36:55]

The CQ bit was checked by kolos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 13:37:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kolos1, 2016-09-07 13:38:43]

The CQ bit was unchecked by kolos@chromium.org

[kolos1, 2016-09-07 14:09:08]

The CQ bit was checked by kolos@chromium.org

[commit-bot: I haz the power, 2016-09-07 14:09:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 14:19:21]

Description was changed from

==========
[Password Generation] Use canonical version of action in for form matching in
PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show
generation popup because form matching failed (e.g. if one action is canonical,
another isn't).

This fix is temporary solution. This CL
(https://codereview.chromium.org/2318533002) will use signature based matching. 

BUG=582434
==========

to

==========
[Password Generation] Use canonical version of action in for form matching in
PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show
generation popup because form matching failed (e.g. if one action is canonical,
another isn't).

This fix is temporary solution. This CL
(https://codereview.chromium.org/2318533002) will use signature based matching. 

BUG=582434
==========

[commit-bot: I haz the power, 2016-09-07 14:19:23]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-07 14:21:09]

Description was changed from

==========
[Password Generation] Use canonical version of action in for form matching in
PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show
generation popup because form matching failed (e.g. if one action is canonical,
another isn't).

This fix is temporary solution. This CL
(https://codereview.chromium.org/2318533002) will use signature based matching. 

BUG=582434
==========

to

==========
[Password Generation] Use canonical version of action in for form matching in
PasswordGenerationAgent

Even if the server found a suitable form for generation, Chrome will not show
generation popup because form matching failed (e.g. if one action is canonical,
another isn't).

This fix is temporary solution. This CL
(https://codereview.chromium.org/2318533002) will use signature based matching.

BUG=582434

Committed: https://crrev.com/3dc6a6b24366b58e4b5f6c29d278c07c902e2b5f
Cr-Commit-Position: refs/heads/master@{#416932}
==========

[commit-bot: I haz the power, 2016-09-07 14:21:10]

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