Credential Manager: Renderer-side implementation.

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 2aa64c2b23c62402b30db95333a606002ecd24f5..b7b7ccbfdcb69cd913e14acf5928af531affea8d 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -74,6 +74,7 @@
 #include "components/autofill/content/renderer/password_generation_agent.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/nacl/renderer/ppb_nacl_private_impl.h"
+#include "components/password_manager/content/renderer/credential_manager_client.h"
 #include "components/plugins/renderer/mobile_youtube_plugin.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "components/visitedlink/renderer/visitedlink_slave.h"
@@ -314,6 +315,14 @@ void ChromeContentRendererClient::RenderThreadStarted() {
 #endif
   search_bouncer_.reset(new SearchBouncer());
 
+  // ChromeRenderViewTest::SetUp() creates its own CredentialManagerClient and

[vabr (Chromium), 2014/08/20 14:40:34]

It would be great if the production code did not know about tests at all. Checks
like this might hide a bug later. I'm not blocking this CL on removing this test
dependency (because it's currently just a minor thing, and I don't know how to
fix it), but if there is a chance to deal with the clashing
CredentialManagerClient instances in the test rather than in the prod. code, we
should do that.

[Ilya Sherman, 2014/08/21 00:11:21]

+1, especially since it doesn't look like the test version of the client is
actually used in the Chrome layer.

[Mike West, 2014/08/21 10:20:46]

This isn't something I invented; I lifted this model from both Spellcheck and
ExtensionDelegate's implementation (lines 290 and 300 above in the same method).
Autofill does something similar in ChromeRenderViewTest, but it's sneakier since
it's implemented as a RenderViewObserver.

I'll try to rearrange things a bit, but there are a number of browser tests that
don't set things up in a way that CredentialManager appreciates. I'm looking for
the least-bad way of enabling them without teaching unrelated tests about things
they shouldn't need to care about.

+  // injects it using SetCredentialManagerClientForTesting(). Don't overwrite
+  // it.
+  if (!credential_manager_client_) {
+    credential_manager_client_.reset(
+        new password_manager::CredentialManagerClient());
+  }
+
   thread->AddObserver(chrome_observer_.get());
   thread->AddObserver(extension_dispatcher_.get());
 #if defined(FULL_SAFE_BROWSING)
@@ -322,6 +331,7 @@ void ChromeContentRendererClient::RenderThreadStarted() {
   thread->AddObserver(visited_link_slave_.get());
   thread->AddObserver(prerender_dispatcher_.get());
   thread->AddObserver(search_bouncer_.get());
+  thread->AddObserver(credential_manager_client_.get());
 
 #if defined(ENABLE_WEBRTC)
   thread->AddFilter(webrtc_logging_message_filter_.get());
@@ -493,6 +503,8 @@ void ChromeContentRendererClient::RenderViewCreated(
     new SearchBox(render_view);
 
   new ChromeRenderViewObserver(render_view, chrome_observer_.get());
+
+  credential_manager_client_->OnRenderViewCreated(render_view);
 }
 
 void ChromeContentRendererClient::SetNumberOfViews(int number_of_views) {
@@ -1306,6 +1318,11 @@ bool ChromeContentRendererClient::ShouldOverridePageVisibilityState(
   return true;
 }
 
+void ChromeContentRendererClient::SetCredentialManagerClientForTest(
+    password_manager::CredentialManagerClient* client) {
+  credential_manager_client_.reset(client);
+}
+
 void ChromeContentRendererClient::SetExtensionDispatcherForTest(
     extensions::Dispatcher* extension_dispatcher) {
   extension_dispatcher_.reset(extension_dispatcher);