diff options
13 files changed, 451 insertions, 0 deletions
diff --git a/chrome/browser/password_manager/password_manager_internals_service_unittest.cc b/chrome/browser/password_manager/password_manager_internals_service_unittest.cc new file mode 100644 index 0000000..0903255 --- /dev/null +++ b/chrome/browser/password_manager/password_manager_internals_service_unittest.cc @@ -0,0 +1,79 @@ +// 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/core/browser/password_manager_internals_service.h" + +#include "chrome/test/base/testing_profile.h" +#include "components/keyed_service/content/browser_context_dependency_manager.h" +#include "components/password_manager/content/browser/password_manager_internals_service_factory.h" +#include "components/password_manager/core/browser/password_manager_logger.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using password_manager::PasswordManagerInternalsService; +using password_manager::PasswordManagerInternalsServiceFactory; + +namespace { + +const char kTestText[] = "abcd1234"; + +class MockLogReceiver : public password_manager::PasswordManagerLogger { + public: + MockLogReceiver() {} + + MOCK_METHOD1(LogSavePasswordProgress, void(const std::string&)); +}; + +enum ProfileType { NORMAL_PROFILE, INCOGNITO_PROFILE }; + +scoped_ptr<TestingProfile> CreateProfile(ProfileType type) { + TestingProfile::Builder builder; + if (type == INCOGNITO_PROFILE) + builder.SetIncognito(); + scoped_ptr<TestingProfile> profile(builder.Build()); +#if !defined(NDEBUG) + // During the test cases, the profiles may get created on the same address. To + // avoid over-zealous asserts we need to mark the newly created one as "live". + // See declaration of MarkBrowserContextLiveForTesting for more details. + BrowserContextDependencyManager::GetInstance() + ->MarkBrowserContextLiveForTesting(profile.get()); +#endif + return profile.Pass(); +} + +} // namespace + +// When the profile is not incognito, it should be possible to activate the +// service. +TEST(PasswordManagerInternalsServiceTest, ServiceActiveNonIncognito) { + scoped_ptr<TestingProfile> profile(CreateProfile(NORMAL_PROFILE)); + PasswordManagerInternalsService* service = + PasswordManagerInternalsServiceFactory::GetForBrowserContext( + profile.get()); + testing::StrictMock<MockLogReceiver> receiver; + + ASSERT_TRUE(profile); + ASSERT_TRUE(service); + EXPECT_EQ(std::string(), service->RegisterReceiver(&receiver)); + + // TODO(vabr): Use a MockPasswordManagerClient to detect activity changes. + EXPECT_CALL(receiver, LogSavePasswordProgress(kTestText)).Times(1); + service->ProcessLog(kTestText); + + service->UnregisterReceiver(&receiver); +} + +// When the browser profile is incognito, it should not be possible to activate +// the service. +TEST(PasswordManagerInternalsServiceTest, ServiceNotActiveIncognito) { + scoped_ptr<TestingProfile> profile(CreateProfile(INCOGNITO_PROFILE)); + ASSERT_TRUE(profile); + PasswordManagerInternalsService* service = + PasswordManagerInternalsServiceFactory::GetForBrowserContext( + profile.get()); + // BrowserContextKeyedBaseFactory::GetBrowserContextToUse should return NULL + // for |profile|, because |profile| is incognito. Therefore the returned + // |service| should also be NULL. + EXPECT_FALSE(service); +} diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 724c627..0821eea 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1132,6 +1132,7 @@ 'browser/password_manager/chrome_password_manager_client_unittest.cc', 'browser/password_manager/native_backend_gnome_x_unittest.cc', 'browser/password_manager/native_backend_kwallet_x_unittest.cc', + 'browser/password_manager/password_manager_internals_service_unittest.cc', 'browser/password_manager/password_manager_metrics_util_unittest.cc', 'browser/password_manager/password_store_mac_unittest.cc', 'browser/password_manager/password_store_win_unittest.cc', diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 2964f13..da7ab7c 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -108,6 +108,7 @@ 'os_crypt/keychain_password_mac_unittest.mm', 'os_crypt/os_crypt_unittest.cc', 'password_manager/core/browser/browser_save_password_progress_logger_unittest.cc', + 'password_manager/core/browser/log_router_unittest.cc', 'password_manager/core/browser/login_database_unittest.cc', 'password_manager/core/browser/password_autofill_manager_unittest.cc', 'password_manager/core/browser/password_form_manager_unittest.cc', diff --git a/components/password_manager.gypi b/components/password_manager.gypi index 58f7370..b396ba0 100644 --- a/components/password_manager.gypi +++ b/components/password_manager.gypi @@ -13,6 +13,7 @@ '../sql/sql.gyp:sql', '../url/url.gyp:url_lib', 'autofill_core_common', + 'keyed_service_core', 'os_crypt', 'password_manager_core_common', ], @@ -22,6 +23,8 @@ 'sources': [ 'password_manager/core/browser/browser_save_password_progress_logger.cc', 'password_manager/core/browser/browser_save_password_progress_logger.h', + 'password_manager/core/browser/log_router.cc', + 'password_manager/core/browser/log_router.h', 'password_manager/core/browser/login_database.cc', 'password_manager/core/browser/login_database.h', 'password_manager/core/browser/login_database_mac.cc', @@ -39,6 +42,8 @@ 'password_manager/core/browser/password_manager_client.cc', 'password_manager/core/browser/password_manager_client.h', 'password_manager/core/browser/password_manager_driver.h', + 'password_manager/core/browser/password_manager_internals_service.cc', + 'password_manager/core/browser/password_manager_internals_service.h', 'password_manager/core/browser/password_manager_logger.h', 'password_manager/core/browser/password_manager_metrics_util.cc', 'password_manager/core/browser/password_manager_metrics_util.h', @@ -146,6 +151,7 @@ 'autofill_content_browser', 'autofill_content_common', 'autofill_core_common', + 'keyed_service_content', 'password_manager_core_browser', '../base/base.gyp:base', '../content/content.gyp:content_browser', @@ -159,6 +165,8 @@ 'sources': [ 'password_manager/content/browser/content_password_manager_driver.cc', 'password_manager/content/browser/content_password_manager_driver.h', + 'password_manager/content/browser/password_manager_internals_service_factory.cc', + 'password_manager/content/browser/password_manager_internals_service_factory.h', ], }, ], diff --git a/components/password_manager/content/browser/DEPS b/components/password_manager/content/browser/DEPS index 736a93e..7f4d25a 100644 --- a/components/password_manager/content/browser/DEPS +++ b/components/password_manager/content/browser/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+components/autofill/content/browser", + "+components/keyed_service/content", "+content/public/browser", "+net", ] diff --git a/components/password_manager/content/browser/password_manager_internals_service_factory.cc b/components/password_manager/content/browser/password_manager_internals_service_factory.cc new file mode 100644 index 0000000..7641ca84 --- /dev/null +++ b/components/password_manager/content/browser/password_manager_internals_service_factory.cc @@ -0,0 +1,41 @@ +// 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/browser/password_manager_internals_service_factory.h" + +#include "components/keyed_service/content/browser_context_dependency_manager.h" +#include "components/password_manager/core/browser/password_manager_internals_service.h" + +namespace password_manager { + +// static +PasswordManagerInternalsService* +PasswordManagerInternalsServiceFactory::GetForBrowserContext( + content::BrowserContext* context) { + return static_cast<PasswordManagerInternalsService*>( + GetInstance()->GetServiceForBrowserContext(context, /* create = */ true)); +} + +// static +PasswordManagerInternalsServiceFactory* +PasswordManagerInternalsServiceFactory::GetInstance() { + return Singleton<PasswordManagerInternalsServiceFactory>::get(); +} + +PasswordManagerInternalsServiceFactory::PasswordManagerInternalsServiceFactory() + : BrowserContextKeyedServiceFactory( + "PasswordManagerInternalsService", + BrowserContextDependencyManager::GetInstance()) { +} + +PasswordManagerInternalsServiceFactory:: + ~PasswordManagerInternalsServiceFactory() { +} + +KeyedService* PasswordManagerInternalsServiceFactory::BuildServiceInstanceFor( + content::BrowserContext* /* context */) const { + return new PasswordManagerInternalsService(); +} + +} // namespace password_manager diff --git a/components/password_manager/content/browser/password_manager_internals_service_factory.h b/components/password_manager/content/browser/password_manager_internals_service_factory.h new file mode 100644 index 0000000..69808c3 --- /dev/null +++ b/components/password_manager/content/browser/password_manager_internals_service_factory.h @@ -0,0 +1,43 @@ +// 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_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_FACTORY_H_ +#define COMPONENTS_PASSWORD_MANAGER_CONTENT_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_FACTORY_H_ + +#include "base/memory/singleton.h" +#include "components/keyed_service/content/browser_context_keyed_service_factory.h" + +namespace content { +class BrowserContext; +} + +namespace password_manager { + +class PasswordManagerInternalsService; + +// BrowserContextKeyedServiceFactory for PasswordManagerInternalsService. +class PasswordManagerInternalsServiceFactory + : public BrowserContextKeyedServiceFactory { + public: + static PasswordManagerInternalsService* GetForBrowserContext( + content::BrowserContext* context); + + static PasswordManagerInternalsServiceFactory* GetInstance(); + + private: + friend struct DefaultSingletonTraits<PasswordManagerInternalsServiceFactory>; + + PasswordManagerInternalsServiceFactory(); + virtual ~PasswordManagerInternalsServiceFactory(); + + // BrowserContextKeyedServiceFactory: + virtual KeyedService* BuildServiceInstanceFor( + content::BrowserContext* context) const OVERRIDE; + + DISALLOW_COPY_AND_ASSIGN(PasswordManagerInternalsServiceFactory); +}; + +} // namespace password_manager + +#endif // COMPONENTS_PASSWORD_MANAGER_CONTENT_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_FACTORY_H_ diff --git a/components/password_manager/core/browser/DEPS b/components/password_manager/core/browser/DEPS index e0699b4..ddba66f 100644 --- a/components/password_manager/core/browser/DEPS +++ b/components/password_manager/core/browser/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+components/autofill/core/browser", + "+components/keyed_service/core", "+components/user_prefs", "+content/public/browser", ] diff --git a/components/password_manager/core/browser/log_router.cc b/components/password_manager/core/browser/log_router.cc new file mode 100644 index 0000000..c8719a8 --- /dev/null +++ b/components/password_manager/core/browser/log_router.cc @@ -0,0 +1,59 @@ +// 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/core/browser/log_router.h" + +#include "base/stl_util.h" +#include "components/password_manager/core/browser/password_manager_client.h" +#include "components/password_manager/core/browser/password_manager_logger.h" + +namespace password_manager { + +LogRouter::LogRouter() { +} + +LogRouter::~LogRouter() { +} + +void LogRouter::ProcessLog(const std::string& text) { + // This may not be called when there are no receivers (i.e., the router is + // inactive), because in that case the logs cannot be displayed. + DCHECK(receivers_.might_have_observers()); + accumulated_logs_.append(text); + FOR_EACH_OBSERVER( + PasswordManagerLogger, receivers_, LogSavePasswordProgress(text)); +} + +bool LogRouter::RegisterClient(PasswordManagerClient* client) { + DCHECK(client); + clients_.AddObserver(client); + return receivers_.might_have_observers(); +} + +void LogRouter::UnregisterClient(PasswordManagerClient* client) { + DCHECK(clients_.HasObserver(client)); + clients_.RemoveObserver(client); +} + +std::string LogRouter::RegisterReceiver(PasswordManagerLogger* receiver) { + DCHECK(receiver); + DCHECK(accumulated_logs_.empty() || receivers_.might_have_observers()); + + // TODO(vabr): Once the clients provide API for that, notify them if the + // number of receivers went from 0 to 1. + receivers_.AddObserver(receiver); + return accumulated_logs_; +} + +void LogRouter::UnregisterReceiver(PasswordManagerLogger* receiver) { + DCHECK(receivers_.HasObserver(receiver)); + receivers_.RemoveObserver(receiver); + if (!receivers_.might_have_observers()) { + accumulated_logs_.clear(); + // TODO(vabr): Once the clients provide API for that, notify them that the + // number of receivers went from 1 to 0. + } +} + +} // namespace password_manager diff --git a/components/password_manager/core/browser/log_router.h b/components/password_manager/core/browser/log_router.h new file mode 100644 index 0000000..c7ab218 --- /dev/null +++ b/components/password_manager/core/browser/log_router.h @@ -0,0 +1,68 @@ +// 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_CORE_BROWSER_LOG_ROUTER_H_ +#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOG_ROUTER_H_ + +#include <set> +#include <string> + +#include "base/macros.h" +#include "base/observer_list.h" + +namespace password_manager { + +class PasswordManagerClient; +class PasswordManagerLogger; + +// The router stands between PasswordManagerClient instances and log receivers. +// During the process of saving a password, the password manager code generates +// the log strings, and passes them to the router. The router distributes the +// logs to the receivers for displaying. +// +// TODO(vabr): The receivers are objects of type PasswordManagerLogger. That +// type should be renamed to LogReceiver instead. +class LogRouter { + public: + LogRouter(); + virtual ~LogRouter(); + + // Passes logs to the router. Only call when there are receivers registered. + void ProcessLog(const std::string& text); + + // All four (Unr|R)egister* methods below are safe to call from the + // constructor of the registered object, because they do not call that object, + // and the router only runs on a single thread. + + // The clients must register to be notified about whether there are some + // receivers or not. RegisterClient adds |client| to the right observer list + // and returns true iff there are some receivers registered. + bool RegisterClient(PasswordManagerClient* client); + // Remove |client| from the observers list. + void UnregisterClient(PasswordManagerClient* client); + + // The receivers must register to get updates with new logs in the future. + // RegisterReceiver adds |receiver| to the right observer list, and returns + // the logs accumulated so far. (It returns by value, not const ref, to + // provide a snapshot as opposed to a link to |accumulated_logs_|.) + std::string RegisterReceiver(PasswordManagerLogger* receiver); + // Remove |receiver| from the observers list. + void UnregisterReceiver(PasswordManagerLogger* receiver); + + private: + // Observer lists for clients and receivers. The |true| in the template + // specialisation means that they will check that all observers were removed + // on destruction. + ObserverList<PasswordManagerClient, true> clients_; + ObserverList<PasswordManagerLogger, true> receivers_; + + // Logs accumulated since the first receiver was registered. + std::string accumulated_logs_; + + DISALLOW_COPY_AND_ASSIGN(LogRouter); +}; + +} // namespace password_manager + +#endif // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOG_ROUTER_H_ diff --git a/components/password_manager/core/browser/log_router_unittest.cc b/components/password_manager/core/browser/log_router_unittest.cc new file mode 100644 index 0000000..b890998 --- /dev/null +++ b/components/password_manager/core/browser/log_router_unittest.cc @@ -0,0 +1,96 @@ +// 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/core/browser/log_router.h" + +#include "components/password_manager/core/browser/password_manager_logger.h" +#include "components/password_manager/core/browser/stub_password_manager_client.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::_; + +namespace password_manager { + +namespace { + +const char kTestText[] = "abcd1234"; + +class MockLogReceiver : public PasswordManagerLogger { + public: + MockLogReceiver() {} + + MOCK_METHOD1(LogSavePasswordProgress, void(const std::string&)); +}; + +} // namespace + +class LogRouterTest : public testing::Test { + protected: + testing::StrictMock<MockLogReceiver> receiver_; + testing::StrictMock<MockLogReceiver> receiver2_; +}; + +TEST_F(LogRouterTest, ProcessLog_NoReceiver) { + LogRouter router; + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + EXPECT_CALL(receiver_, LogSavePasswordProgress(kTestText)).Times(1); + router.ProcessLog(kTestText); + router.UnregisterReceiver(&receiver_); + // Without receivers, accumulated logs should not have been kept. That means + // that on the registration of the first receiver, none are returned. + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + router.UnregisterReceiver(&receiver_); +} + +TEST_F(LogRouterTest, ProcessLog_OneReceiver) { + LogRouter router; + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + // Check that logs generated after activation are passed. + EXPECT_CALL(receiver_, LogSavePasswordProgress(kTestText)).Times(1); + router.ProcessLog(kTestText); + router.UnregisterReceiver(&receiver_); +} + +TEST_F(LogRouterTest, ProcessLog_TwoReceiversAccumulatedLogsPassed) { + LogRouter router; + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + + // Log something with only the first receiver, to accumulate some logs. + EXPECT_CALL(receiver_, LogSavePasswordProgress(kTestText)).Times(1); + EXPECT_CALL(receiver2_, LogSavePasswordProgress(kTestText)).Times(0); + router.ProcessLog(kTestText); + // Accumulated logs get passed on registration. + EXPECT_EQ(kTestText, router.RegisterReceiver(&receiver2_)); + router.UnregisterReceiver(&receiver_); + router.UnregisterReceiver(&receiver2_); +} + +TEST_F(LogRouterTest, ProcessLog_TwoReceiversBothUpdated) { + LogRouter router; + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver2_)); + + // Check that both receivers get log updates. + EXPECT_CALL(receiver_, LogSavePasswordProgress(kTestText)).Times(1); + EXPECT_CALL(receiver2_, LogSavePasswordProgress(kTestText)).Times(1); + router.ProcessLog(kTestText); + router.UnregisterReceiver(&receiver2_); + router.UnregisterReceiver(&receiver_); +} + +TEST_F(LogRouterTest, ProcessLog_TwoReceiversNoUpdateAfterUnregistering) { + LogRouter router; + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver_)); + EXPECT_EQ(std::string(), router.RegisterReceiver(&receiver2_)); + + // Check that no logs are passed to an unregistered receiver. + router.UnregisterReceiver(&receiver_); + EXPECT_CALL(receiver_, LogSavePasswordProgress(_)).Times(0); + EXPECT_CALL(receiver2_, LogSavePasswordProgress(kTestText)).Times(1); + router.ProcessLog(kTestText); + router.UnregisterReceiver(&receiver2_); +} + +} // namespace password_manager diff --git a/components/password_manager/core/browser/password_manager_internals_service.cc b/components/password_manager/core/browser/password_manager_internals_service.cc new file mode 100644 index 0000000..3d80b32 --- /dev/null +++ b/components/password_manager/core/browser/password_manager_internals_service.cc @@ -0,0 +1,15 @@ +// 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/core/browser/password_manager_internals_service.h" + +namespace password_manager { + +PasswordManagerInternalsService::PasswordManagerInternalsService() { +} + +PasswordManagerInternalsService::~PasswordManagerInternalsService() { +} + +} // namespace password_manager diff --git a/components/password_manager/core/browser/password_manager_internals_service.h b/components/password_manager/core/browser/password_manager_internals_service.h new file mode 100644 index 0000000..b0d5f7a --- /dev/null +++ b/components/password_manager/core/browser/password_manager_internals_service.h @@ -0,0 +1,38 @@ +// 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_CORE_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_ +#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_ + +#include <string> + +#include "components/keyed_service/core/keyed_service.h" +#include "components/password_manager/core/browser/log_router.h" + +namespace content { +class BrowserContext; +} + +namespace password_manager { + +// Collects the logs for the password manager internals page and distributes +// them to all open tabs with the internals page. +class PasswordManagerInternalsService : public KeyedService, + public LogRouter { + public: + // There are only two ways in which the service depends on the BrowserContext: + // 1) There is one service per each non-incognito BrowserContext. + // 2) No service will be created for an incognito BrowserContext. + // Both properties are guarantied by the BrowserContextKeyedFactory framework, + // so the service itself does not need the context on creation. + PasswordManagerInternalsService(); + virtual ~PasswordManagerInternalsService(); + + private: + DISALLOW_COPY_AND_ASSIGN(PasswordManagerInternalsService); +}; + +} // namespace password_manager + +#endif // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_PASSWORD_MANAGER_INTERNALS_SERVICE_H_ |