Credential Manager: Respect the 'skip_zero_click' flag for 'request()'.

Credential Manager: Respect the 'skip_zero_click' flag for 'request()'.

Now that we're setting the 'skip_zero_click' flag in 'notifySignedOut',
we ought to respect it when generating the list of items for 'request()'

BUG=450581
R=vabr@chromium.org, vasilii@chromium.org
NOTRY=true

Committed: https://crrev.com/8b92b6ea59cebc775ac2fc455db019ce469306aa
Cr-Commit-Position: refs/heads/master@{#315287}

[Mike West, 2015-02-09 10:16:45]

Would one of you two mind taking a look?

Note that this depends on the rename in
https://codereview.chromium.org/913433002, so I can't throw it at the bots quite
yet.

[vabr (Chromium), 2015-02-09 10:31:13]

This LGTM.

Cheers,
Vaclav

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
File
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
(right):

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:429:
EXPECT_EQ(CredentialType::CREDENTIAL_TYPE_LOCAL, get<1>(send_param).type);
Should you also check, e.g., the .name field to verify that this is indeed form_
being sent back?

[vasilii, 2015-02-09 10:44:07]

Do you have a test with two skip_zero_click==false credentials?

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
File
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
(right):

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:428:
// We should get |form_| back, as |form2_| is marked as skipping zero-click.
Actually |form_| is marked as skipping zero-click.

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:429:
EXPECT_EQ(CredentialType::CREDENTIAL_TYPE_LOCAL, get<1>(send_param).type);
On 2015/02/09 10:31:13, vabr (Chromium) wrote:
> Should you also check, e.g., the .name field to verify that this is indeed
form_
> being sent back?

Acknowledged.

[Mike West, 2015-02-09 10:46:01]

On 2015/02/09 at 10:44:07, vasilii wrote:
> Do you have a test with two skip_zero_click==false credentials?

Yes: the preexisting
`CredentialManagerDispatcherTest.CredentialManagerOnRequestCredentialWithZeroClickOnlyTwoPasswordStore`.

[Mike West, 2015-02-09 10:50:27]

New patchsets have been uploaded after l-g-t-m from vabr@chromium.org

[Mike West, 2015-02-09 10:51:50]

Thanks, vabr@! Vasilii, are you happy with patchset #2?

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
File
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
(right):

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:428:
// We should get |form_| back, as |form2_| is marked as skipping zero-click.
On 2015/02/09 at 10:44:07, vasilii wrote:
> Actually |form_| is marked as skipping zero-click.

Good eye, thanks!

https://codereview.chromium.org/896803003/diff/1/components/password_manager/...
components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc:429:
EXPECT_EQ(CredentialType::CREDENTIAL_TYPE_LOCAL, get<1>(send_param).type);
On 2015/02/09 at 10:44:07, vasilii wrote:
> On 2015/02/09 10:31:13, vabr (Chromium) wrote:
> > Should you also check, e.g., the .name field to verify that this is indeed
form_
> > being sent back?
> 
> Acknowledged.

Done.

[vasilii, 2015-02-09 10:54:41]

lgtm

[Mike West, 2015-02-09 11:34:20]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-02-09 11:35:12]

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

[Mike West, 2015-02-09 13:31:23]

On 2015/02/09 at 11:35:12, commit-bot wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/896803003/20001

NOTRYing, as the iOS bots are dead (see https://crbug.com/456753), and all the
other bots are happy.

[Mike West, 2015-02-09 13:31:28]

The CQ bit was unchecked by mkwst@chromium.org

[Mike West, 2015-02-09 13:31:31]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2015-02-09 13:32:21]

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

[commit-bot: I haz the power, 2015-02-09 13:33:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-09 13:34:07]

Patchset 2 (id:??) landed as
https://crrev.com/8b92b6ea59cebc775ac2fc455db019ce469306aa
Cr-Commit-Position: refs/heads/master@{#315287}