Credential Manager: Renderer-side implementation.

components/password_manager/content/renderer/credential_manager_dispatcher.h

+// 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.
+
+#ifndef COMPONENTS_PASSWORD_MANAGER_CONTENT_RENDERER_CREDENTIAL_MANAGER_DISPATCHER_H_
+#define COMPONENTS_PASSWORD_MANAGER_CONTENT_RENDERER_CREDENTIAL_MANAGER_DISPATCHER_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/id_map.h"
+#include "content/public/renderer/render_process_observer.h"
+#include "ipc/ipc_listener.h"
+#include "ipc/ipc_sender.h"

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

nit: Looks like this is unused.

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

Done.

+#include "third_party/WebKit/public/platform/WebCredential.h"
+#include "third_party/WebKit/public/platform/WebCredentialManager.h"
+#include "third_party/WebKit/public/platform/WebString.h"
+#include "third_party/WebKit/public/platform/WebURL.h"
+#include "third_party/WebKit/public/platform/WebVector.h"

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

Are all of these needed in the header / can any of them be forward-declared?

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

I can drop all except WebVector and WebCredentialManagerClient. Done.

+
+namespace content {
+class RenderThread;
+}
+
+namespace password_manager {
+
+struct CredentialInfo;
+
+// The CredentialManagerDispatcher implements the Blink platform interface
+// WebCredentialManager, and acts as an intermediary between Blink-side calls
+// to 'navigator.credential.*' and the password manager internals which live
+// in the browser process.
+//
+// One instance of CredentialManagerDispatcher is created per RenderThread,
+// held in a scoped_ptr on ChromeContentRendererClient. The dispatcher holds
+// a raw pointer to the RenderThread on which it lives, and uses that pointer
+// to send messages to the browser process, and to route responses to itself.
+//
+// When the render thread is shutdown (or the dispatcher is destructed), the

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

nit: "shutdown" -> "shut down"

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

Done.

+// routing is removed, the pointer is cleared, and any pending responses are
+// rejected.
+class CredentialManagerDispatcher : public blink::WebCredentialManager,
+                                    public content::RenderProcessObserver,
+                                    public IPC::Listener {
+ public:
+  typedef IDMap<blink::WebCredentialManager::RequestCallbacks, IDMapOwnPointer>
+      RequestCallbacksMap;
+  typedef IDMap<blink::WebCredentialManager::NotificationCallbacks,
+                IDMapOwnPointer> NotificationCallbacksMap;

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

nit: Why are these public?  Can they be protected or private instead?

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

They're public so I can use them in an anonymous-namespaced helper function. I
can move that onto the object, I suppose.

+
+  CredentialManagerDispatcher();
+  virtual ~CredentialManagerDispatcher();
+
+  // content::RenderProcessObserver:
+  virtual void OnRenderProcessShutdown() OVERRIDE;
+
+  // IPC::Listener:
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+  // Message handlers for messages from the browser process:
+  virtual void OnFailedSignInResponse(int request_id);
+  virtual void OnSignedInResponse(int request_id);
+  virtual void OnSignedOutResponse(int request_id);
+  virtual void OnSendCredentials(int request_id, const CredentialInfo&);
+
+  // blink::WebCredentialManager:
+  virtual void dispatchFailedSignIn(
+      const blink::WebCredential&,
+      WebCredentialManager::NotificationCallbacks*) OVERRIDE;

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

nit: Please include variable names, throughout.

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

Huh. Ok, that's repetitive, but I guess it's Chromium style. Get me back to
Blink, quickly! :)

+  virtual void dispatchSignedIn(
+      const blink::WebCredential&,
+      WebCredentialManager::NotificationCallbacks*) OVERRIDE;
+  virtual void dispatchSignedOut(NotificationCallbacks*) OVERRIDE;
+  virtual void dispatchRequest(
+      bool zeroClickOnly,

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

nit: hacker_case.

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

Done.

+      const blink::WebVector<blink::WebURL>& federations,
+      RequestCallbacks*) OVERRIDE;
+
+ protected:
+  virtual int GetRoutingID();
+  virtual void ClearCallbackErrorMaps();
+
+  // Track the various blink::WebCredentialManager::*Callbacks objects generated
+  // from Blink. This class takes ownership of these objects. These properties
+  // are protected so we can expose these maps to unit tests.
+  NotificationCallbacksMap failed_sign_in_callbacks_;
+  NotificationCallbacksMap signed_in_callbacks_;
+  NotificationCallbacksMap signed_out_callbacks_;
+  RequestCallbacksMap request_callbacks_;

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

nit: Member variables should always be private.  If subclasses need to access
them, please provide accessors.

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

Done.

+
+ private:
+  // Nulls out the raw pointer to |render_thread_| after ensuring that any
+  // message routing is removed.
+  void DisconnectFromRenderThread();
+
+  int routing_id_;
+  content::RenderThread* render_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(CredentialManagerDispatcher);
+};
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CONTENT_RENDERER_CREDENTIAL_MANAGER_DISPATCHER_H_