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

chrome/browser/password_manager/password_form_manager.cc

Index: chrome/browser/password_manager/password_form_manager.cc
diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc
index f5c0f840779eda8b788e33b7ae0379b01a64366d..5a7ca5cf24503279c18e45af99719115421097de 100644
--- a/chrome/browser/password_manager/password_form_manager.cc
+++ b/chrome/browser/password_manager/password_form_manager.cc
@@ -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.
 
@@ -39,7 +39,6 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
 }
 
 PasswordFormManager::~PasswordFormManager() {
-  CancelLoginsQuery();
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
                             GetActionsTaken(),
                             kMaxNumActionsTaken);
@@ -306,7 +305,8 @@ void PasswordFormManager::OnRequestDone(int handle,
 }
 
 void PasswordFormManager::OnPasswordStoreRequestDone(
-    int handle, const std::vector<PasswordForm*>& result) {
+    PasswordStore::Handle handle,
+    const std::vector<PasswordForm*>& result) {
   DCHECK_EQ(state_, MATCHING_PHASE);
   DCHECK_EQ(pending_login_query_, handle);
 
@@ -316,6 +316,7 @@ void PasswordFormManager::OnPasswordStoreRequestDone(
   }
 
   OnRequestDone(handle, result);
+  pending_login_query_ = PasswordStore::Handle(0);

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

You're ignoring the fact that PasswordStore::Handle is opaque and implying that
the value '0' means something.

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

Stuart said something similar before.  I think this solution is better - I
provided a handle_is_valid static method for checking and a kInvalidHandle
constant for initialization.

 }
 
 bool PasswordFormManager::IgnoreResult(const PasswordForm& form) const {
@@ -420,16 +421,6 @@ void PasswordFormManager::UpdateLogin() {
   }
 }
 
-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: