When looking at this bug, I found at least the following issues:
1) PasswordManagerHandler was not canceling requests if new requests were made (by double-clicking the "Manage Saved Passwords..." button with a slow database - simulated with long Sleep).
2) PasswordStore starts its handle numbering at 0 meaning that the very first request is non-true and could hit DCHECK(s).
3) PasswordManagerHandler doesn't free the results.
When talking with the DOMUI TL (jhawkins) and an author of PasswordStore (stuartmorgan), I learned that it was modeled after HistoryService, which had since been refactored into a reusable suite of classes in content/browser/cancelable_request.h
So the CL will include fixes for the 3 issues, as well as a refactor to use the shared code in cancelable_requst.h
BUG=71466
TEST=chrome://settings/personal, then click "Manage Saved Passwords". Put
PlatformThread::Sleep(10000), then try double-clicking the button to see no error.
Review URL: http://codereview.chromium.org/6646051
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79625 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/base/base.gyp b/base/base.gyp
index 1315c96..0c2ac86 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -106,6 +106,7 @@
'memory/scoped_native_library_unittest.cc',
'memory/scoped_ptr_unittest.cc',
'memory/scoped_temp_dir_unittest.cc',
+ 'memory/scoped_vector_unittest.cc',
'memory/singleton_unittest.cc',
'memory/weak_ptr_unittest.cc',
'message_loop_proxy_impl_unittest.cc',
diff --git a/base/memory/scoped_vector.h b/base/memory/scoped_vector.h
index aec4375..6e0cf05 100644
--- a/base/memory/scoped_vector.h
+++ b/base/memory/scoped_vector.h
@@ -62,6 +62,12 @@
return v.insert(position, x);
}
+ // Lets the ScopedVector take ownership of elements in [first,last).
+ template<typename InputIterator>
+ void insert(iterator position, InputIterator first, InputIterator last) {
+ v.insert(position, first, last);
+ }
+
iterator erase(iterator position) {
delete *position;
return v.erase(position);
diff --git a/base/memory/scoped_vector_unittest.cc b/base/memory/scoped_vector_unittest.cc
new file mode 100644
index 0000000..d2f3d0aef
--- /dev/null
+++ b/base/memory/scoped_vector_unittest.cc
@@ -0,0 +1,156 @@
+// Copyright (c) 2011 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 "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// The LifeCycleObject notifies its Observer upon construction & destruction.
+class LifeCycleObject {
+ public:
+ class Observer {
+ public:
+ virtual void OnLifeCycleConstruct(LifeCycleObject* o) = 0;
+ virtual void OnLifeCycleDestroy(LifeCycleObject* o) = 0;
+
+ protected:
+ virtual ~Observer() {}
+ };
+
+ explicit LifeCycleObject(Observer* observer)
+ : observer_(observer) {
+ observer_->OnLifeCycleConstruct(this);
+ }
+
+ ~LifeCycleObject() {
+ observer_->OnLifeCycleDestroy(this);
+ }
+
+ private:
+ Observer* observer_;
+
+ DISALLOW_COPY_AND_ASSIGN(LifeCycleObject);
+};
+
+// The life cycle states we care about for the purposes of testing ScopedVector
+// against objects.
+enum LifeCycleState {
+ LC_INITIAL,
+ LC_CONSTRUCTED,
+ LC_DESTROYED,
+};
+
+// Because we wish to watch the life cycle of an object being constructed and
+// destroyed, and further wish to test expectations against the state of that
+// object, we cannot save state in that object itself. Instead, we use this
+// pairing of the watcher, which observes the object and notifies of
+// construction & destruction. Since we also may be testing assumptions about
+// things not getting freed, this class also acts like a scoping object and
+// deletes the |constructed_life_cycle_object_|, if any when the
+// LifeCycleWatcher is destroyed. To keep this simple, the only expected state
+// changes are:
+// INITIAL -> CONSTRUCTED -> DESTROYED.
+// Anything more complicated than that should start another test.
+class LifeCycleWatcher : public LifeCycleObject::Observer {
+ public:
+ LifeCycleWatcher()
+ : life_cycle_state_(LC_INITIAL),
+ constructed_life_cycle_object_(NULL) {}
+ ~LifeCycleWatcher() {
+ }
+
+ // Assert INITIAL -> CONSTRUCTED and no LifeCycleObject associated with this
+ // LifeCycleWatcher.
+ virtual void OnLifeCycleConstruct(LifeCycleObject* object) {
+ ASSERT_EQ(LC_INITIAL, life_cycle_state_);
+ ASSERT_EQ(NULL, constructed_life_cycle_object_.get());
+ life_cycle_state_ = LC_CONSTRUCTED;
+ constructed_life_cycle_object_.reset(object);
+ }
+
+ // Assert CONSTRUCTED -> DESTROYED and the |object| being destroyed is the
+ // same one we saw constructed.
+ virtual void OnLifeCycleDestroy(LifeCycleObject* object) {
+ ASSERT_EQ(LC_CONSTRUCTED, life_cycle_state_);
+ LifeCycleObject* constructed_life_cycle_object =
+ constructed_life_cycle_object_.release();
+ ASSERT_EQ(constructed_life_cycle_object, object);
+ life_cycle_state_ = LC_DESTROYED;
+ }
+
+ LifeCycleState life_cycle_state() const { return life_cycle_state_; }
+
+ // Factory method for creating a new LifeCycleObject tied to this
+ // LifeCycleWatcher.
+ LifeCycleObject* NewLifeCycleObject() {
+ return new LifeCycleObject(this);
+ }
+
+ private:
+ LifeCycleState life_cycle_state_;
+ scoped_ptr<LifeCycleObject> constructed_life_cycle_object_;
+
+ DISALLOW_COPY_AND_ASSIGN(LifeCycleWatcher);
+};
+
+TEST(ScopedVectorTest, LifeCycleWatcher) {
+ LifeCycleWatcher watcher;
+ EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+ LifeCycleObject* object = watcher.NewLifeCycleObject();
+ EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+ delete object;
+ EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, Reset) {
+ LifeCycleWatcher watcher;
+ EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+ ScopedVector<LifeCycleObject> scoped_vector;
+ scoped_vector.push_back(watcher.NewLifeCycleObject());
+ EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+ scoped_vector.reset();
+ EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, Scope) {
+ LifeCycleWatcher watcher;
+ EXPECT_EQ(LC_INITIAL, watcher.life_cycle_state());
+ {
+ ScopedVector<LifeCycleObject> scoped_vector;
+ scoped_vector.push_back(watcher.NewLifeCycleObject());
+ EXPECT_EQ(LC_CONSTRUCTED, watcher.life_cycle_state());
+ }
+ EXPECT_EQ(LC_DESTROYED, watcher.life_cycle_state());
+}
+
+TEST(ScopedVectorTest, InsertRange) {
+ LifeCycleWatcher watchers[5];
+
+ std::vector<LifeCycleObject*> vec;
+ for(LifeCycleWatcher* it = watchers; it != watchers + arraysize(watchers);
+ ++it) {
+ EXPECT_EQ(LC_INITIAL, it->life_cycle_state());
+ vec.push_back(it->NewLifeCycleObject());
+ EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+ }
+ // Start scope for ScopedVector.
+ {
+ ScopedVector<LifeCycleObject> scoped_vector;
+ scoped_vector.insert(scoped_vector.end(), vec.begin() + 1, vec.begin() + 3);
+ for(LifeCycleWatcher* it = watchers; it != watchers + arraysize(watchers);
+ ++it)
+ EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+ }
+ for(LifeCycleWatcher* it = watchers; it != watchers + 1; ++it)
+ EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+ for(LifeCycleWatcher* it = watchers + 1; it != watchers + 3; ++it)
+ EXPECT_EQ(LC_DESTROYED, it->life_cycle_state());
+ for(LifeCycleWatcher* it = watchers + 3; it != watchers + arraysize(watchers);
+ ++it)
+ EXPECT_EQ(LC_CONSTRUCTED, it->life_cycle_state());
+}
+
+} // namespace
diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index b246e1c..94b80a3 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -1464,7 +1464,8 @@
~AutomationProviderGetPasswordsObserver() {}
void AutomationProviderGetPasswordsObserver::OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result) {
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) {
if (!provider_) {
delete this;
return;
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index 0bac880..1802da8d5 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -27,7 +27,7 @@
#include "chrome/browser/history/history_types.h"
#include "chrome/browser/importer/importer_data_types.h"
#include "chrome/browser/importer/importer_progress_observer.h"
-#include "chrome/browser/password_manager/password_store.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/search_engines/template_url_model_observer.h"
#include "chrome/browser/tabs/tab_strip_model.h"
#include "chrome/common/automation_constants.h"
@@ -908,8 +908,7 @@
};
// Allows automation provider to wait for getting passwords to finish.
-class AutomationProviderGetPasswordsObserver
- : public PasswordStoreConsumer {
+class AutomationProviderGetPasswordsObserver : public PasswordStoreConsumer {
public:
AutomationProviderGetPasswordsObserver(
AutomationProvider* provider,
@@ -917,7 +916,8 @@
virtual ~AutomationProviderGetPasswordsObserver();
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result);
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result);
private:
base::WeakPtr<AutomationProvider> provider_;
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index 47e7f3ac..b93d9a0 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -51,6 +51,7 @@
#include "chrome/browser/notifications/balloon_collection.h"
#include "chrome/browser/notifications/notification.h"
#include "chrome/browser/notifications/notification_ui_manager.h"
+#include "chrome/browser/password_manager/password_store.h"
#include "chrome/browser/platform_util.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profiles/profile.h"
diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc
index 11efdd22..c0a6724 100644
--- a/chrome/browser/password_manager/password_form_manager.cc
+++ b/chrome/browser/password_manager/password_form_manager.cc
@@ -10,6 +10,7 @@
#include "base/string_split.h"
#include "base/string_util.h"
#include "chrome/browser/password_manager/password_manager.h"
+#include "chrome/browser/password_manager/password_store.h"
#include "chrome/browser/profiles/profile.h"
#include "webkit/glue/password_form_dom_manager.h"
@@ -39,7 +40,6 @@
}
PasswordFormManager::~PasswordFormManager() {
- CancelLoginsQuery();
UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
GetActionsTaken(),
kMaxNumActionsTaken);
@@ -306,7 +306,8 @@
}
void PasswordFormManager::OnPasswordStoreRequestDone(
- int handle, const std::vector<PasswordForm*>& result) {
+ CancelableRequestProvider::Handle handle,
+ const std::vector<PasswordForm*>& result) {
DCHECK_EQ(state_, MATCHING_PHASE);
DCHECK_EQ(pending_login_query_, handle);
@@ -316,6 +317,7 @@
}
OnRequestDone(handle, result);
+ pending_login_query_ = 0;
}
bool PasswordFormManager::IgnoreResult(const PasswordForm& form) const {
@@ -420,16 +422,6 @@
}
}
-void PasswordFormManager::CancelLoginsQuery() {
- PasswordStore* password_store =
- profile_->GetPasswordStore(Profile::EXPLICIT_ACCESS);
- if (!password_store) {
- // Can be NULL in unit tests.
- return;
- }
- password_store->CancelLoginsQuery(pending_login_query_);
-}
-
int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const {
DCHECK_EQ(state_, MATCHING_PHASE);
// For scoring of candidate login data:
diff --git a/chrome/browser/password_manager/password_form_manager.h b/chrome/browser/password_manager/password_form_manager.h
index 698c3e2b..f1c48bc 100644
--- a/chrome/browser/password_manager/password_form_manager.h
+++ b/chrome/browser/password_manager/password_form_manager.h
@@ -12,10 +12,11 @@
#include "build/build_config.h"
#include "base/stl_util-inl.h"
-#include "chrome/browser/password_manager/password_store.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "webkit/glue/password_form.h"
class PasswordManager;
+class PasswordStore;
class Profile;
// Per-password-form-{on-page, dialog} class responsible for interactions
@@ -72,7 +73,8 @@
// PasswordStoreConsumer implementation.
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result);
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result);
// A user opted to 'never remember' passwords for this form.
// Blacklist it so that from now on when it is seen we ignore it.
@@ -136,10 +138,6 @@
static const int kMaxNumActionsTaken = kManagerActionMax * kUserActionMax *
kSubmitResultMax;
- // Called by destructor to ensure if this object is deleted, no potential
- // outstanding callbacks can call OnPasswordStoreRequestDone.
- void CancelLoginsQuery();
-
// Helper for OnPasswordStoreRequestDone to determine whether or not
// the given result form is worth scoring.
bool IgnoreResult(const webkit_glue::PasswordForm& form) const;
@@ -197,7 +195,7 @@
const PasswordManager* const password_manager_;
// Handle to any pending PasswordStore::GetLogins query.
- int pending_login_query_;
+ CancelableRequestProvider::Handle pending_login_query_;
// Convenience pointer to entry in best_matches_ that is marked
// as preferred. This is only allowed to be null if there are no best matches
diff --git a/chrome/browser/password_manager/password_store.cc b/chrome/browser/password_manager/password_store.cc
index 1bab90f..7436a54 100644
--- a/chrome/browser/password_manager/password_store.cc
+++ b/chrome/browser/password_manager/password_store.cc
@@ -6,13 +6,27 @@
#include "base/memory/scoped_ptr.h"
#include "base/message_loop.h"
+#include "base/stl_util-inl.h"
#include "base/task.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "content/browser/browser_thread.h"
+#include "webkit/glue/password_form.h"
using std::vector;
using webkit_glue::PasswordForm;
-PasswordStore::PasswordStore() : handle_(0) {
+PasswordStore::GetLoginsRequest::GetLoginsRequest(GetLoginsCallback* callback)
+ : CancelableRequest1<GetLoginsCallback,
+ std::vector<webkit_glue::PasswordForm*> >(callback) {
+}
+
+PasswordStore::GetLoginsRequest::~GetLoginsRequest() {
+ if (canceled()) {
+ STLDeleteElements(&value);
+ }
+}
+
+PasswordStore::PasswordStore() {
}
bool PasswordStore::Init() {
@@ -47,36 +61,19 @@
NewRunnableMethod(this, &PasswordStore::WrapModificationTask, task));
}
-int PasswordStore::GetLogins(const PasswordForm& form,
- PasswordStoreConsumer* consumer) {
- int handle = GetNewRequestHandle();
- GetLoginsRequest* request = new GetLoginsRequest(consumer, handle);
- ScheduleTask(NewRunnableMethod(this, &PasswordStore::GetLoginsImpl, request,
- form));
- return handle;
+CancelableRequestProvider::Handle PasswordStore::GetLogins(
+ const PasswordForm& form, PasswordStoreConsumer* consumer) {
+ return Schedule(&PasswordStore::GetLoginsImpl, consumer, form);
}
-int PasswordStore::GetAutofillableLogins(PasswordStoreConsumer* consumer) {
- int handle = GetNewRequestHandle();
- GetLoginsRequest* request = new GetLoginsRequest(consumer, handle);
- ScheduleTask(NewRunnableMethod(this,
- &PasswordStore::GetAutofillableLoginsImpl,
- request));
- return handle;
+CancelableRequestProvider::Handle PasswordStore::GetAutofillableLogins(
+ PasswordStoreConsumer* consumer) {
+ return Schedule(&PasswordStore::GetAutofillableLoginsImpl, consumer);
}
-int PasswordStore::GetBlacklistLogins(PasswordStoreConsumer* consumer) {
- int handle = GetNewRequestHandle();
- GetLoginsRequest* request = new GetLoginsRequest(consumer, handle);
- ScheduleTask(NewRunnableMethod(this,
- &PasswordStore::GetBlacklistLoginsImpl,
- request));
- return handle;
-}
-
-void PasswordStore::CancelLoginsQuery(int handle) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- pending_requests_.erase(handle);
+CancelableRequestProvider::Handle PasswordStore::GetBlacklistLogins(
+ PasswordStoreConsumer* consumer) {
+ return Schedule(&PasswordStore::GetBlacklistLoginsImpl, consumer);
}
void PasswordStore::ReportMetrics() {
@@ -93,50 +90,34 @@
PasswordStore::~PasswordStore() {}
-PasswordStore::GetLoginsRequest::GetLoginsRequest(
- PasswordStoreConsumer* consumer, int handle)
- : consumer(consumer), handle(handle), message_loop(MessageLoop::current()) {
-}
-
void PasswordStore::ScheduleTask(Task* task) {
BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, task);
}
-
-void PasswordStore::NotifyConsumer(GetLoginsRequest* request,
- const vector<PasswordForm*>& forms) {
- scoped_ptr<GetLoginsRequest> request_ptr(request);
-
-#if !defined(OS_MACOSX)
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-#endif
- request->message_loop->PostTask(
- FROM_HERE,
- NewRunnableMethod(this,
- &PasswordStore::NotifyConsumerImpl,
- request->consumer, request->handle, forms));
+void PasswordStore::ForwardLoginsResult(GetLoginsRequest* request) {
+ request->ForwardResult(GetLoginsRequest::TupleType(request->handle(),
+ request->value));
}
-void PasswordStore::NotifyConsumerImpl(PasswordStoreConsumer* consumer,
- int handle,
- const vector<PasswordForm*>& forms) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- // Don't notify the consumer if the request was canceled.
- if (pending_requests_.find(handle) == pending_requests_.end()) {
- // |forms| is const so we iterate rather than use STLDeleteElements().
- for (size_t i = 0; i < forms.size(); ++i)
- delete forms[i];
- return;
- }
- pending_requests_.erase(handle);
-
- consumer->OnPasswordStoreRequestDone(handle, forms);
+template<typename BackendFunc>
+CancelableRequestProvider::Handle PasswordStore::Schedule(
+ BackendFunc func, PasswordStoreConsumer* consumer) {
+ scoped_refptr<GetLoginsRequest> request(new GetLoginsRequest(
+ NewCallback(consumer,
+ &PasswordStoreConsumer::OnPasswordStoreRequestDone)));
+ AddRequest(request, consumer->cancelable_consumer());
+ ScheduleTask(NewRunnableMethod(this, func, request));
+ return request->handle();
}
-int PasswordStore::GetNewRequestHandle() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- int handle = handle_++;
- pending_requests_.insert(handle);
- return handle;
+template<typename BackendFunc, typename ArgA>
+CancelableRequestProvider::Handle PasswordStore::Schedule(
+ BackendFunc func, PasswordStoreConsumer* consumer, const ArgA& a) {
+ scoped_refptr<GetLoginsRequest> request(new GetLoginsRequest(
+ NewCallback(consumer,
+ &PasswordStoreConsumer::OnPasswordStoreRequestDone)));
+ AddRequest(request, consumer->cancelable_consumer());
+ ScheduleTask(NewRunnableMethod(this, func, request, a));
+ return request->handle();
}
void PasswordStore::WrapModificationTask(Task* task) {
diff --git a/chrome/browser/password_manager/password_store.h b/chrome/browser/password_manager/password_store.h
index 4b8b435..0212790b 100644
--- a/chrome/browser/password_manager/password_store.h
+++ b/chrome/browser/password_manager/password_store.h
@@ -6,16 +6,16 @@
#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_H_
#pragma once
-#include <set>
#include <vector>
+#include "base/callback.h"
#include "base/memory/ref_counted.h"
#include "base/observer_list.h"
#include "base/threading/thread.h"
#include "base/time.h"
-#include "webkit/glue/password_form.h"
+#include "content/browser/cancelable_request.h"
-class Profile;
+class PasswordStoreConsumer;
class Task;
namespace browser_sync {
@@ -24,20 +24,39 @@
class PasswordModelWorker;
};
-class PasswordStoreConsumer {
- public:
- virtual ~PasswordStoreConsumer() {}
- // Call this when the request is finished. If there are no results, call it
- // anyway with an empty vector.
- virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result) = 0;
+namespace webkit_glue {
+struct PasswordForm;
};
// Interface for storing form passwords in a platform-specific secure way.
// The login request/manipulation API is not threadsafe and must be used
// from the UI thread.
-class PasswordStore : public base::RefCountedThreadSafe<PasswordStore> {
+class PasswordStore
+ : public base::RefCountedThreadSafe<PasswordStore>,
+ public CancelableRequestProvider {
public:
+ typedef Callback2<Handle,
+ const std::vector<webkit_glue::PasswordForm*>&>::Type
+ GetLoginsCallback;
+
+ // PasswordForm vector elements are meant to be owned by the
+ // PasswordStoreConsumer. However, if the request is canceled after the
+ // allocation, then the request must take care of the deletion.
+ // TODO(scr) If we can convert vector<PasswordForm*> to
+ // ScopedVector<PasswordForm>, then we can move the following class to merely
+ // a typedef. At the moment, a subclass of CancelableRequest1 is required to
+ // provide a destructor, which cleans up after canceled requests by deleting
+ // vector elements.
+ class GetLoginsRequest : public CancelableRequest1<
+ GetLoginsCallback, std::vector<webkit_glue::PasswordForm*> > {
+ public:
+ explicit GetLoginsRequest(GetLoginsCallback* callback);
+ virtual ~GetLoginsRequest();
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(GetLoginsRequest);
+ };
+
// An interface used to notify clients (observers) of this object that data in
// the password store has changed. Register the observer via
// PasswordStore::SetObserver.
@@ -69,24 +88,21 @@
const base::Time& delete_end);
// Searches for a matching PasswordForm and returns a handle so the async
- // request can be tracked. Implement the PasswordStoreConsumer interface to
- // be notified on completion.
- virtual int GetLogins(const webkit_glue::PasswordForm& form,
- PasswordStoreConsumer* consumer);
+ // request can be tracked. Implement the PasswordStoreConsumer interface to be
+ // notified on completion.
+ virtual Handle GetLogins(const webkit_glue::PasswordForm& form,
+ PasswordStoreConsumer* consumer);
// Gets the complete list of PasswordForms that are not blacklist entries--and
// are thus auto-fillable--and returns a handle so the async request can be
- // tracked. Implement the PasswordStoreConsumer interface to be notified
- // on completion.
- int GetAutofillableLogins(PasswordStoreConsumer* consumer);
+ // tracked. Implement the PasswordStoreConsumer interface to be notified on
+ // completion.
+ Handle GetAutofillableLogins(PasswordStoreConsumer* consumer);
// Gets the complete list of PasswordForms that are blacklist entries, and
// returns a handle so the async request can be tracked. Implement the
// PasswordStoreConsumer interface to be notified on completion.
- int GetBlacklistLogins(PasswordStoreConsumer* consumer);
-
- // Cancels a previous Get*Logins query (async)
- void CancelLoginsQuery(int handle);
+ Handle GetBlacklistLogins(PasswordStoreConsumer* consumer);
// Reports usage metrics for the database.
void ReportMetrics();
@@ -106,25 +122,7 @@
virtual ~PasswordStore();
- // Simple container class that represents a login lookup request.
- class GetLoginsRequest {
- public:
- GetLoginsRequest(PasswordStoreConsumer* c,
- int handle);
-
- // The consumer to notify when this request is complete.
- PasswordStoreConsumer* consumer;
- // A unique handle for the request
- int handle;
- // The message loop that the request was made from. We send the result
- // back to the consumer in this same message loop.
- MessageLoop* message_loop;
-
- private:
- DISALLOW_COPY_AND_ASSIGN(GetLoginsRequest);
- };
-
- // Schedule the given task to be run in the PasswordStore's own thread.
+ // Schedule the given |task| to be run in the PasswordStore's own thread.
virtual void ScheduleTask(Task* task);
// These will be run in PasswordStore's own thread.
@@ -156,22 +154,23 @@
virtual bool FillBlacklistLogins(
std::vector<webkit_glue::PasswordForm*>* forms) = 0;
- // Notifies the consumer that a Get*Logins() request is complete.
- virtual void NotifyConsumer(
- GetLoginsRequest* request,
- const std::vector<webkit_glue::PasswordForm*>& forms);
+ // Dispatches the result to the PasswordStoreConsumer on the original caller's
+ // thread so the callback can be executed there. This should be the UI
+ // thread.
+ virtual void ForwardLoginsResult(GetLoginsRequest* request);
+
+ // Schedule the given |func| to be run in the PasswordStore's own thread with
+ // responses delivered to |consumer| on the current thread.
+ template<typename BackendFunc>
+ Handle Schedule(BackendFunc func, PasswordStoreConsumer* consumer);
+
+ // Schedule the given |func| to be run in the PasswordStore's own thread with
+ // argument |a| and responses delivered to |consumer| on the current thread.
+ template<typename BackendFunc, typename ArgA>
+ Handle Schedule(BackendFunc func, PasswordStoreConsumer* consumer,
+ const ArgA& a);
private:
- // Called by NotifyConsumer, but runs in the consumer's thread. Will not
- // call the consumer if the request was canceled. This extra layer is here so
- // that PasswordStoreConsumer doesn't have to be reference counted (we assume
- // consumers will cancel their requests before they are destroyed).
- void NotifyConsumerImpl(PasswordStoreConsumer* consumer, int handle,
- const std::vector<webkit_glue::PasswordForm*>& forms);
-
- // Returns a new request handle tracked in pending_requests_.
- int GetNewRequestHandle();
-
// Wrapper method called on the destination thread (DB for non-mac) that calls
// the method specified in |task| and then calls back into the source thread
// to notify observers that the password store may have been modified via
@@ -185,14 +184,6 @@
// may have been changed.
void NotifyLoginsChanged();
- // Next handle to return from Get*Logins() to allow callers to track
- // their request.
- int handle_;
-
- // List of pending request handles. Handles are removed from the set when
- // they finish or are canceled.
- std::set<int> pending_requests_;
-
// The observers.
ObserverList<Observer> observers_;
diff --git a/chrome/browser/password_manager/password_store_consumer.cc b/chrome/browser/password_manager/password_store_consumer.cc
new file mode 100644
index 0000000..f0756ec
--- /dev/null
+++ b/chrome/browser/password_manager/password_store_consumer.cc
@@ -0,0 +1,8 @@
+// Copyright (c) 2011 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/password_manager/password_store_consumer.h"
+
+PasswordStoreConsumer::~PasswordStoreConsumer() {
+}
diff --git a/chrome/browser/password_manager/password_store_consumer.h b/chrome/browser/password_manager/password_store_consumer.h
new file mode 100644
index 0000000..ad399d7
--- /dev/null
+++ b/chrome/browser/password_manager/password_store_consumer.h
@@ -0,0 +1,43 @@
+// Copyright (c) 2011 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 CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_CONSUMER_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_CONSUMER_H_
+#pragma once
+
+#include "content/browser/cancelable_request.h"
+
+namespace webkit_glue {
+struct PasswordForm;
+};
+
+// Reads from the PasswordStore are done asynchrously on a separate
+// thread. PasswordStoreConsumer provides the virtual callback method, which is
+// guaranteed to be executed on this (the UI) thread. It also provides the
+// CancelableRequestConsumer member, which cancels any outstanding requests upon
+// destruction.
+class PasswordStoreConsumer {
+ public:
+ // Call this when the request is finished. If there are no results, call it
+ // anyway with an empty vector.
+ virtual void OnPasswordStoreRequestDone(
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) = 0;
+
+ // The CancelableRequest framework needs both a callback (the
+ // PasswordStoreConsumer::OnPasswordStoreRequestDone) as well as a
+ // CancelableRequestConsumer. This accessor makes the API simpler for callers
+ // as PasswordStore can get both from the PasswordStoreConsumer.
+ CancelableRequestConsumerBase* cancelable_consumer() {
+ return &cancelable_consumer_;
+ }
+
+ protected:
+ virtual ~PasswordStoreConsumer();
+
+ private:
+ CancelableRequestConsumer cancelable_consumer_;
+};
+
+#endif // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_CONSUMER_H_
diff --git a/chrome/browser/password_manager/password_store_default.cc b/chrome/browser/password_manager/password_store_default.cc
index c029e2c..2be52076 100644
--- a/chrome/browser/password_manager/password_store_default.cc
+++ b/chrome/browser/password_manager/password_store_default.cc
@@ -93,23 +93,20 @@
void PasswordStoreDefault::GetLoginsImpl(
GetLoginsRequest* request, const webkit_glue::PasswordForm& form) {
- std::vector<PasswordForm*> forms;
- login_db_->GetLogins(form, &forms);
- NotifyConsumer(request, forms);
+ login_db_->GetLogins(form, &request->value);
+ ForwardLoginsResult(request);
}
void PasswordStoreDefault::GetAutofillableLoginsImpl(
GetLoginsRequest* request) {
- std::vector<PasswordForm*> forms;
- FillAutofillableLogins(&forms);
- NotifyConsumer(request, forms);
+ FillAutofillableLogins(&request->value);
+ ForwardLoginsResult(request);
}
void PasswordStoreDefault::GetBlacklistLoginsImpl(
GetLoginsRequest* request) {
- std::vector<PasswordForm*> forms;
- FillBlacklistLogins(&forms);
- NotifyConsumer(request, forms);
+ FillBlacklistLogins(&request->value);
+ ForwardLoginsResult(request);
}
bool PasswordStoreDefault::FillAutofillableLogins(
diff --git a/chrome/browser/password_manager/password_store_default_unittest.cc b/chrome/browser/password_manager/password_store_default_unittest.cc
index 97f67b386..19b70a1 100644
--- a/chrome/browser/password_manager/password_store_default_unittest.cc
+++ b/chrome/browser/password_manager/password_store_default_unittest.cc
@@ -10,6 +10,7 @@
#include "base/utf_string_conversions.h"
#include "base/synchronization/waitable_event.h"
#include "chrome/browser/password_manager/password_store_change.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/password_manager/password_store_default.h"
#include "chrome/browser/password_manager/password_form_data.h"
#include "chrome/browser/prefs/pref_service.h"
@@ -38,7 +39,8 @@
class MockPasswordStoreConsumer : public PasswordStoreConsumer {
public:
MOCK_METHOD2(OnPasswordStoreRequestDone,
- void(int, const std::vector<webkit_glue::PasswordForm*>&));
+ void(CancelableRequestProvider::Handle,
+ const std::vector<webkit_glue::PasswordForm*>&));
};
class MockWebDataServiceConsumer : public WebDataServiceConsumer {
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index 2945032f..ee1222c 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -886,7 +886,7 @@
std::vector<PasswordForm*> database_forms;
login_metadata_db_->GetLogins(form, &database_forms);
- std::vector<PasswordForm*> merged_forms;
+ std::vector<PasswordForm*>& merged_forms = request->value;
internal_keychain_helpers::MergePasswordForms(&keychain_forms,
&database_forms,
&merged_forms);
@@ -904,19 +904,17 @@
RemoveDatabaseForms(database_forms);
STLDeleteElements(&database_forms);
- NotifyConsumer(request, merged_forms);
+ ForwardLoginsResult(request);
}
void PasswordStoreMac::GetBlacklistLoginsImpl(GetLoginsRequest* request) {
- std::vector<PasswordForm*> database_forms;
- FillBlacklistLogins(&database_forms);
- NotifyConsumer(request, database_forms);
+ FillBlacklistLogins(&request->value);
+ ForwardLoginsResult(request);
}
void PasswordStoreMac::GetAutofillableLoginsImpl(GetLoginsRequest* request) {
- std::vector<PasswordForm*> database_forms;
- FillAutofillableLogins(&database_forms);
- NotifyConsumer(request, database_forms);
+ FillAutofillableLogins(&request->value);
+ ForwardLoginsResult(request);
}
bool PasswordStoreMac::FillAutofillableLogins(
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc
index 1d2690a..3deabb3 100644
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc
+++ b/chrome/browser/password_manager/password_store_mac_unittest.cc
@@ -14,6 +14,7 @@
#include "base/utf_string_conversions.h"
#include "content/browser/browser_thread.h"
#include "chrome/browser/keychain_mock_mac.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/password_manager/password_store_mac.h"
#include "chrome/browser/password_manager/password_store_mac_internal.h"
#include "chrome/common/chrome_paths.h"
@@ -28,7 +29,8 @@
class MockPasswordStoreConsumer : public PasswordStoreConsumer {
public:
MOCK_METHOD2(OnPasswordStoreRequestDone,
- void(int, const std::vector<webkit_glue::PasswordForm*>&));
+ void(CancelableRequestProvider::Handle,
+ const std::vector<webkit_glue::PasswordForm*>&));
};
ACTION(STLDeleteElements0) {
diff --git a/chrome/browser/password_manager/password_store_win.cc b/chrome/browser/password_manager/password_store_win.cc
index 350ce8c2..9adf0ab8 100644
--- a/chrome/browser/password_manager/password_store_win.cc
+++ b/chrome/browser/password_manager/password_store_win.cc
@@ -36,14 +36,13 @@
return request_handle;
}
-void PasswordStoreWin::NotifyConsumer(GetLoginsRequest* request,
- const std::vector<PasswordForm*> forms) {
- if (!forms.empty()) {
- pending_request_forms_.erase(request->handle);
- PasswordStore::NotifyConsumer(request, forms);
+void PasswordStoreWin::ForwardLoginsResult(GetLoginsRequest* request) {
+ if (!request->value.empty()) {
+ pending_request_forms_.erase(request->handle());
+ PasswordStore::ForwardLoginsResult(request);
} else {
PendingRequestFormMap::iterator it(pending_request_forms_.find(
- request->handle));
+ request->handle()));
if (it != pending_request_forms_.end()) {
IE7PasswordInfo info;
std::wstring url = ASCIIToWide(it->second.origin.spec());
@@ -69,16 +68,15 @@
// This is a response from WebDataService::GetIE7Login.
PendingRequestFormMap::iterator it(pending_request_forms_.find(
- request->handle));
+ request->handle()));
DCHECK(pending_request_forms_.end() != it);
PasswordForm* ie7_form = GetIE7Result(result, it->second);
- std::vector<PasswordForm*> forms;
if (ie7_form)
- forms.push_back(ie7_form);
+ request->value.push_back(ie7_form);
pending_request_forms_.erase(it);
- PasswordStore::NotifyConsumer(request.release(), forms);
+ PasswordStore::ForwardLoginsResult(request.release());
} else {
PasswordStoreDefault::OnWebDataServiceRequestDone(handle, result);
}
diff --git a/chrome/browser/password_manager/password_store_win.h b/chrome/browser/password_manager/password_store_win.h
index e58f717..cdde4a2 100644
--- a/chrome/browser/password_manager/password_store_win.h
+++ b/chrome/browser/password_manager/password_store_win.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -34,9 +34,7 @@
void OnWebDataServiceRequestDone(WebDataService::Handle h,
const WDTypedResult* result);
- virtual void NotifyConsumer(
- GetLoginsRequest* request,
- const std::vector<webkit_glue::PasswordForm*> forms);
+ virtual void ForwardLoginsResult(GetLoginsRequest* request);
// Takes ownership of |request| and tracks it under |handle|.
void TrackRequest(WebDataService::Handle handle, GetLoginsRequest* request);
diff --git a/chrome/browser/password_manager/password_store_win_unittest.cc b/chrome/browser/password_manager/password_store_win_unittest.cc
index 42e7dc42..4031e7f 100644
--- a/chrome/browser/password_manager/password_store_win_unittest.cc
+++ b/chrome/browser/password_manager/password_store_win_unittest.cc
@@ -14,6 +14,7 @@
#include "base/time.h"
#include "base/synchronization/waitable_event.h"
#include "chrome/browser/password_manager/password_form_data.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/password_manager/password_store_win.h"
#include "chrome/browser/password_manager/ie7_password.h"
#include "chrome/browser/prefs/pref_service.h"
@@ -35,7 +36,8 @@
class MockPasswordStoreConsumer : public PasswordStoreConsumer {
public:
MOCK_METHOD2(OnPasswordStoreRequestDone,
- void(int, const std::vector<webkit_glue::PasswordForm*>&));
+ void(CancelableRequestProvider::Handle,
+ const std::vector<webkit_glue::PasswordForm*>&));
};
class MockWebDataServiceConsumer : public WebDataServiceConsumer {
diff --git a/chrome/browser/password_manager/password_store_x.cc b/chrome/browser/password_manager/password_store_x.cc
index 13ab8fb..7c3559f 100644
--- a/chrome/browser/password_manager/password_store_x.cc
+++ b/chrome/browser/password_manager/password_store_x.cc
@@ -99,45 +99,44 @@
}
void PasswordStoreX::GetLoginsImpl(GetLoginsRequest* request,
- const PasswordForm& form) {
+ const PasswordForm& form) {
CheckMigration();
- vector<PasswordForm*> forms;
- if (use_native_backend() && backend_->GetLogins(form, &forms)) {
- NotifyConsumer(request, forms);
+ if (use_native_backend() && backend_->GetLogins(form, &request->value)) {
+ ForwardLoginsResult(request);
allow_fallback_ = false;
} else if (allow_default_store()) {
PasswordStoreDefault::GetLoginsImpl(request, form);
} else {
// The consumer will be left hanging unless we reply.
- NotifyConsumer(request, forms);
+ ForwardLoginsResult(request);
}
}
void PasswordStoreX::GetAutofillableLoginsImpl(GetLoginsRequest* request) {
CheckMigration();
- vector<PasswordForm*> forms;
- if (use_native_backend() && backend_->GetAutofillableLogins(&forms)) {
- NotifyConsumer(request, forms);
+ if (use_native_backend() &&
+ backend_->GetAutofillableLogins(&request->value)) {
+ ForwardLoginsResult(request);
allow_fallback_ = false;
} else if (allow_default_store()) {
PasswordStoreDefault::GetAutofillableLoginsImpl(request);
} else {
// The consumer will be left hanging unless we reply.
- NotifyConsumer(request, forms);
+ ForwardLoginsResult(request);
}
}
void PasswordStoreX::GetBlacklistLoginsImpl(GetLoginsRequest* request) {
CheckMigration();
- vector<PasswordForm*> forms;
- if (use_native_backend() && backend_->GetBlacklistLogins(&forms)) {
- NotifyConsumer(request, forms);
+ if (use_native_backend() &&
+ backend_->GetBlacklistLogins(&request->value)) {
+ ForwardLoginsResult(request);
allow_fallback_ = false;
} else if (allow_default_store()) {
PasswordStoreDefault::GetBlacklistLoginsImpl(request);
} else {
// The consumer will be left hanging unless we reply.
- NotifyConsumer(request, forms);
+ ForwardLoginsResult(request);
}
}
diff --git a/chrome/browser/password_manager/password_store_x_unittest.cc b/chrome/browser/password_manager/password_store_x_unittest.cc
index df0778b..80eaa6a 100644
--- a/chrome/browser/password_manager/password_store_x_unittest.cc
+++ b/chrome/browser/password_manager/password_store_x_unittest.cc
@@ -36,7 +36,8 @@
class MockPasswordStoreConsumer : public PasswordStoreConsumer {
public:
MOCK_METHOD2(OnPasswordStoreRequestDone,
- void(int, const std::vector<PasswordForm*>&));
+ void(CancelableRequestProvider::Handle,
+ const std::vector<PasswordForm*>&));
};
class MockWebDataServiceConsumer : public WebDataServiceConsumer {
diff --git a/chrome/browser/ui/webui/options/password_manager_handler.cc b/chrome/browser/ui/webui/options/password_manager_handler.cc
index 850718de..463c6aa 100644
--- a/chrome/browser/ui/webui/options/password_manager_handler.cc
+++ b/chrome/browser/ui/webui/options/password_manager_handler.cc
@@ -5,7 +5,6 @@
#include "chrome/browser/ui/webui/options/password_manager_handler.h"
#include "base/callback.h"
-#include "base/stl_util-inl.h"
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
@@ -17,6 +16,7 @@
#include "grit/generated_resources.h"
#include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util.h"
+#include "webkit/glue/password_form.h"
PasswordManagerHandler::PasswordManagerHandler()
: ALLOW_THIS_IN_INITIALIZER_LIST(populater_(this)),
@@ -24,9 +24,6 @@
}
PasswordManagerHandler::~PasswordManagerHandler() {
- // TODO(scr): ScopedVector.
- STLDeleteElements(&password_list_);
- STLDeleteElements(&password_exception_list_);
GetPasswordStore()->RemoveObserver(this);
}
@@ -90,8 +87,8 @@
void PasswordManagerHandler::UpdatePasswordLists(const ListValue* args) {
// Reset the current lists.
- STLDeleteElements(&password_list_);
- STLDeleteElements(&password_exception_list_);
+ password_list_.reset();
+ password_exception_list_.reset();
languages_ =
web_ui_->GetProfile()->GetPrefs()->GetString(prefs::kAcceptLanguages);
@@ -158,14 +155,12 @@
}
PasswordManagerHandler::ListPopulater::ListPopulater(
- PasswordManagerHandler* page) : page_(page),
- pending_login_query_(0) {
+ PasswordManagerHandler* page)
+ : page_(page),
+ pending_login_query_(0) {
}
PasswordManagerHandler::ListPopulater::~ListPopulater() {
- PasswordStore* store = page_->GetPasswordStore();
- if (store)
- store->CancelLoginsQuery(pending_login_query_);
}
PasswordManagerHandler::PasswordListPopulater::PasswordListPopulater(
@@ -173,20 +168,26 @@
}
void PasswordManagerHandler::PasswordListPopulater::Populate() {
- DCHECK(!pending_login_query_);
PasswordStore* store = page_->GetPasswordStore();
- if (store != NULL)
+ if (store != NULL) {
+ if (pending_login_query_)
+ store->CancelRequest(pending_login_query_);
+
pending_login_query_ = store->GetAutofillableLogins(this);
- else
+ } else {
LOG(ERROR) << "No password store! Cannot display passwords.";
+ }
}
void PasswordManagerHandler::PasswordListPopulater::
- OnPasswordStoreRequestDone(int handle,
- const std::vector<webkit_glue::PasswordForm*>& result) {
+ OnPasswordStoreRequestDone(
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) {
DCHECK_EQ(pending_login_query_, handle);
pending_login_query_ = 0;
- page_->password_list_ = result;
+ page_->password_list_.reset();
+ page_->password_list_.insert(page_->password_list_.end(),
+ result.begin(), result.end());
page_->SetPasswordList();
}
@@ -196,19 +197,25 @@
}
void PasswordManagerHandler::PasswordExceptionListPopulater::Populate() {
- DCHECK(!pending_login_query_);
PasswordStore* store = page_->GetPasswordStore();
- if (store != NULL)
+ if (store != NULL) {
+ if (pending_login_query_)
+ store->CancelRequest(pending_login_query_);
+
pending_login_query_ = store->GetBlacklistLogins(this);
- else
+ } else {
LOG(ERROR) << "No password store! Cannot display exceptions.";
+ }
}
void PasswordManagerHandler::PasswordExceptionListPopulater::
- OnPasswordStoreRequestDone(int handle,
- const std::vector<webkit_glue::PasswordForm*>& result) {
+ OnPasswordStoreRequestDone(
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) {
DCHECK_EQ(pending_login_query_, handle);
pending_login_query_ = 0;
- page_->password_exception_list_ = result;
+ page_->password_exception_list_.reset();
+ page_->password_exception_list_.insert(page_->password_exception_list_.end(),
+ result.begin(), result.end());
page_->SetPasswordExceptionList();
}
diff --git a/chrome/browser/ui/webui/options/password_manager_handler.h b/chrome/browser/ui/webui/options/password_manager_handler.h
index 83b729dc..b316a757 100644
--- a/chrome/browser/ui/webui/options/password_manager_handler.h
+++ b/chrome/browser/ui/webui/options/password_manager_handler.h
@@ -8,7 +8,9 @@
#include <string>
#include <vector>
+#include "base/memory/scoped_vector.h"
#include "chrome/browser/password_manager/password_store.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/ui/webui/options/options_ui.h"
class PasswordManagerHandler : public OptionsPageUIHandler,
@@ -66,11 +68,12 @@
// Send the password store's reply back to the handler.
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result) = 0;
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) = 0;
protected:
PasswordManagerHandler* page_;
- int pending_login_query_;
+ CancelableRequestProvider::Handle pending_login_query_;
};
// A short class to mediate requests to the password store for passwordlist.
@@ -83,7 +86,8 @@
// Send the password store's reply back to the handler.
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result);
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result);
};
// A short class to mediate requests to the password store for exceptions.
@@ -96,15 +100,16 @@
// Send the password store's reply back to the handler.
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result);
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result);
};
// Password store consumer for populating the password list and exceptions.
PasswordListPopulater populater_;
PasswordExceptionListPopulater exception_populater_;
- std::vector<webkit_glue::PasswordForm*> password_list_;
- std::vector<webkit_glue::PasswordForm*> password_exception_list_;
+ ScopedVector<webkit_glue::PasswordForm> password_list_;
+ ScopedVector<webkit_glue::PasswordForm> password_exception_list_;
// User's pref
std::string languages_;
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 31cfa033..d5000e7 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1422,6 +1422,8 @@
'browser/password_manager/password_manager.h',
'browser/password_manager/password_store.cc',
'browser/password_manager/password_store.h',
+ 'browser/password_manager/password_store_consumer.cc',
+ 'browser/password_manager/password_store_consumer.h',
'browser/password_manager/password_store_default.cc',
'browser/password_manager/password_store_default.h',
'browser/password_manager/password_store_mac.cc',
diff --git a/chrome/test/live_sync/live_passwords_sync_test.h b/chrome/test/live_sync/live_passwords_sync_test.h
index 81fe215..6663f9fe 100644
--- a/chrome/test/live_sync/live_passwords_sync_test.h
+++ b/chrome/test/live_sync/live_passwords_sync_test.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -9,6 +9,7 @@
#include <set>
#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/password_manager/password_store_consumer.h"
#include "chrome/browser/password_manager/password_store.h"
#include "chrome/test/live_sync/live_sync_test.h"
#include "chrome/test/ui_test_utils.h"
@@ -64,7 +65,8 @@
: PasswordStoreConsumer(), result_(result) {}
virtual void OnPasswordStoreRequestDone(
- int handle, const std::vector<webkit_glue::PasswordForm*>& result) {
+ CancelableRequestProvider::Handle handle,
+ const std::vector<webkit_glue::PasswordForm*>& result) {
result_.clear();
for (std::vector<webkit_glue::PasswordForm*>::const_iterator it =
result.begin(); it != result.end(); ++it) {
diff --git a/content/browser/cancelable_request.cc b/content/browser/cancelable_request.cc
index f1826fd..5e36d89 100644
--- a/content/browser/cancelable_request.cc
+++ b/content/browser/cancelable_request.cc
@@ -4,7 +4,8 @@
#include "content/browser/cancelable_request.h"
-CancelableRequestProvider::CancelableRequestProvider() : next_handle_(1) {
+CancelableRequestProvider::CancelableRequestProvider()
+ : next_handle_(1) {
}
CancelableRequestProvider::~CancelableRequestProvider() {
@@ -28,6 +29,8 @@
handle = next_handle_;
pending_requests_[next_handle_] = request;
++next_handle_;
+ DCHECK(next_handle_)
+ << "next_handle_ may have wrapped around to invalid state.";
}
consumer->OnRequestAdded(this, handle);
