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 7fdd4390e88c1d4f96b8a21355260f3d8cba1ec4..bbcc6a31947d441250bf310775eefa23ad49b570 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"
@@ -114,7 +113,7 @@ void PasswordManagerHandler::RemoveAllSavedPasswords(
   PasswordStore* store = GetPasswordStore();
   for (size_t i = 0; i < password_list_.size(); ++i)
     store->RemoveLogin(*password_list_[i]);
-  STLDeleteElements(&password_list_);
+  password_list_.reset();
   SetPasswordList();
 }
 
@@ -123,7 +122,7 @@ void PasswordManagerHandler::RemoveAllPasswordExceptions(
   PasswordStore* store = GetPasswordStore();
   for (size_t i = 0; i < password_exception_list_.size(); ++i)
     store->RemoveLogin(*password_exception_list_[i]);
-  STLDeleteElements(&password_exception_list_);
+  password_exception_list_.reset();
   SetPasswordExceptionList();
 }
 
@@ -158,31 +157,29 @@ PasswordManagerHandler::ListPopulater::ListPopulater(
                                     pending_login_query_(0) {
 }
 
-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_)
+      store->CancelRequest(pending_login_query_);

[stuartmorgan, 2011/03/16 23:25:20]

Why the change from DCHECK to handling gracefully?

[Sheridan Rawlins, 2011/03/18 15:45:48]

My thought was that Populate should just "make it so" rather than requiring the
client to need to cancel first and then call.  I can change the commenting in
header if necessary - do you agree with this change, with explanation?

+
     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(result);
   page_->SetPasswordList();
 }
 
@@ -192,19 +189,23 @@ 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(result);
   page_->SetPasswordExceptionList();
 }