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

chrome/browser/password_manager/password_form_manager.cc

-// 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.
 
 #include "chrome/browser/password_manager/password_form_manager.h"
 
 #include <algorithm>
 
 #include "base/metrics/histogram.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
@@ skipping 20 matching lines @@
       manager_action_(kManagerActionNone),
       user_action_(kUserActionNone),
       submit_result_(kSubmitResultNotSubmitted) {
   DCHECK(profile_);
   if (observed_form_.origin.is_valid())
     base::SplitString(observed_form_.origin.path(), '/', &form_path_tokens_);
   observed_form_.ssl_valid = ssl_valid;
 }
 
 PasswordFormManager::~PasswordFormManager() {
-  CancelLoginsQuery();
   UMA_HISTOGRAM_ENUMERATION("PasswordManager.ActionsTaken",
                             GetActionsTaken(),
                             kMaxNumActionsTaken);
 }
 
 int PasswordFormManager::GetActionsTaken() {
   return user_action_ + kUserActionMax * (manager_action_ +
          kManagerActionMax * submit_result_);
 };
 
@@ skipping 246 matching lines @@
                            preferred_match_->action.GetWithEmptyPath();
   if (wait_for_username)
     manager_action_ = kManagerActionNone;
   else
     manager_action_ = kManagerActionAutoFilled;
   password_manager_->Autofill(observed_form_, best_matches_,
                               preferred_match_, wait_for_username);
 }
 
 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);
 
   if (result.empty()) {
     state_ = POST_MATCHING_PHASE;
     return;
   }
 
   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 {
   // Ignore change password forms until we have some change password
   // functionality
   if (observed_form_.old_password_element.length() != 0) {
     return true;
   }
   // Don't match an invalid SSL form with one saved under secure
   // circumstances.
@@ skipping 84 matching lines @@
     // TODO(timsteele): Bug 1188626 - expire the master copies.
     PasswordForm copy(pending_credentials_);
     copy.origin = observed_form_.origin;
     copy.action = observed_form_.action;
     password_store->AddLogin(copy);
   } else {
     password_store->UpdateLogin(pending_credentials_);
   }
 }
 
-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:
   // The most important element that should match is the origin, followed by
   // the action, the password name, the submit button name, and finally the
   // username input field name.
   // Exact origin match gives an addition of 32 (1 << 5) + # of matching url
   // dirs.
   // Partial match gives an addition of 16 (1 << 4) + # matching url dirs
   // That way, a partial match cannot trump an exact match even if
@@ skipping 36 matching lines @@
   return score;
 }
 
 void PasswordFormManager::SubmitPassed() {
   submit_result_ = kSubmitResultPassed;
 }
 
 void PasswordFormManager::SubmitFailed() {
   submit_result_ = kSubmitResultFailed;
 }