Credential Manager: Renderer-side implementation.

components/password_manager/content/renderer/credential_manager_dispatcher.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/password_manager/content/renderer/credential_manager_dispatcher.h"
+
+#include "components/password_manager/content/common/credential_manager_messages.h"
+#include "components/password_manager/content/common/credential_manager_types.h"
+#include "content/public/renderer/render_thread.h"
+#include "third_party/WebKit/public/platform/WebCredentialManagerError.h"
+
+namespace password_manager {
+
+namespace {
+
+template <typename T>
+void ClearCallbacksMapWithErrors(T* callbacks_map) {
+  typename T::iterator iter(callbacks_map);
+  while (!iter.IsAtEnd()) {
+    blink::WebCredentialManagerError reason(
+        blink::WebCredentialManagerError::ErrorTypeUnknown,
+        "An unknown error occurred.");

[Ilya Sherman, 2014/08/19 23:55:52]

Is there a plan to add more error types?  If not, this parameter seems pretty
redundant.

[Mike West, 2014/08/20 13:17:19]

I put it in for flexibility as the API develops; it's still pretty early.
*shrug* If we never end up using it, then I agree completely that it's overkill,
and I'll remove it.

+    iter.GetCurrentValue()->onError(&reason);
+    callbacks_map->Remove(iter.GetCurrentKey());
+    iter.Advance();
+  }
+}
+
+// Limit the size of the federations array that we pass to the browser to
+// something reasonably sane.

[palmer, 2014/08/19 18:28:44]

Does the browser enforce this limit upon receipt? If so, it probably can't share
this declaration, which means it'll have to be duplicated, which means there is
a risk of drift...

100 seems high.

[Mike West, 2014/08/20 13:17:19]

Makes sense, moving this to 'content/common/credential_manager_types.h'.
How about 50?

+const size_t kMaxNumFederations = 100;

[palmer, 2014/08/19 18:28:44]

Could also be declared static?

[Ilya Sherman, 2014/08/19 23:55:52]

I'm pretty sure that 'static' would have no effect for this declaration, both
because it's an integer constant declaration and because it's in an anonymous
namespace.

+
+void RespondToNotificationCallback(
+    int request_id,
+    CredentialManagerDispatcher::NotificationCallbacksMap* map) {
+  blink::WebCredentialManager::NotificationCallbacks* callbacks =
+      map->Lookup(request_id);
+  DCHECK(callbacks);

[Ilya Sherman, 2014/08/19 23:55:53]

nit: This seems redundant with the call on the line below.

[Mike West, 2014/08/20 13:17:19]

Sure, but it's a nicer error message if it blows up. *shrug* I can drop it, I
suppose.

+  callbacks->onSuccess();
+  map->Remove(request_id);
+}
+
+}  // namespace
+
+CredentialManagerDispatcher::CredentialManagerDispatcher()
+    : routing_id_(MSG_ROUTING_NONE)
+    , render_thread_(content::RenderThread::Get()) {
+  DCHECK(render_thread_);
+}
+
+CredentialManagerDispatcher::~CredentialManagerDispatcher() {
+  DisconnectFromRenderThread();
+  ClearCallbackErrorMaps();

[Ilya Sherman, 2014/08/19 23:55:52]

nit: Perhaps "ClearCallbackMapsWithErrors"?

[Mike West, 2014/08/20 13:17:19]

Dropping this. Earlier, I needed to override this in the test subclass. Now I
don't need to, so I can just inline these calls.

+}
+
+void CredentialManagerDispatcher::OnRenderProcessShutdown() {
+  DisconnectFromRenderThread();
+}
+
+void CredentialManagerDispatcher::ClearCallbackErrorMaps() {
+  ClearCallbacksMapWithErrors(&failed_sign_in_callbacks_);
+  ClearCallbacksMapWithErrors(&signed_in_callbacks_);
+  ClearCallbacksMapWithErrors(&signed_out_callbacks_);
+  ClearCallbacksMapWithErrors(&request_callbacks_);
+}
+
+int CredentialManagerDispatcher::GetRoutingID() {
+  if (render_thread_ && routing_id_ == MSG_ROUTING_NONE) {
+    routing_id_ = render_thread_->GenerateRoutingID();
+    render_thread_->AddRoute(routing_id_, this);
+  }
+  return routing_id_;
+}
+
+void CredentialManagerDispatcher::DisconnectFromRenderThread() {
+  if (render_thread_ && routing_id_ != MSG_ROUTING_NONE)
+    render_thread_->RemoveRoute(GetRoutingID());

[Ilya Sherman, 2014/08/19 23:55:52]

nit: Why do you call GetRoutingID() here, rather than using routing_id_
directly, even though you use routing_id_ directly on the line above?

[Mike West, 2014/08/20 13:17:19]

Done.

+  render_thread_ = NULL;
+  routing_id_ = MSG_ROUTING_NONE;
+}
+
+// -----------------------------------------------------------------------------
+// Handle messages from the browser.
+
+bool CredentialManagerDispatcher::OnMessageReceived(
+    const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(CredentialManagerDispatcher, message)
+    IPC_MESSAGE_HANDLER(CredentialManagerMsg_AcknowledgeFailedSignIn,
+                        OnFailedSignInResponse)

[Ilya Sherman, 2014/08/19 23:55:52]

nit: Why not have the method names match the IPC names, i.e.
OnAcknowledgeFailedSignIn in this case?

[Mike West, 2014/08/20 13:17:19]

Done.

+    IPC_MESSAGE_HANDLER(CredentialManagerMsg_AcknowledgeSignedIn,
+                        OnSignedInResponse)
+    IPC_MESSAGE_HANDLER(CredentialManagerMsg_AcknowledgeSignedOut,
+                        OnSignedOutResponse)
+    IPC_MESSAGE_HANDLER(CredentialManagerMsg_SendCredentials, OnSendCredentials)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void CredentialManagerDispatcher::OnFailedSignInResponse(int request_id) {
+  RespondToNotificationCallback(request_id, &failed_sign_in_callbacks_);
+}
+
+void CredentialManagerDispatcher::OnSignedInResponse(int request_id) {
+  RespondToNotificationCallback(request_id, &signed_in_callbacks_);
+}
+
+void CredentialManagerDispatcher::OnSignedOutResponse(int request_id) {
+  RespondToNotificationCallback(request_id, &signed_out_callbacks_);
+}
+
+void CredentialManagerDispatcher::OnSendCredentials(
+    int request_id,
+    const CredentialInfo& info) {
+  RequestCallbacks* callbacks = request_callbacks_.Lookup(request_id);
+  DCHECK(callbacks);
+  // TODO(mkwst): Split into local/federated credentials.
+  blink::WebCredential credential(info.id, info.name, info.avatarURL);
+  callbacks->onSuccess(&credential);
+  request_callbacks_.Remove(request_id);
+}
+
+// -----------------------------------------------------------------------------
+// Dispatch messages from the renderer to the browser.
+
+void CredentialManagerDispatcher::dispatchFailedSignIn(
+    const blink::WebCredential& credential,
+    blink::WebCredentialManager::NotificationCallbacks* callbacks) {
+  int request_id = failed_sign_in_callbacks_.Add(callbacks);
+  CredentialInfo info(
+      credential.id(), credential.name(), credential.avatarURL());
+  if (render_thread_)

[Ilya Sherman, 2014/08/19 23:55:52]

nit: Please add curly braces, since the body spans multiple lines of text.

[Mike West, 2014/08/20 13:17:19]

Done.

+    render_thread_->Send(new CredentialManagerHostMsg_NotifyFailedSignIn(
+        GetRoutingID(), request_id, info));
+}
+
+void CredentialManagerDispatcher::dispatchSignedIn(
+    const blink::WebCredential& credential,
+    blink::WebCredentialManager::NotificationCallbacks* callbacks) {
+  int request_id = signed_in_callbacks_.Add(callbacks);
+  CredentialInfo info(
+      credential.id(), credential.name(), credential.avatarURL());
+  if (render_thread_)
+    render_thread_->Send(new CredentialManagerHostMsg_NotifySignedIn(
+        GetRoutingID(), request_id, info));
+}
+
+void CredentialManagerDispatcher::dispatchSignedOut(
+    NotificationCallbacks* callbacks) {
+  int request_id = signed_out_callbacks_.Add(callbacks);
+  if (render_thread_)
+    render_thread_->Send(
+        new CredentialManagerHostMsg_NotifySignedOut(GetRoutingID(),
+                                                     request_id));
+}
+
+void CredentialManagerDispatcher::dispatchRequest(
+    bool zeroClickOnly,
+    const blink::WebVector<blink::WebURL>& federations,
+    RequestCallbacks* callbacks) {
+  int request_id = request_callbacks_.Add(callbacks);
+  std::vector<GURL> federationVector;

[Ilya Sherman, 2014/08/19 23:55:52]

nit: hacker_case

[Mike West, 2014/08/20 13:17:19]

Done.

+  for (size_t i = 0; i < federations.size() && i < kMaxNumFederations; i++)

[Ilya Sherman, 2014/08/19 23:55:52]

Optional nit: I'd use std::min rather than having two comparisons.

[Ilya Sherman, 2014/08/19 23:55:52]

nit: ++i

[Mike West, 2014/08/20 13:17:19]

Done.

[Mike West, 2014/08/20 13:17:19]

An excellent idea!

+    federationVector.push_back(federations[i]);
+  if (render_thread_)
+    render_thread_->Send(new CredentialManagerHostMsg_RequestCredentials(
+        GetRoutingID(), request_id, zeroClickOnly, federationVector));
+}
+
+}  // namespace password_manager