summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormsramek <msramek@chromium.org>2015-08-13 12:22:11 -0700
committerCommit bot <commit-bot@chromium.org>2015-08-13 19:23:01 +0000
commita3e5f5f683aafbad37f1ef3e203d727509ccd526 (patch)
treeec7522f22e376bb2a3031b056fcecc23b38e649a
parent18e765a7caeaa9951a3fa0a5f9718f6ea75226f4 (diff)
downloadchromium_src-a3e5f5f683aafbad37f1ef3e203d727509ccd526.zip
chromium_src-a3e5f5f683aafbad37f1ef3e203d727509ccd526.tar.gz
chromium_src-a3e5f5f683aafbad37f1ef3e203d727509ccd526.tar.bz2
Add a passwords counter to the Clear Browsing Data dialog.
The counting is restarted whenever the deletion preference or the contents of the password store change. This CL also contains some polishing of the parent BrowsingDataCounter class. BUG=510028 Review URL: https://codereview.chromium.org/1275593002 Cr-Commit-Position: refs/heads/master@{#343254}
-rw-r--r--chrome/browser/browsing_data/browsing_data_counter.cc18
-rw-r--r--chrome/browser/browsing_data/browsing_data_counter.h15
-rw-r--r--chrome/browser/browsing_data/passwords_counter.cc37
-rw-r--r--chrome/browser/browsing_data/passwords_counter.h20
-rw-r--r--chrome/browser/browsing_data/passwords_counter_browsertest.cc170
-rw-r--r--chrome/chrome_tests.gypi1
6 files changed, 246 insertions, 15 deletions
diff --git a/chrome/browser/browsing_data/browsing_data_counter.cc b/chrome/browser/browsing_data/browsing_data_counter.cc
index 07700a9..39b14cd 100644
--- a/chrome/browser/browsing_data/browsing_data_counter.cc
+++ b/chrome/browser/browsing_data/browsing_data_counter.cc
@@ -24,14 +24,22 @@ void BrowsingDataCounter::Init(
pref_.Init(
GetPrefName(),
profile_->GetPrefs(),
- base::Bind(&BrowsingDataCounter::OnPrefChanged,
+ base::Bind(&BrowsingDataCounter::RestartCounting,
base::Unretained(this)));
initialized_ = true;
- OnPrefChanged();
+ OnInitialized();
+ RestartCounting();
}
-void BrowsingDataCounter::OnPrefChanged() {
+Profile* BrowsingDataCounter::GetProfile() const {
+ return profile_;
+}
+
+void BrowsingDataCounter::OnInitialized() {
+}
+
+void BrowsingDataCounter::RestartCounting() {
DCHECK(initialized_);
// If this data type was unchecked for deletion, we do not need to count it.
@@ -48,10 +56,6 @@ void BrowsingDataCounter::OnPrefChanged() {
Count();
}
-// virtual
-void BrowsingDataCounter::Count() {
-}
-
void BrowsingDataCounter::ReportResult(uint32 value) {
DCHECK(initialized_);
DCHECK(counting_);
diff --git a/chrome/browser/browsing_data/browsing_data_counter.h b/chrome/browser/browsing_data/browsing_data_counter.h
index bfc0db2..f9e0d26 100644
--- a/chrome/browser/browsing_data/browsing_data_counter.h
+++ b/chrome/browser/browsing_data/browsing_data_counter.h
@@ -25,18 +25,25 @@ class BrowsingDataCounter {
// Name of the preference associated with this counter.
virtual const std::string& GetPrefName() const = 0;
+ // The profile associated with this counter.
+ Profile* GetProfile() const;
+
protected:
+ // Called when some conditions have changed and the counting should be
+ // restarted, e.g. when the deletion preference changed state or when
+ // we were notified of data changes.
+ void RestartCounting();
+
// Should be called from |Count| by any overriding class to indicate that
// counting is finished and report the |result|.
void ReportResult(uint32 result);
private:
- // Called when the boolean preference indicating whether this data type
- // is to be deleted changes.
- void OnPrefChanged();
+ // Called after the class is initialized by calling |Init|.
+ virtual void OnInitialized();
// Count the data.
- virtual void Count();
+ virtual void Count() = 0;
// The profile for which we will count the data volume.
Profile* profile_;
diff --git a/chrome/browser/browsing_data/passwords_counter.cc b/chrome/browser/browsing_data/passwords_counter.cc
index 30b0dd7..340450c 100644
--- a/chrome/browser/browsing_data/passwords_counter.cc
+++ b/chrome/browser/browsing_data/passwords_counter.cc
@@ -3,13 +3,27 @@
// found in the LICENSE file.
#include "chrome/browser/browsing_data/passwords_counter.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
#include "chrome/common/pref_names.h"
+#include "components/password_manager/core/browser/password_store.h"
PasswordsCounter::PasswordsCounter()
- : pref_name_(prefs::kDeletePasswords) {
+ : pref_name_(prefs::kDeletePasswords),
+ store_(nullptr) {
}
PasswordsCounter::~PasswordsCounter() {
+ if (store_)
+ store_->RemoveObserver(this);
+}
+
+void PasswordsCounter::OnInitialized() {
+ store_ = PasswordStoreFactory::GetForProfile(
+ GetProfile(), ServiceAccessType::EXPLICIT_ACCESS).get();
+ if (store_)
+ store_->AddObserver(this);
+ else
+ LOG(ERROR) << "No password store! Cannot count passwords.";
}
const std::string& PasswordsCounter::GetPrefName() const {
@@ -17,6 +31,23 @@ const std::string& PasswordsCounter::GetPrefName() const {
}
void PasswordsCounter::Count() {
- // TODO(msramek): Count the passwords of |profile|
- // and call |ReportResult| when finished.
+ if (!store_) {
+ ReportResult(0);
+ return;
+ }
+
+ cancelable_task_tracker()->TryCancelAll();
+ // TODO(msramek): We don't actually need the logins themselves, just their
+ // count. Consider implementing |PasswordStore::CountAutofillableLogins|.
+ store_->GetAutofillableLogins(this);
+}
+
+void PasswordsCounter::OnGetPasswordStoreResults(
+ ScopedVector<autofill::PasswordForm> results) {
+ ReportResult(results.size());
+}
+
+void PasswordsCounter::OnLoginsChanged(
+ const password_manager::PasswordStoreChangeList& changes) {
+ RestartCounting();
}
diff --git a/chrome/browser/browsing_data/passwords_counter.h b/chrome/browser/browsing_data/passwords_counter.h
index 1a94c40..211f0faa1b 100644
--- a/chrome/browser/browsing_data/passwords_counter.h
+++ b/chrome/browser/browsing_data/passwords_counter.h
@@ -6,8 +6,12 @@
#define CHROME_BROWSER_BROWSING_DATA_PASSWORDS_COUNTER_H_
#include "chrome/browser/browsing_data/browsing_data_counter.h"
+#include "components/password_manager/core/browser/password_store.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
-class PasswordsCounter: public BrowsingDataCounter {
+class PasswordsCounter: public BrowsingDataCounter,
+ public password_manager::PasswordStoreConsumer,
+ public password_manager::PasswordStore::Observer {
public:
PasswordsCounter();
~PasswordsCounter() override;
@@ -16,6 +20,20 @@ class PasswordsCounter: public BrowsingDataCounter {
private:
const std::string pref_name_;
+ base::CancelableTaskTracker cancelable_task_tracker_;
+ password_manager::PasswordStore* store_;
+
+ void OnInitialized() override;
+
+ // Counting is done asynchronously in a request to PasswordStore.
+ // This callback returns the results, which are subsequently reported.
+ void OnGetPasswordStoreResults(
+ ScopedVector<autofill::PasswordForm> results) override;
+
+ // Called when the contents of the password store change. Triggers new
+ // counting.
+ void OnLoginsChanged(
+ const password_manager::PasswordStoreChangeList& changes) override;
void Count() override;
};
diff --git a/chrome/browser/browsing_data/passwords_counter_browsertest.cc b/chrome/browser/browsing_data/passwords_counter_browsertest.cc
new file mode 100644
index 0000000..32504bc
--- /dev/null
+++ b/chrome/browser/browsing_data/passwords_counter_browsertest.cc
@@ -0,0 +1,170 @@
+// Copyright (c) 2015 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 "chrome/browser/browsing_data/passwords_counter.h"
+
+#include "base/prefs/pref_service.h"
+#include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/autofill/core/common/password_form.h"
+
+namespace {
+
+using autofill::PasswordForm;
+
+class PasswordsCounterTest : public InProcessBrowserTest {
+ public:
+ void AddLogin(const std::string& origin,
+ const std::string& username,
+ bool blacklisted) {
+ PasswordStoreFactory::GetForProfile(
+ browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)->
+ AddLogin(CreateCredentials(origin, username, blacklisted));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void RemoveLogin(const std::string& origin,
+ const std::string& username,
+ bool blacklisted) {
+ PasswordStoreFactory::GetForProfile(
+ browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS)->
+ RemoveLogin(CreateCredentials(origin, username, blacklisted));
+ base::RunLoop().RunUntilIdle();
+ }
+
+ void SetPasswordsDeletionPref(bool value) {
+ browser()->profile()->GetPrefs()->SetBoolean(
+ prefs::kDeletePasswords, value);
+ }
+
+ void WaitForCounting() {
+ run_loop_.reset(new base::RunLoop());
+ run_loop_->Run();
+ }
+
+ uint32 GetResult() {
+ DCHECK(finished_);
+ return result_;
+ }
+
+ void Callback(bool finished, uint32 count) {
+ finished_ = finished;
+ result_ = count;
+ if (run_loop_ && finished)
+ run_loop_->Quit();
+ }
+
+ private:
+ PasswordForm CreateCredentials(const std::string& origin,
+ const std::string& username,
+ bool blacklisted) {
+ PasswordForm result;
+ result.signon_realm = origin;
+ result.origin = GURL(origin);
+ result.username_value = base::ASCIIToUTF16(username);
+ result.password_value = base::ASCIIToUTF16("hunter2");
+ result.blacklisted_by_user = blacklisted;
+ return result;
+ }
+
+ scoped_ptr<base::RunLoop> run_loop_;
+
+ bool finished_;
+ uint32 result_;
+};
+
+// Tests that the counter correctly counts each individual credential on
+// the same domain.
+IN_PROC_BROWSER_TEST_F(PasswordsCounterTest, SameDomain) {
+ SetPasswordsDeletionPref(true);
+ AddLogin("https://www.google.com", "user1", false);
+ AddLogin("https://www.google.com", "user2", false);
+ AddLogin("https://www.google.com", "user3", false);
+ AddLogin("https://www.chrome.com", "user1", false);
+ AddLogin("https://www.chrome.com", "user2", false);
+
+ PasswordsCounter counter;
+ counter.Init(browser()->profile(),
+ base::Bind(&PasswordsCounterTest::Callback,
+ base::Unretained(this)));
+
+ WaitForCounting();
+ EXPECT_EQ(5u, GetResult());
+}
+
+// Tests that the counter doesn't count blacklisted entries.
+IN_PROC_BROWSER_TEST_F(PasswordsCounterTest, Blacklisted) {
+ SetPasswordsDeletionPref(true);
+ AddLogin("https://www.google.com", "user1", false);
+ AddLogin("https://www.google.com", "user2", true);
+ AddLogin("https://www.chrome.com", "user3", true);
+
+ PasswordsCounter counter;
+ counter.Init(browser()->profile(),
+ base::Bind(&PasswordsCounterTest::Callback,
+ base::Unretained(this)));
+
+ WaitForCounting();
+ EXPECT_EQ(1u, GetResult());
+}
+
+// Tests that the counter starts counting automatically when the deletion
+// pref changes to true.
+IN_PROC_BROWSER_TEST_F(PasswordsCounterTest, PrefChanged) {
+ SetPasswordsDeletionPref(false);
+ AddLogin("https://www.google.com", "user", false);
+ AddLogin("https://www.chrome.com", "user", false);
+
+ PasswordsCounter counter;
+ counter.Init(browser()->profile(),
+ base::Bind(&PasswordsCounterTest::Callback,
+ base::Unretained(this)));
+ SetPasswordsDeletionPref(true);
+
+ WaitForCounting();
+ EXPECT_EQ(2u, GetResult());
+}
+
+// Tests that the counter does not count passwords if the deletion
+// preference is false.
+IN_PROC_BROWSER_TEST_F(PasswordsCounterTest, PrefIsFalse) {
+ SetPasswordsDeletionPref(false);
+ AddLogin("https://www.google.com", "user", false);
+
+ PasswordsCounter counter;
+ counter.Init(browser()->profile(),
+ base::Bind(&PasswordsCounterTest::Callback,
+ base::Unretained(this)));
+
+ EXPECT_FALSE(counter.cancelable_task_tracker()->HasTrackedTasks());
+}
+
+// Tests that the counter starts counting automatically when
+// the password store changes.
+IN_PROC_BROWSER_TEST_F(PasswordsCounterTest, StoreChanged) {
+ SetPasswordsDeletionPref(true);
+ AddLogin("https://www.google.com", "user", false);
+
+ PasswordsCounter counter;
+ counter.Init(browser()->profile(),
+ base::Bind(&PasswordsCounterTest::Callback,
+ base::Unretained(this)));
+
+ WaitForCounting();
+ EXPECT_EQ(1u, GetResult());
+
+ AddLogin("https://www.chrome.com", "user", false);
+ WaitForCounting();
+ EXPECT_EQ(2u, GetResult());
+
+ RemoveLogin("https://www.chrome.com", "user", false);
+ WaitForCounting();
+ EXPECT_EQ(1u, GetResult());
+}
+
+} // namespace
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 39c28d6..be2c0f3 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -79,6 +79,7 @@
'browser/browsing_data/browsing_data_remover_browsertest.cc',
'browser/browsing_data/browsing_data_remover_test_util.cc',
'browser/browsing_data/browsing_data_remover_test_util.h',
+ 'browser/browsing_data/passwords_counter_browsertest.cc',
'browser/chrome_content_browser_client_browsertest.cc',
'browser/chrome_main_browsertest.cc',
'browser/chrome_plugin_browsertest.cc',