Credential Manager: Stub out call to PasswordStore.

Credential Manager: Stub out call to PasswordStore.

This CL changes the ContentCredentialManagerDispatcher implementation of
requesting credentials to pass the request through the PasswordStore. We
don't yet use the result; we just verify that the callback is called and
passes data around correctly. Baby steps.

BUG=400674
Committed: https://crrev.com/e8b7bb544f4836862a2ab702beb76bd72bfce40e
Cr-Commit-Position: refs/heads/master@{#297404}

[Mike West, 2014-09-30 09:15:56]

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

[Mike West, 2014-09-30 09:15:57]

WDYT, vabr@? I'm trying to keep the patches small; you'll see a few steps. If
you'd prefer bigger chunks, just let me know.

-mike

[vabr (Chromium), 2014-09-30 09:38:32]

LGTM with comments.

Small chunks (as you do know) are preferable, please keep doing that. Also, feel
free to ping me physically (e.g., with a club, not too hard to damage my
reviewing brain) to make me aware of incoming reviews in case I fail to notice
them early. :)

Cheers,
Vaclav

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

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:24:
client_(client) {
pending_request_id_(0)

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:73:
int request_id,
Should you DCHECK(request_id)?

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:79:
// if we're already requesting credentials.
Did you want to add a return here?
(Maybe to avoid accessing a NULL store below, but up to you.)

[Mike West, 2014-09-30 09:43:28]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-09-30 09:44:07]

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

[Mike West, 2014-09-30 09:44:49]

Thanks! And none of this is a rush; I'm happy for you to look at patches as you
find time. I know I like to batch up reviews so that I have time to concentrate
on work, so I'm certainly not going to interrupt your work to make you look at
mine. :)

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

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:24:
client_(client) {
On 2014/09/30 09:38:31, vabr (Chromium) wrote:
> pending_request_id_(0)

Done.

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:73:
int request_id,
On 2014/09/30 09:38:31, vabr (Chromium) wrote:
> Should you DCHECK(request_id)?

Hrm. That's actually a good question. I don't know whether this is 0-based or
1-based. Let me check that... :)

It's 1-based! Yay!

https://codereview.chromium.org/615863002/diff/1/components/password_manager/...
components/password_manager/content/browser/content_credential_manager_dispatcher.cc:79:
// if we're already requesting credentials.
On 2014/09/30 09:38:32, vabr (Chromium) wrote:
> Did you want to add a return here?
> (Maybe to avoid accessing a NULL store below, but up to you.)

I should have done it here, but I'm now doing it in the next patch. So I'll just
leave this one as-is.

[vabr (Chromium), 2014-09-30 09:50:08]

Still LGTM,
Thanks!

[commit-bot: I haz the power, 2014-09-30 11:24:43]

Committed patchset #2 (id:20001) as 834c9df852f39ad8ff1a456001956fe29160e5a7

[commit-bot: I haz the power, 2014-09-30 11:25:31]

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