Fix password manager internals renderer reporting

chrome/browser/ui/passwords/manage_passwords_state_unittest.cc

Index: chrome/browser/ui/passwords/manage_passwords_state_unittest.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc
index b4bdb6684cbdb0d0031c632854ea47d1f2448947..cafa505c11fbc40ae239542a325081749a9c6aea 100644
--- a/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_state_unittest.cc
@@ -7,6 +7,7 @@
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
+#include "components/password_manager/core/browser/dummy_log_manager.h"
 #include "components/password_manager/core/browser/password_form_manager.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
@@ -28,8 +29,13 @@ namespace {
 class MockPasswordManagerClient
     : public password_manager::StubPasswordManagerClient {
  public:
-  MOCK_CONST_METHOD0(GetPasswordManager,
-                     const password_manager::PasswordManager*());
+  MockPasswordManagerClient() = default;
+  ~MockPasswordManagerClient() override = default;
+
+  MOCK_CONST_METHOD0(GetLogManager, const password_manager::LogManager*());

[vasilii, 2015/11/13 12:17:31]

Should you move this logic with DummyLogManager to StubPasswordManagerClient? I
see this code much more than once in this CL.

[vabr (Chromium), 2015/11/13 20:48:18]

I should move it, but not in this CL. The proper fix is tracked in
http://crbug.com/554886, and will likely result in removing the stubs and just
providing the mocks.

Why not in this CL -- if I were to introduce the mock client, I would need to
mock all the virtual methods (I do not want to assume where such a centrally
distributed mock client can be used). But then I would also need to provide a
good way to provide default return values, and also switch all the existing
places which mock the client. This CL is large already, I'd prefer to separate
that work.

[vasilii, 2015/11/16 14:12:25]

I don't buy this argument. If you add GetLogManager to StubPasswordManagerClient
that returns a   new member variable log_manager_, you'll make this CL smaller
and the code neater regardless of the upcoming refactoring.

[vabr (Chromium), 2015/11/16 16:08:48]

Yes. I was going to argue that StubPasswordManagerClient should not return
anything but nulls, but I see I already added the stub filter object to it in
the past :), so let's add the StubLogManager. I still need to keep the
StubLogManager separate, for tests where I actually do want to derive a mock
from it, like in
components/password_manager/content/browser/content_password_manager_driver_unittest.cc.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockPasswordManagerClient);
 };
 
 class ManagePasswordsStateTest : public testing::Test {
@@ -37,8 +43,8 @@ class ManagePasswordsStateTest : public testing::Test {
   ManagePasswordsStateTest() : password_manager_(&mock_client_) {}
 
   void SetUp() override {
-    ON_CALL(mock_client_, GetPasswordManager())
-        .WillByDefault(testing::Return(&password_manager_));
+    ON_CALL(mock_client_, GetLogManager())
+        .WillByDefault(testing::Return(&log_manager_));
 
     test_local_form_.origin = GURL("http://example.com");
     test_local_form_.username_value = base::ASCIIToUTF16("username");
@@ -76,6 +82,7 @@ class ManagePasswordsStateTest : public testing::Test {
                void(const password_manager::CredentialInfo&));
 
  private:
+  password_manager::DummyLogManager log_manager_;
   MockPasswordManagerClient mock_client_;
   password_manager::StubPasswordManagerDriver driver_;
   password_manager::PasswordManager password_manager_;