Ignore form action URL when determine if a credential should be autofilled.

Ignore form action URL when determine if a credential should be autofilled.

The Credential Manager API saves passwords without action like Android does. The former credentials should be treated as the latter ones.
As a side effect the CL fixes an apparently buggy condition for the change password forms coming from r335019. Before: Android credentials were autofilled into the change password forms. Now: the are offered in the drop-down like normal web credentials.

BUG=725495
Review-Url: https://codereview.chromium.org/2900983002
Cr-Commit-Position: refs/heads/master@{#474284}
Committed: https://chromium.googlesource.com/chromium/src/+/40f676517d2d8b840c41eb67a7f874a154e7f54a

[vasilii, 2017-05-23 14:26:24]

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

[commit-bot: I haz the power, 2017-05-23 14:26:42]

Dry run: CQ is trying da patch.

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

[vasilii, 2017-05-23 14:29:16]

vasilii@chromium.org changed reviewers:
+ dvadym@chromium.org

[vasilii, 2017-05-23 14:29:17]

Hi Vadym,

please review. The strange condition about change password forms is originally
from https://codereview.chromium.org/1161023008

[dvadym, 2017-05-23 14:59:52]

https://codereview.chromium.org/2900983002/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/password_manager_test_base.cc (right):

https://codereview.chromium.org/2900983002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/password_manager_test_base.cc:68: void
OnPasswordSubmitted(
Why do we need this, it looks that this method is not used in tests now?

https://codereview.chromium.org/2900983002/diff/1/components/password_manager...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2900983002/diff/1/components/password_manager...
components/password_manager/core/browser/password_form_manager.cc:627: //
However, (2) should not apply to Android-based credentials found
The comment mentions Android credentials, but the condition for
|wait_for_username| doesn't contain anything about it.

[commit-bot: I haz the power, 2017-05-23 15:03:00]

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

[commit-bot: I haz the power, 2017-05-23 15:03:01]

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

[vasilii, 2017-05-24 12:48:28]

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

[vasilii, 2017-05-24 12:48:37]

https://codereview.chromium.org/2900983002/diff/1/chrome/browser/password_man...
File chrome/browser/password_manager/password_manager_test_base.cc (right):

https://codereview.chromium.org/2900983002/diff/1/chrome/browser/password_man...
chrome/browser/password_manager/password_manager_test_base.cc:68: void
OnPasswordSubmitted(
On 2017/05/23 14:59:52, dvadym wrote:
> Why do we need this, it looks that this method is not used in tests now?

It's how WaitForSavePrompt() is implemented. This method is called when the
bubble is to be popped up. Thus, it's time to quit the RunLoop.

https://codereview.chromium.org/2900983002/diff/1/components/password_manager...
File components/password_manager/core/browser/password_form_manager.cc (right):

https://codereview.chromium.org/2900983002/diff/1/components/password_manager...
components/password_manager/core/browser/password_form_manager.cc:627: //
However, (2) should not apply to Android-based credentials found
On 2017/05/23 14:59:52, dvadym wrote:
> The comment mentions Android credentials, but the condition for
> |wait_for_username| doesn't contain anything about it.

Done.

[commit-bot: I haz the power, 2017-05-24 12:48:45]

Dry run: CQ is trying da patch.

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

[dvadym, 2017-05-24 13:19:59]

LGTM

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

https://codereview.chromium.org/2900983002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:2380:
scoped_feature_list.InitAndEnableFeature(features::kFillOnAccountSelect);
Cool solution for preventing filling.

[commit-bot: I haz the power, 2017-05-24 13:41:38]

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

[commit-bot: I haz the power, 2017-05-24 13:41:39]

Dry run: This issue passed the CQ dry run.

[vasilii, 2017-05-24 14:31:50]

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

https://codereview.chromium.org/2900983002/diff/20001/chrome/browser/password...
chrome/browser/password_manager/password_manager_browsertest.cc:2380:
scoped_feature_list.InitAndEnableFeature(features::kFillOnAccountSelect);
On 2017/05/24 13:19:59, dvadym wrote:
> Cool solution for preventing filling.

I'm not proud of myself because strictly speaking it's not a production code. On
the other hand, the test is as readable and useful as before :)

[vasilii, 2017-05-24 14:31:55]

The CQ bit was checked by vasilii@chromium.org

[commit-bot: I haz the power, 2017-05-24 14:32:21]

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, 2017-05-24 14:37:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495636315152860,
"parent_rev": "5c368544217d411cc810f1afe74cc86dd075b423", "commit_rev":
"40f676517d2d8b840c41eb67a7f874a154e7f54a"}

[commit-bot: I haz the power, 2017-05-24 14:37:36]

Description was changed from

==========
Ignore form action URL when determine if a credential should be autofilled.

The Credential Manager API saves passwords without action like Android does. The
former credentials should be treated as the latter ones.
As a side effect the CL fixes an apparently buggy condition for the change
password forms coming from r335019. Before: Android credentials were autofilled
into the change password forms. Now: the are offered in the drop-down like
normal web credentials.

BUG=725495
==========

to

==========
Ignore form action URL when determine if a credential should be autofilled.

The Credential Manager API saves passwords without action like Android does. The
former credentials should be treated as the latter ones.
As a side effect the CL fixes an apparently buggy condition for the change
password forms coming from r335019. Before: Android credentials were autofilled
into the change password forms. Now: the are offered in the drop-down like
normal web credentials.

BUG=725495

Review-Url: https://codereview.chromium.org/2900983002
Cr-Commit-Position: refs/heads/master@{#474284}
Committed:
https://chromium.googlesource.com/chromium/src/+/40f676517d2d8b840c41eb67a7f8...
==========

[commit-bot: I haz the power, 2017-05-24 14:37:38]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/40f676517d2d8b840c41eb67a7f8...