[Password Autofill] Catch XHR submitted forms where onsubmit() wasn't called.

[Password Autofill] Catch XHR submitted forms where onsubmit() isn't called.

This fixes a bug where the password manager would fail to trigger on the registration form for Facebook.com. The password manager will still fail for synchonous XHR, but
this should be less prevalent.

BUG=43219
R=vabr@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246421

[Garrett Casto, 2014-01-15 21:33:54]

I'm a little worried about this being slow on these types of commits, but we'll
see if the perf bots pick up on this. They should be relatively rare though.

[vabr (Chromium), 2014-01-16 09:22:09]

LGTM, but please make sure that NoPromptIfLinkClicked tests what you want (see
my comment).

Regarding performance, I'm OK with landing this and seeing how it turns out.

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
chrome/browser/password_manager/password_manager_browsertest.cc:307: // Verify
that if XHR navigation and the form is properly filled out, we
nit: The sentence "if XHR navigation" seems to lack the verb.

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
chrome/browser/password_manager/password_manager_browsertest.cc:328: }
Did you forget to check that the prompt does not show?

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
components/autofill/content/renderer/password_autofill_agent.cc:504: for (size_t
i = 0; i < forms.size(); ++i) {
If your performance concerns were about this for cycle, then I share them a
little. :)

I agree that we can start with just looking at how it performs.
If needed, we can try to shorten the time to give up on forms which are not
password forms, e.g., by reordering WebPasswordFormData c-tor to check password
fields earlier.

[Garrett Casto, 2014-01-16 22:23:59]

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
chrome/browser/password_manager/password_manager_browsertest.cc:307: // Verify
that if XHR navigation and the form is properly filled out, we
On 2014/01/16 09:22:10, vabr (Chromium) wrote:
> nit: The sentence "if XHR navigation" seems to lack the verb.

Done.

https://codereview.chromium.org/137893009/diff/1/chrome/browser/password_mana...
chrome/browser/password_manager/password_manager_browsertest.cc:328: }
On 2014/01/16 09:22:10, vabr (Chromium) wrote:
> Did you forget to check that the prompt does not show?

Whoops, fixed.

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
components/autofill/content/renderer/password_autofill_agent.cc:504: for (size_t
i = 0; i < forms.size(); ++i) {
On 2014/01/16 09:22:10, vabr (Chromium) wrote:
> If your performance concerns were about this for cycle, then I share them a
> little. :)
> 
> I agree that we can start with just looking at how it performs.
> If needed, we can try to shorten the time to give up on forms which are not
> password forms, e.g., by reordering WebPasswordFormData c-tor to check
password
> fields earlier.

Yeah. From a performance standpoint the most optimal thing to do would be to
save a reference to the form when it's being filled out so that we don't have to
loop at all, but that's reasonably complicated.

[vabr (Chromium), 2014-01-17 09:28:01]

Still LGTM. Thanks!

Vacalv

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
File components/autofill/content/renderer/password_autofill_agent.cc (right):

https://codereview.chromium.org/137893009/diff/1/components/autofill/content/...
components/autofill/content/renderer/password_autofill_agent.cc:504: for (size_t
i = 0; i < forms.size(); ++i) {
On 2014/01/16 22:23:59, Garrett Casto wrote:
> On 2014/01/16 09:22:10, vabr (Chromium) wrote:
> > If your performance concerns were about this for cycle, then I share them a
> > little. :)
> > 
> > I agree that we can start with just looking at how it performs.
> > If needed, we can try to shorten the time to give up on forms which are not
> > password forms, e.g., by reordering WebPasswordFormData c-tor to check
> password
> > fields earlier.
> 
> Yeah. From a performance standpoint the most optimal thing to do would be to
> save a reference to the form when it's being filled out so that we don't have
to
> loop at all, but that's reasonably complicated.

Sure, let's wait if we need to optimise this at all.

[commit-bot: I haz the power, 2014-01-17 20:39:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gcasto@chromium.org/137893009/60001

[commit-bot: I haz the power, 2014-01-17 22:27:14]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-22 19:34:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gcasto@chromium.org/137893009/60001

[commit-bot: I haz the power, 2014-01-22 21:16:08]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Garrett Casto, 2014-01-22 21:45:42]

Committed patchset #3 manually as r246421 (presubmit successful).