Credential Manager: Stub out the browser-side IPC handlers.

Credential Manager: Stub out the browser-side IPC handlers.

BUG=400674
TBR=palmer@chromium.org

Committed: https://crrev.com/7b4bf4d19b1412460c3512f1632a3cc86eb0fd8c
Cr-Commit-Position: refs/heads/master@{#292852}

[Mike West, 2014-08-22 13:38:13]

Hi Vaclav! Would you mind taking a look at the simplest stub implementation I
could come up with? I just want to get to a point where I can land the WebKit
side change to actually wire the JavaScript up to the browser. This should be
enough for that.

-mike

[vabr (Chromium), 2014-08-22 14:04:32]

LGTM with nits and optional comments.

https://codereview.chromium.org/496703002/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/chrome_password_manager_client.cc (right):

https://codereview.chromium.org/496703002/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client.cc:311:
std::vector<GURL> federations) {
optional: My gut feeling would be that making this a const ref would spare
copying. But I did not check the IPC macros thoroughly for that.

https://codereview.chromium.org/496703002/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
(right):

https://codereview.chromium.org/496703002/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client_unittest.cc:5:
#include "base/command_line.h"
nit: Since you touch this anyway, please move the base/command_line.h #include
to the #include block below.

https://codereview.chromium.org/496703002/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client_unittest.cc:286:
EXPECT_TRUE(!!message);
nit: I'm surprised you need to "!!" this -- pointers used to work as bools for
EXPECT_TRUE. Did some compiler scream at you?

https://codereview.chromium.org/496703002/diff/20001/components/password_mana...
File components/password_manager/core/browser/password_manager_client.h (right):

https://codereview.chromium.org/496703002/diff/20001/components/password_mana...
components/password_manager/core/browser/password_manager_client.h:120: virtual
void OnNotifyFailedSignIn(int request_id, const CredentialInfo&) = 0;
optional: I would suggest stubbing these right here in the interface.
Everything pure virtual added here needs an immediate action (implementation) on
the iOS part of the code. That is useful if the implementation actually needs to
be done on iOS, which IIUC is not the case here right now.

You might want to confirm with blundell@, except he comes back from holiday at
the moment you leave for it. :)

[Mike West, 2014-09-01 07:37:49]

Incorporated your feedback, and dropped an include from the messages file that
made the GN builder sad. Let's see what the bots think.

[Mike West, 2014-09-01 07:38:08]

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

[Mike West, 2014-09-01 08:15:34]

palmer@: TBRing you on the IPC change; it's strictly negative in scope (removing
an enum), so I hope that's a reasonable thing to do.

[Mike West, 2014-09-01 08:15:37]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-09-01 08:15:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mkwst@chromium.org/496703002/80001

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

Patch set 5 still LGTM.
Vaclav

[commit-bot: I haz the power, 2014-09-01 08:55:27]

Committed patchset #5 (id:80001) as c2cc8e9dbe0122525f9f63455d9950ddf05fc54f

[palmer, 2014-09-02 21:01:38]

LGTM post facto FWIW

[commit-bot: I haz the power, 2014-09-10 03:16:11]

Patchset 5 (id:??) landed as
https://crrev.com/7b4bf4d19b1412460c3512f1632a3cc86eb0fd8c
Cr-Commit-Position: refs/heads/master@{#292852}