Merge 83999 - Attempt at fixing crash in PasswordStoreWin. It seems like there are

two problems with this class:
. PasswordStoreDefault doesn't cancel requests in its destructor.
. PasswordStoreWin maintains the set of GetLoginsRequests in a
  map. PasswordStoreWin doesn't own the requests, so its entirely
  possible for the request to be deleted out from under it.

These are just guesses though at whats causing the crash.

BUG=79498
TEST=none
R=scr@chromium.org

Review URL: http://codereview.chromium.org/6893042

TBR=sky@chromium.org
Review URL: http://codereview.chromium.org/6995158

git-svn-id: svn://svn.chromium.org/chrome/branches/742/src@88845 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 282 insertions, 151 deletions

diff --git a/chrome/browser/password_manager/password_manager_unittest.cc b/chrome/browser/password_manager/password_manager_unittest.cc
index 0db4973..a22d81b 100644
--- a/chrome/browser/password_manager/password_manager_unittest.cc
+++ b/chrome/browser/password_manager/password_manager_unittest.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.
 
@@ -38,11 +38,14 @@ class TestingProfileWithPasswordStore : public TestingProfile {
  public:
   explicit TestingProfileWithPasswordStore(PasswordStore* store)
       : store_(store) {}
+  virtual ~TestingProfileWithPasswordStore() {
+    store_->Shutdown();
+  }
   virtual PasswordStore* GetPasswordStore(ServiceAccessType access) {
     return store_;
   }
  private:
-  PasswordStore* store_;
+  scoped_refptr<PasswordStore> store_;
 };
 
 class MockPasswordStore : public PasswordStore {
diff --git a/chrome/browser/password_manager/password_store.cc b/chrome/browser/password_manager/password_store.cc
index 4d00b23..09ed969 100644
--- a/chrome/browser/password_manager/password_store.cc
+++ b/chrome/browser/password_manager/password_store.cc
@@ -34,6 +34,9 @@ bool PasswordStore::Init() {
   return true;
 }
 
+void PasswordStore::Shutdown() {
+}
+
 void PasswordStore::AddLogin(const PasswordForm& form) {
   Task* task = NewRunnableMethod(this, &PasswordStore::AddLoginImpl, form);
   ScheduleTask(
@@ -98,6 +101,7 @@ PasswordStore::GetLoginsRequest* PasswordStore::NewGetLoginsRequest(
 void PasswordStore::ScheduleTask(Task* task) {
   BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, task);
 }
+
 void PasswordStore::ForwardLoginsResult(GetLoginsRequest* request) {
   request->ForwardResult(GetLoginsRequest::TupleType(request->handle(),
                                                      request->value));
diff --git a/chrome/browser/password_manager/password_store.h b/chrome/browser/password_manager/password_store.h
index 75a3e80..1e7f69d 100644
--- a/chrome/browser/password_manager/password_store.h
+++ b/chrome/browser/password_manager/password_store.h
@@ -74,6 +74,9 @@ class PasswordStore
   // Reimplement this to add custom initialization. Always call this too.
   virtual bool Init();
 
+  // Invoked from the profiles destructor to shutdown the PasswordStore.
+  virtual void Shutdown();
+
   // Adds the given PasswordForm to the secure password store asynchronously.
   virtual void AddLogin(const webkit_glue::PasswordForm& form);
 
diff --git a/chrome/browser/password_manager/password_store_default.cc b/chrome/browser/password_manager/password_store_default.cc
index 2be5207..d99ba4a 100644
--- a/chrome/browser/password_manager/password_store_default.cc
+++ b/chrome/browser/password_manager/password_store_default.cc
@@ -4,7 +4,7 @@
 
 #include "chrome/browser/password_manager/password_store_default.h"
 
-#include <vector>
+#include <set>
 
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
@@ -19,6 +19,85 @@
 
 using webkit_glue::PasswordForm;
 
+// MigrateHelper handles migration from WebDB to PasswordStore. It runs
+// entirely on the UI thread and is owned by PasswordStoreDefault.
+class PasswordStoreDefault::MigrateHelper : public WebDataServiceConsumer {
+ public:
+  MigrateHelper(Profile* profile,
+                WebDataService* web_data_service,
+                PasswordStore* password_store)
+      : profile_(profile),
+        web_data_service_(web_data_service),
+        password_store_(password_store) {
+  }
+  ~MigrateHelper();
+
+  void Init();
+
+  // WebDataServiceConsumer:
+  virtual void OnWebDataServiceRequestDone(
+      WebDataService::Handle handle,
+      const WDTypedResult *result) OVERRIDE;
+
+ private:
+  typedef std::set<WebDataService::Handle> Handles;
+
+  Profile* profile_;
+
+  scoped_refptr<WebDataService> web_data_service_;
+
+  // This creates a cycle between us and PasswordStore. The cycle is broken
+  // from PasswordStoreDefault::Shutdown, which deletes us.
+  scoped_refptr<PasswordStore> password_store_;
+
+  // Set of handles from requesting data from the WebDB.
+  Handles handles_;
+
+  DISALLOW_COPY_AND_ASSIGN(MigrateHelper);
+};
+
+PasswordStoreDefault::MigrateHelper::~MigrateHelper() {
+  for (Handles::const_iterator i = handles_.begin(); i != handles_.end(); ++i)
+    web_data_service_->CancelRequest(*i);
+  handles_.clear();
+}
+
+void PasswordStoreDefault::MigrateHelper::Init() {
+  handles_.insert(web_data_service_->GetAutofillableLogins(this));
+  handles_.insert(web_data_service_->GetBlacklistLogins(this));
+}
+
+void PasswordStoreDefault::MigrateHelper::OnWebDataServiceRequestDone(
+    WebDataService::Handle handle,
+    const WDTypedResult* result) {
+  typedef std::vector<const PasswordForm*> PasswordForms;
+
+  DCHECK(handles_.end() != handles_.find(handle));
+  DCHECK(password_store_);
+
+  handles_.erase(handle);
+  if (!result)
+    return;
+
+  if (PASSWORD_RESULT != result->GetType()) {
+    NOTREACHED();
+    return;
+  }
+
+  const PasswordForms& forms =
+      static_cast<const WDResult<PasswordForms>*>(result)->GetValue();
+  for (PasswordForms::const_iterator it = forms.begin();
+       it != forms.end(); ++it) {
+    password_store_->AddLogin(**it);
+    web_data_service_->RemoveLogin(**it);
+    delete *it;
+  }
+  if (handles_.empty()) {
+    profile_->GetPrefs()->RegisterBooleanPref(prefs::kLoginDatabaseMigrated,
+                                              true);
+  }
+}
+
 PasswordStoreDefault::PasswordStoreDefault(LoginDatabase* login_db,
                                            Profile* profile,
                                            WebDataService* web_data_service)
@@ -31,6 +110,13 @@ PasswordStoreDefault::PasswordStoreDefault(LoginDatabase* login_db,
 }
 
 PasswordStoreDefault::~PasswordStoreDefault() {
+  // MigrateHelper should always be NULL as Shutdown should be invoked before
+  // the destructor.
+  DCHECK(!migrate_helper_.get());
+}
+
+void PasswordStoreDefault::Shutdown() {
+  migrate_helper_.reset();
 }
 
 void PasswordStoreDefault::ReportMetricsImpl() {
@@ -125,36 +211,7 @@ void PasswordStoreDefault::MigrateIfNecessary() {
   PrefService* prefs = profile_->GetPrefs();
   if (prefs->FindPreference(prefs::kLoginDatabaseMigrated))
     return;
-  handles_.insert(web_data_service_->GetAutofillableLogins(this));
-  handles_.insert(web_data_service_->GetBlacklistLogins(this));
-}
-
-typedef std::vector<const PasswordForm*> PasswordForms;
-
-void PasswordStoreDefault::OnWebDataServiceRequestDone(
-    WebDataService::Handle handle,
-    const WDTypedResult* result) {
-  DCHECK(handles_.end() != handles_.find(handle));
-
-  handles_.erase(handle);
-  if (!result)
-    return;
-
-  if (PASSWORD_RESULT != result->GetType()) {
-    NOTREACHED();
-    return;
-  }
-
-  const PasswordForms& forms =
-      static_cast<const WDResult<PasswordForms>*>(result)->GetValue();
-  for (PasswordForms::const_iterator it = forms.begin();
-       it != forms.end(); ++it) {
-    AddLogin(**it);
-    web_data_service_->RemoveLogin(**it);
-    delete *it;
-  }
-  if (handles_.empty()) {
-    profile_->GetPrefs()->RegisterBooleanPref(prefs::kLoginDatabaseMigrated,
-                                              true);
-  }
+  DCHECK(!migrate_helper_.get());
+  migrate_helper_.reset(new MigrateHelper(profile_, web_data_service_, this));
+  migrate_helper_->Init();
 }
diff --git a/chrome/browser/password_manager/password_store_default.h b/chrome/browser/password_manager/password_store_default.h
index c819b14..4c6163a 100644
--- a/chrome/browser/password_manager/password_store_default.h
+++ b/chrome/browser/password_manager/password_store_default.h
@@ -6,21 +6,19 @@
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_DEFAULT_H_
 #pragma once
 
-#include <set>
 #include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/password_manager/login_database.h"
 #include "chrome/browser/password_manager/password_store.h"
-#include "chrome/browser/webdata/web_data_service.h"
 
 class Profile;
+class WebDataService;
 
 // Simple password store implementation that delegates everything to
 // the LoginDatabase.
-class PasswordStoreDefault : public PasswordStore,
-                             public WebDataServiceConsumer {
+class PasswordStoreDefault : public PasswordStore {
  public:
   // Takes ownership of |login_db|.
   PasswordStoreDefault(LoginDatabase* login_db,
@@ -31,6 +29,7 @@ class PasswordStoreDefault : public PasswordStore,
   virtual ~PasswordStoreDefault();
 
   // Implements PasswordStore interface.
+  virtual void Shutdown();
   virtual void ReportMetricsImpl();
   virtual void AddLoginImpl(const webkit_glue::PasswordForm& form);
   virtual void UpdateLoginImpl(const webkit_glue::PasswordForm& form);
@@ -48,23 +47,21 @@ class PasswordStoreDefault : public PasswordStore,
 
   scoped_refptr<WebDataService> web_data_service_;
 
-  // Implements the WebDataService consumer interface.
-  virtual void OnWebDataServiceRequestDone(WebDataService::Handle handle,
-                                           const WDTypedResult *result);
-
  protected:
   inline bool DeleteAndRecreateDatabaseFile() {
     return login_db_->DeleteAndRecreateDatabaseFile();
   }
 
  private:
+  class MigrateHelper;
+
   // Migrates logins from the WDS to the LoginDatabase.
   void MigrateIfNecessary();
 
   scoped_ptr<LoginDatabase> login_db_;
   Profile* profile_;
 
-  std::set<WebDataService::Handle> handles_;
+  scoped_ptr<MigrateHelper> migrate_helper_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordStoreDefault);
 };
diff --git a/chrome/browser/password_manager/password_store_default_unittest.cc b/chrome/browser/password_manager/password_store_default_unittest.cc
index 19b70a1..212de2f 100644
--- a/chrome/browser/password_manager/password_store_default_unittest.cc
+++ b/chrome/browser/password_manager/password_store_default_unittest.cc
@@ -285,7 +285,7 @@ TEST_F(PasswordStoreDefaultTest, Migration) {
   done.Wait();
 
   // Initializing the PasswordStore should trigger a migration.
-  scoped_refptr<PasswordStoreDefault> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreDefault(login_db_.release(),
           profile_.get(), wds_.get()));
   store->Init();
@@ -364,6 +364,8 @@ TEST_F(PasswordStoreDefaultTest, Migration) {
 
   STLDeleteElements(&expected_autofillable);
   STLDeleteElements(&expected_blacklisted);
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreDefaultTest, MigrationAlreadyDone) {
@@ -400,7 +402,7 @@ TEST_F(PasswordStoreDefaultTest, MigrationAlreadyDone) {
                                             true);
 
   // Initializing the PasswordStore shouldn't trigger a migration.
-  scoped_refptr<PasswordStoreDefault> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreDefault(login_db_.release(), profile_.get(),
                                wds_.get()));
   store->Init();
@@ -421,6 +423,8 @@ TEST_F(PasswordStoreDefaultTest, MigrationAlreadyDone) {
   MessageLoop::current()->Run();
 
   STLDeleteElements(&unexpected_autofillable);
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreDefaultTest, Notifications) {
@@ -429,7 +433,7 @@ TEST_F(PasswordStoreDefaultTest, Notifications) {
                                             true);
 
   // Initializing the PasswordStore shouldn't trigger a migration.
-  scoped_refptr<PasswordStoreDefault> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreDefault(login_db_.release(), profile_.get(),
                                wds_.get()));
   store->Init();
@@ -511,4 +515,6 @@ TEST_F(PasswordStoreDefaultTest, Notifications) {
   BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
       new SignalingTask(&done));
   done.Wait();
+
+  store->Shutdown();
 }
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc
index 3deabb3..01a5b6f 100644
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc
+++ b/chrome/browser/password_manager/password_store_mac_unittest.cc
@@ -913,6 +913,7 @@ class PasswordStoreMacTest : public testing::Test {
   }
 
   virtual void TearDown() {
+    store_->Shutdown();
     MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask);
     MessageLoop::current()->Run();
   }
diff --git a/chrome/browser/password_manager/password_store_win.cc b/chrome/browser/password_manager/password_store_win.cc
index 415fbc4..a1100fc 100644
--- a/chrome/browser/password_manager/password_store_win.cc
+++ b/chrome/browser/password_manager/password_store_win.cc
@@ -4,12 +4,15 @@
 
 #include "chrome/browser/password_manager/password_store_win.h"
 
+#include <map>
+
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/password_manager/ie7_password.h"
 #include "chrome/browser/password_manager/password_manager.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/webdata/web_data_service.h"
 
 using webkit_glue::PasswordForm;
 
@@ -36,100 +39,73 @@ class FormGetLoginsRequest : public PasswordStore::GetLoginsRequest {
  private:
   scoped_ptr<PasswordForm> form_;
 };
-}
 
-PasswordStoreWin::PasswordStoreWin(LoginDatabase* login_database,
-                                   Profile* profile,
-                                   WebDataService* web_data_service)
-    : PasswordStoreDefault(login_database, profile, web_data_service) {
-}
-
-PasswordStoreWin::~PasswordStoreWin() {
-  DCHECK(pending_requests_.empty() ||
-         BrowserThread::CurrentlyOn(BrowserThread::DB));
+}  // namespace
 
-  for (PendingRequestMap::const_iterator it = pending_requests_.begin();
-       it != pending_requests_.end(); ++it) {
-    web_data_service_->CancelRequest(it->first);
+// Handles requests to WebDataService.
+class PasswordStoreWin::DBHandler : public WebDataServiceConsumer {
+ public:
+  DBHandler(WebDataService* web_data_service,
+            PasswordStoreWin* password_store)
+      : web_data_service_(web_data_service),
+        password_store_(password_store) {
   }
-}
 
-PasswordStore::GetLoginsRequest* PasswordStoreWin::NewGetLoginsRequest(
-    GetLoginsCallback* callback) {
-  return new FormGetLoginsRequest(callback);
-}
+  ~DBHandler();
 
-void PasswordStoreWin::ForwardLoginsResult(GetLoginsRequest* request) {
-  if (static_cast<FormGetLoginsRequest*>(request)->IsLoginsRequest() &&
-      request->value.empty()) {
-    IE7PasswordInfo info;
-    std::wstring url = ASCIIToWide(
-        static_cast<FormGetLoginsRequest*>(request)->form()->origin.spec());
-    info.url_hash = ie7_password::GetUrlHash(url);
-    WebDataService::Handle handle = web_data_service_->GetIE7Login(info,
-                                                                   this);
-    TrackRequest(handle, request);
-  } else {
-    PasswordStore::ForwardLoginsResult(request);
-  }
-}
+  // Requests the IE7 login for |url| and |request|. This is async. The request
+  // is processed when complete.
+  void GetIE7Login(const GURL& url, GetLoginsRequest* request);
 
-void PasswordStoreWin::OnWebDataServiceRequestDone(
-    WebDataService::Handle handle, const WDTypedResult* result) {
-  if (!result)
-    return;  // The WDS returns NULL if it is shutting down.
+ private:
+  // Holds requests associated with in-flight GetLogin queries.
+  typedef std::map<WebDataService::Handle,
+                   scoped_refptr<GetLoginsRequest> > PendingRequestMap;
 
-  if (PASSWORD_IE7_RESULT == result->GetType()) {
-    scoped_refptr<GetLoginsRequest> request(TakeRequestWithHandle(handle));
+  // Gets logins from IE7 if no others are found. Also copies them into
+  // Chrome's WebDatabase so we don't need to look next time.
+  PasswordForm* GetIE7Result(const WDTypedResult* result,
+                             const PasswordForm& form);
 
-    // If the request was cancelled, we are done.
-    if (!request.get())
-      return;
+  // WebDataServiceConsumer.
+  virtual void OnWebDataServiceRequestDone(
+      WebDataService::Handle handle,
+      const WDTypedResult* result) OVERRIDE;
 
-    // This is a response from WebDataService::GetIE7Login.
-    PasswordForm* form = static_cast<FormGetLoginsRequest*>(
-        request.get())->form();
-    DCHECK(form);
-    PasswordForm* ie7_form = GetIE7Result(result, *form);
+  scoped_refptr<WebDataService> web_data_service_;
 
-    if (ie7_form)
-      request->value.push_back(ie7_form);
+  // This creates a cycle between us and PasswordStore. The cycle is broken
+  // from PasswordStoreWin::Shutdown, which deletes us.
+  scoped_refptr<PasswordStoreWin> password_store_;
 
-    PasswordStore::ForwardLoginsResult(request.get());
-  } else {
-    PasswordStoreDefault::OnWebDataServiceRequestDone(handle, result);
-  }
-}
+  // Holds requests associated with in-flight GetLogin queries.
+  PendingRequestMap pending_requests_;
 
-void PasswordStoreWin::GetLoginsImpl(GetLoginsRequest* request,
-                                     const PasswordForm& form) {
-  static_cast<FormGetLoginsRequest*>(request)->SetLoginsRequestForm(form);
+  DISALLOW_COPY_AND_ASSIGN(DBHandler);
+};
 
-  PasswordStoreDefault::GetLoginsImpl(request, form);
+PasswordStoreWin::DBHandler::~DBHandler() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  for (PendingRequestMap::const_iterator i = pending_requests_.begin();
+       i != pending_requests_.end(); ++i) {
+    web_data_service_->CancelRequest(i->first);
+  }
 }
 
-void PasswordStoreWin::TrackRequest(WebDataService::Handle handle,
-                                    GetLoginsRequest* request) {
+void PasswordStoreWin::DBHandler::GetIE7Login(const GURL& url,
+                                              GetLoginsRequest* request) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
-
+  IE7PasswordInfo info;
+  info.url_hash = ie7_password::GetUrlHash(UTF8ToWide(url.spec()));
+  WebDataService::Handle handle = web_data_service_->GetIE7Login(info, this);
   pending_requests_.insert(PendingRequestMap::value_type(handle, request));
 }
 
-PasswordStoreDefault::GetLoginsRequest* PasswordStoreWin::TakeRequestWithHandle(
-    WebDataService::Handle handle) {
+PasswordForm* PasswordStoreWin::DBHandler::GetIE7Result(
+    const WDTypedResult *result,
+    const PasswordForm& form) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
 
-  PendingRequestMap::iterator it(pending_requests_.find(handle));
-  if (it == pending_requests_.end())
-    return NULL;
-
-  GetLoginsRequest* request = it->second;
-  pending_requests_.erase(it);
-  return request;
-}
-
-PasswordForm* PasswordStoreWin::GetIE7Result(const WDTypedResult *result,
-                                             const PasswordForm& form) {
   const WDResult<IE7PasswordInfo>* r =
       static_cast<const WDResult<IE7PasswordInfo>*>(result);
   IE7PasswordInfo info = r->GetValue();
@@ -153,9 +129,82 @@ PasswordForm* PasswordStoreWin::GetIE7Result(const WDTypedResult *result,
     autofill->preferred = true;
     autofill->ssl_valid = form.origin.SchemeIsSecure();
     autofill->date_created = info.date_created;
-    // Add this PasswordForm to the saved password table.
-    AddLogin(*autofill);
+    // Add this PasswordForm to the saved password table. We're on the DB thread
+    // already, so we use AddLoginImpl.
+    password_store_->AddLoginImpl(*autofill);
     return autofill;
   }
   return NULL;
 }
+
+void PasswordStoreWin::DBHandler::OnWebDataServiceRequestDone(
+    WebDataService::Handle handle,
+    const WDTypedResult* result) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
+  PendingRequestMap::iterator i(pending_requests_.find(handle));
+  DCHECK(i != pending_requests_.end());
+  scoped_refptr<GetLoginsRequest> request(i->second);
+  pending_requests_.erase(i);
+
+  if (!result)
+    return;  // The WDS returns NULL if it is shutting down.
+
+  DCHECK_EQ(PASSWORD_IE7_RESULT, result->GetType());
+  PasswordForm* form =
+      static_cast<FormGetLoginsRequest*>(request.get())->form();
+  DCHECK(form);
+  PasswordForm* ie7_form = GetIE7Result(result, *form);
+
+  if (ie7_form)
+    request->value.push_back(ie7_form);
+
+  request->ForwardResult(GetLoginsRequest::TupleType(request->handle(),
+                                                     request->value));
+}
+
+PasswordStoreWin::PasswordStoreWin(LoginDatabase* login_database,
+                                   Profile* profile,
+                                   WebDataService* web_data_service)
+    : PasswordStoreDefault(login_database, profile, web_data_service) {
+  db_handler_.reset(new DBHandler(web_data_service, this));
+}
+
+PasswordStoreWin::~PasswordStoreWin() {
+}
+
+void PasswordStoreWin::ShutdownOnDBThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  db_handler_.reset();
+}
+
+PasswordStore::GetLoginsRequest* PasswordStoreWin::NewGetLoginsRequest(
+    GetLoginsCallback* callback) {
+  return new FormGetLoginsRequest(callback);
+}
+
+void PasswordStoreWin::Shutdown() {
+  BrowserThread::PostTask(
+      BrowserThread::DB, FROM_HERE,
+      NewRunnableMethod(this, &PasswordStoreWin::ShutdownOnDBThread));
+  PasswordStoreDefault::Shutdown();
+}
+
+void PasswordStoreWin::ForwardLoginsResult(GetLoginsRequest* request) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+  if (static_cast<FormGetLoginsRequest*>(request)->IsLoginsRequest() &&
+      request->value.empty() && db_handler_.get()) {
+    db_handler_->GetIE7Login(
+        static_cast<FormGetLoginsRequest*>(request)->form()->origin,
+        request);
+  } else {
+    PasswordStore::ForwardLoginsResult(request);
+  }
+}
+
+void PasswordStoreWin::GetLoginsImpl(GetLoginsRequest* request,
+                                     const PasswordForm& form) {
+  static_cast<FormGetLoginsRequest*>(request)->SetLoginsRequestForm(form);
+
+  PasswordStoreDefault::GetLoginsImpl(request, form);
+}
diff --git a/chrome/browser/password_manager/password_store_win.h b/chrome/browser/password_manager/password_store_win.h
index 2cf238d..f08bbcd 100644
--- a/chrome/browser/password_manager/password_store_win.h
+++ b/chrome/browser/password_manager/password_store_win.h
@@ -6,8 +6,7 @@
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_WIN_H_
 #pragma once
 
-#include <map>
-
+#include "base/scoped_ptr.h"
 #include "chrome/browser/password_manager/password_store_default.h"
 
 class LoginDatabase;
@@ -28,38 +27,25 @@ class PasswordStoreWin : public PasswordStoreDefault {
                    WebDataService* web_data_service);
 
  private:
+  class DBHandler;
+
   virtual ~PasswordStoreWin();
 
+  // Invoked from Shutdown, but run on the DB thread.
+  void ShutdownOnDBThread();
+
   virtual GetLoginsRequest* NewGetLoginsRequest(
       GetLoginsCallback* callback) OVERRIDE;
 
   // See PasswordStoreDefault.
+  virtual void Shutdown() OVERRIDE;
   virtual void ForwardLoginsResult(GetLoginsRequest* request) OVERRIDE;
-  virtual void OnWebDataServiceRequestDone(
-      WebDataService::Handle h, const WDTypedResult* result) OVERRIDE;
 
   // Overridden so that we can save the form for later use.
   virtual void GetLoginsImpl(GetLoginsRequest* request,
                              const webkit_glue::PasswordForm& form) OVERRIDE;
 
-  // Takes ownership of |request| and tracks it under |handle|.
-  void TrackRequest(WebDataService::Handle handle, GetLoginsRequest* request);
-
-  // Finds the GetLoginsRequest associated with the in-flight WebDataService
-  // request identified by |handle|, removes it from the tracking list, and
-  // returns it. Ownership of the GetLoginsRequest passes to the caller.
-  // Returns NULL if the request has been cancelled.
-  GetLoginsRequest* TakeRequestWithHandle(WebDataService::Handle handle);
-
-  // Gets logins from IE7 if no others are found. Also copies them into
-  // Chrome's WebDatabase so we don't need to look next time.
-  webkit_glue::PasswordForm* GetIE7Result(
-      const WDTypedResult* result,
-      const webkit_glue::PasswordForm& form);
-
-  // Holds requests associated with in-flight GetLogin queries.
-  typedef std::map<WebDataService::Handle, GetLoginsRequest*> PendingRequestMap;
-  PendingRequestMap pending_requests_;
+  scoped_ptr<DBHandler> db_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordStoreWin);
 };
diff --git a/chrome/browser/password_manager/password_store_win_unittest.cc b/chrome/browser/password_manager/password_store_win_unittest.cc
index 98ad2be..6477b12 100644
--- a/chrome/browser/password_manager/password_store_win_unittest.cc
+++ b/chrome/browser/password_manager/password_store_win_unittest.cc
@@ -18,6 +18,7 @@
 #include "chrome/browser/password_manager/password_store_win.h"
 #include "chrome/browser/password_manager/ie7_password.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/webdata/web_data_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/signaling_task.h"
 #include "chrome/test/testing_profile.h"
@@ -171,7 +172,7 @@ TEST_F(PasswordStoreWinTest, DISABLED_ConvertIE7Login) {
                                             true);
 
   // Initializing the PasswordStore shouldn't trigger a migration.
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   EXPECT_TRUE(store->Init());
 
@@ -220,6 +221,8 @@ TEST_F(PasswordStoreWinTest, DISABLED_ConvertIE7Login) {
   MessageLoop::current()->Run();
 
   STLDeleteElements(&forms);
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreWinTest, OutstandingWDSQueries) {
@@ -228,7 +231,7 @@ TEST_F(PasswordStoreWinTest, OutstandingWDSQueries) {
                                             true);
 
   // Initializing the PasswordStore shouldn't trigger a migration.
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   EXPECT_TRUE(store->Init());
 
@@ -250,6 +253,7 @@ TEST_F(PasswordStoreWinTest, OutstandingWDSQueries) {
   store->GetLogins(*form, &consumer);
 
   // Release the PSW and the WDS before the query can return.
+  store->Shutdown();
   store = NULL;
   wds_->Shutdown();
   wds_ = NULL;
@@ -277,7 +281,7 @@ TEST_F(PasswordStoreWinTest, DISABLED_MultipleWDSQueriesOnDifferentThreads) {
                                             true);
 
   // Initializing the PasswordStore shouldn't trigger a migration.
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   EXPECT_TRUE(store->Init());
 
@@ -338,6 +342,8 @@ TEST_F(PasswordStoreWinTest, DISABLED_MultipleWDSQueriesOnDifferentThreads) {
   MessageLoop::current()->Run();
 
   STLDeleteElements(&forms);
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreWinTest, Migration) {
@@ -426,7 +432,7 @@ TEST_F(PasswordStoreWinTest, Migration) {
   done.Wait();
 
   // Initializing the PasswordStore should trigger a migration.
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   store->Init();
 
@@ -507,7 +513,7 @@ TEST_F(PasswordStoreWinTest, Migration) {
 }
 
 TEST_F(PasswordStoreWinTest, EmptyLogins) {
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   store->Init();
 
@@ -539,10 +545,12 @@ TEST_F(PasswordStoreWinTest, EmptyLogins) {
 
   store->GetLogins(*form, &consumer);
   MessageLoop::current()->Run();
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreWinTest, EmptyBlacklistLogins) {
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   store->Init();
 
@@ -560,10 +568,12 @@ TEST_F(PasswordStoreWinTest, EmptyBlacklistLogins) {
 
   store->GetBlacklistLogins(&consumer);
   MessageLoop::current()->Run();
+
+  store->Shutdown();
 }
 
 TEST_F(PasswordStoreWinTest, EmptyAutofillableLogins) {
-  scoped_refptr<PasswordStoreWin> store(
+  scoped_refptr<PasswordStore> store(
       new PasswordStoreWin(login_db_.release(), profile_.get(), wds_.get()));
   store->Init();
 
@@ -581,4 +591,6 @@ TEST_F(PasswordStoreWinTest, EmptyAutofillableLogins) {
 
   store->GetAutofillableLogins(&consumer);
   MessageLoop::current()->Run();
+
+  store->Shutdown();
 }
diff --git a/chrome/browser/password_manager/password_store_x_unittest.cc b/chrome/browser/password_manager/password_store_x_unittest.cc
index 80eaa6a..721368d 100644
--- a/chrome/browser/password_manager/password_store_x_unittest.cc
+++ b/chrome/browser/password_manager/password_store_x_unittest.cc
@@ -436,6 +436,8 @@ TEST_P(PasswordStoreXTest, WDSMigration) {
 
   STLDeleteElements(&expected_autofillable);
   STLDeleteElements(&expected_blacklisted);
+
+  store->Shutdown();
 }
 
 TEST_P(PasswordStoreXTest, WDSMigrationAlreadyDone) {
@@ -499,6 +501,8 @@ TEST_P(PasswordStoreXTest, WDSMigrationAlreadyDone) {
   MessageLoop::current()->Run();
 
   STLDeleteElements(&unexpected_autofillable);
+
+  store->Shutdown();
 }
 
 TEST_P(PasswordStoreXTest, Notifications) {
@@ -591,6 +595,8 @@ TEST_P(PasswordStoreXTest, Notifications) {
   BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
       new SignalingTask(&done));
   done.Wait();
+
+  store->Shutdown();
 }
 
 TEST_P(PasswordStoreXTest, NativeMigration) {
@@ -730,6 +736,8 @@ TEST_P(PasswordStoreXTest, NativeMigration) {
 
   STLDeleteElements(&expected_autofillable);
   STLDeleteElements(&expected_blacklisted);
+
+  store->Shutdown();
 }
 
 INSTANTIATE_TEST_CASE_P(NoBackend,
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 9f507eb..9e8adce 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -609,6 +609,10 @@ ProfileImpl::~ProfileImpl() {
   // The sync service needs to be deleted before the services it calls.
   sync_service_.reset();
 
+  // Password store uses WebDB, shut it down before the WebDB has been shutdown.
+  if (password_store_.get())
+    password_store_->Shutdown();
+
   // Both HistoryService and WebDataService maintain threads for background
   // processing. Its possible each thread still has tasks on it that have
   // increased the ref count of the service. In such a situation, when we
diff --git a/chrome/browser/sync/profile_sync_service_password_unittest.cc b/chrome/browser/sync/profile_sync_service_password_unittest.cc
index 16cb99e..2f0f719 100644
--- a/chrome/browser/sync/profile_sync_service_password_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_password_unittest.cc
@@ -169,6 +169,7 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
   }
 
   virtual void TearDown() {
+    password_store_->Shutdown();
     service_.reset();
     notification_service_->TearDown();
     db_thread_.Stop();