Fix DCHECK, memory leak, and refactor PasswordStore to use CancelableRequest

chrome/browser/ui/webui/options/password_manager_handler.cc

Index: chrome/browser/ui/webui/options/password_manager_handler.cc
diff --git a/chrome/browser/ui/webui/options/password_manager_handler.cc b/chrome/browser/ui/webui/options/password_manager_handler.cc
index 850718dec08270479983d3deb540aa0a92826ab2..d97e66cc900825093d435070e52088629073cfad 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"
@@ -24,9 +23,6 @@ PasswordManagerHandler::PasswordManagerHandler()
 }
 
 PasswordManagerHandler::~PasswordManagerHandler() {
-  // TODO(scr): ScopedVector.
-  STLDeleteElements(&password_list_);
-  STLDeleteElements(&password_exception_list_);
   GetPasswordStore()->RemoveObserver(this);
 }
 
@@ -90,8 +86,8 @@ PasswordStore* PasswordManagerHandler::GetPasswordStore() {
 
 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);
@@ -157,36 +153,39 @@ void PasswordManagerHandler::SetPasswordExceptionList() {
                                   entries);
 }
 
+PasswordManagerHandler::ListPopulater::~ListPopulater() {

[James Hawkins, 2011/03/21 01:45:06]

Source ordering: ~ListPopulater() implementation should go after the
ListPopulater() implementation.

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+}
+
 PasswordManagerHandler::ListPopulater::ListPopulater(
     PasswordManagerHandler* page) : page_(page),
                                     pending_login_query_(0) {

[James Hawkins, 2011/03/21 01:45:06]

|pending_login_query_| should be a PasswordStore::Handle, and that should be
opaque, so we're going to need another way to initialize this.

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

 }
 
-PasswordManagerHandler::ListPopulater::~ListPopulater() {
-  PasswordStore* store = page_->GetPasswordStore();
-  if (store)
-    store->CancelLoginsQuery(pending_login_query_);
-}
-
 PasswordManagerHandler::PasswordListPopulater::PasswordListPopulater(
     PasswordManagerHandler* page) : ListPopulater(page) {
 }
 
 void PasswordManagerHandler::PasswordListPopulater::Populate() {
-  DCHECK(!pending_login_query_);
   PasswordStore* store = page_->GetPasswordStore();
-  if (store != NULL)
+  if (store != NULL) {
+    if (pending_login_query_)

[James Hawkins, 2011/03/21 01:45:06]

Isn't PasswordStore::Handle opaque?  We're assuming an unset Handle is the value
0/false.

[Sheridan Rawlins, 2011/03/21 05:20:17]

Done.

+      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(
+        PasswordStore::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 +195,25 @@ PasswordManagerHandler::PasswordExceptionListPopulater::
 }
 
 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(
+        PasswordStore::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();
 }