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

chrome/browser/password_manager/password_form_manager.cc

 // 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"
 #include "chrome/browser/password_manager/password_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "webkit/glue/password_form_dom_manager.h"
 
 using base::Time;
 using webkit_glue::PasswordForm;
 using webkit_glue::PasswordFormMap;
 
 PasswordFormManager::PasswordFormManager(Profile* profile,
                                          PasswordManager* password_manager,
                                          const PasswordForm& observed_form,
                                          bool ssl_valid)
     : best_matches_deleter_(&best_matches_),
       observed_form_(observed_form),
       is_new_login_(true),
       password_manager_(password_manager),
-      pending_login_query_(0),
+      pending_login_query_(PasswordStore::kInvalidHandle),
       preferred_match_(NULL),
       state_(PRE_MATCHING_PHASE),
       profile_(profile),
       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 137 matching lines @@
   DCHECK(!profile_->IsOffTheRecord());
 
   if (IsNewLogin())
     SaveAsNewLogin(true);
   else
     UpdateLogin();
 }
 
 void PasswordFormManager::FetchMatchingLoginsFromPasswordStore() {
   DCHECK_EQ(state_, PRE_MATCHING_PHASE);
-  DCHECK(!pending_login_query_);
+  DCHECK(!PasswordStore::handle_is_valid(pending_login_query_));
   state_ = MATCHING_PHASE;
   PasswordStore* password_store =
       profile_->GetPasswordStore(Profile::EXPLICIT_ACCESS);
   if (!password_store) {
     NOTREACHED();
     return;
   }
   pending_login_query_ = password_store->GetLogins(observed_form_, this);
 }
 
@@ skipping 88 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::kInvalidHandle;
 }
 
 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;
 }