[Password Manager] Unify action and origin detection

[Password Manager] Unify action and origin detection

Currently it's possible for the action in a PasswordForm to not match the
action for the same WebFormElement in PasswordAutofillAgent because the action
is computed differently. This can cause us to not fill a password on forms
where <base href=> is set and the action on the form is empty.

BUG=360230
Committed: https://crrev.com/37ca817dee72534c2835f69d8f905aa27ee23e22
Cr-Commit-Position: refs/heads/master@{#323592}

[Garrett Casto, 2015-03-27 22:36:42]

gcasto@chromium.org changed reviewers:
+ engedy@chromium.org

[Garrett Casto, 2015-03-27 22:36:43]

Hey Balazs,

In looking into this I wasn't actually able to reproduce
https://crbug.com/469790. The path was always unless <base> was set. Can you
reproduce this issue without <base>?

[Garrett Casto, 2015-04-01 17:47:41]

Ping?

[engedy, 2015-04-01 18:00:57]

On 2015/04/01 17:47:41, Garrett Casto wrote:
> Ping?

Ah, sorry, this fell off my radar. Taking a look right now.

To reproduce the behavior without <base>, I used
https://rateflicks.appspot.com/login?mode=select. I will send you credentials
off-line.

[engedy, 2015-04-01 19:06:24]

LGTM % minor comments. Thanks!

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:1850: // the
form is empty. Regression test for https://crbug.com/360230.
We should mention the form not having an action attribute here -- having an
empty action worked before just fine.

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:1875:
base::RunLoop run_loop;
nit: Although the current solution is consistent with the rest of the file, you
could just write:

base::RunLoop().RunUntilIdle();

[Garrett Casto, 2015-04-01 23:59:30]

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
File chrome/browser/password_manager/password_manager_browsertest.cc (right):

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:1850: // the
form is empty. Regression test for https://crbug.com/360230.
On 2015/04/01 19:06:24, engedy wrote:
> We should mention the form not having an action attribute here -- having an
> empty action worked before just fine.

Yeah, that wording is confusing. I meant the same thing, but empty is ambiguous.
Changed.

https://codereview.chromium.org/1039833002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:1875:
base::RunLoop run_loop;
On 2015/04/01 19:06:24, engedy wrote:
> nit: Although the current solution is consistent with the rest of the file,
you
> could just write:
> 
> base::RunLoop().RunUntilIdle();

Sounds good.

[Garrett Casto, 2015-04-01 23:59:33]

The CQ bit was checked by gcasto@chromium.org

[Garrett Casto, 2015-04-01 23:59:34]

The patchset sent to the CQ was uploaded after l-g-t-m from engedy@chromium.org
Link to the patchset: https://codereview.chromium.org/1039833002/#ps40001
(title: "Comments")

[commit-bot: I haz the power, 2015-04-02 00:00:40]

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

[commit-bot: I haz the power, 2015-04-02 01:46:08]

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

[commit-bot: I haz the power, 2015-04-02 01:46:09]

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

[Garrett Casto, 2015-04-02 20:15:47]

The CQ bit was checked by gcasto@chromium.org

[Garrett Casto, 2015-04-02 20:15:47]

The patchset sent to the CQ was uploaded after l-g-t-m from engedy@chromium.org
Link to the patchset: https://codereview.chromium.org/1039833002/#ps60001
(title: "Test")

[commit-bot: I haz the power, 2015-04-02 20:17:00]

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

[commit-bot: I haz the power, 2015-04-03 05:01:28]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-03 20:30:31]

Patchset 4 (id:??) landed as
https://crrev.com/37ca817dee72534c2835f69d8f905aa27ee23e22
Cr-Commit-Position: refs/heads/master@{#323592}