diff options
author | stuartmorgan@chromium.org <stuartmorgan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-03 22:00:41 +0000 |
---|---|---|
committer | stuartmorgan@chromium.org <stuartmorgan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-03 22:00:41 +0000 |
commit | e69d33959a5b34c3796ffc55f187a073d92fd28f (patch) | |
tree | 6437de4b98dbd0e89e1e0e6a6431bd4842447c46 | |
parent | d1d1f9f14a63c8b8a6b882c5e9bf5f67eeb3d574 (diff) | |
download | chromium_src-e69d33959a5b34c3796ffc55f187a073d92fd28f.zip chromium_src-e69d33959a5b34c3796ffc55f187a073d92fd28f.tar.gz chromium_src-e69d33959a5b34c3796ffc55f187a073d92fd28f.tar.bz2 |
Change PasswordStoreDefault to access the WebDataService from the UI thread only.
Enables the PasswordStore refactoring yet again (third time's the charm?).
BUG=12479
TEST=Password save/autofill should continue to work on Windows.
Review URL: http://codereview.chromium.org/118131
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17545 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | build/linux/system.gyp | 37 | ||||
-rw-r--r-- | chrome/browser/browser.vcproj | 22 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_form_manager.cc | 87 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_form_manager.h | 23 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_form_manager_win.cc | 77 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_manager.cc | 4 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_manager.h | 2 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_store.h | 13 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_store_default.cc | 95 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_store_default.h | 27 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_store_win.cc | 22 | ||||
-rw-r--r-- | chrome/browser/profile.cc | 54 | ||||
-rw-r--r-- | chrome/browser/profile.h | 9 | ||||
-rw-r--r-- | chrome/chrome.gyp | 33 | ||||
-rw-r--r-- | chrome/test/testing_profile.h | 3 |
15 files changed, 283 insertions, 225 deletions
diff --git a/build/linux/system.gyp b/build/linux/system.gyp index a3e8cbc..68b52be 100644 --- a/build/linux/system.gyp +++ b/build/linux/system.gyp @@ -140,5 +140,42 @@ ], }, }, +# TODO(evanm): temporarily disabled while we figure out whether to depend +# on gnome-keyring etc. +# http://code.google.com/p/chromium/issues/detail?id=12351 +# { +# 'target_name': 'gnome-keyring', +# 'type': 'settings', +# 'direct_dependent_settings': { +# 'cflags': [ +# '<!@(pkg-config --cflags gnome-keyring-1)', +# ], +# }, +# 'link_settings': { +# 'ldflags': [ +# '<!@(pkg-config --libs-only-L --libs-only-other gnome-keyring-1)', +# ], +# 'libraries': [ +# '<!@(pkg-config --libs-only-l gnome-keyring-1)', +# ], +# }, +# }, +# { +# 'target_name': 'dbus-glib', +# 'type': 'settings', +# 'direct_dependent_settings': { +# 'cflags': [ +# '<!@(pkg-config --cflags dbus-glib-1)', +# ], +# }, +# 'link_settings': { +# 'ldflags': [ +# '<!@(pkg-config --libs-only-L --libs-only-other dbus-glib-1)', +# ], +# 'libraries': [ +# '<!@(pkg-config --libs-only-l dbus-glib-1)', +# ], +# }, +# }, ], } diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj index 108fb73..403a2e3b 100644 --- a/chrome/browser/browser.vcproj +++ b/chrome/browser/browser.vcproj @@ -1532,15 +1532,31 @@ > </File> <File - RelativePath=".\password_manager\password_form_manager_win.cc" + RelativePath=".\password_manager\password_manager.cc" > </File> <File - RelativePath=".\password_manager\password_manager.cc" + RelativePath=".\password_manager\password_manager.h" > </File> <File - RelativePath=".\password_manager\password_manager.h" + RelativePath=".\password_manager\password_store.cc" + > + </File> + <File + RelativePath=".\password_manager\password_store.h" + > + </File> + <File + RelativePath=".\password_manager\password_store_default.cc" + > + </File> + <File + RelativePath=".\password_manager\password_store_default.h" + > + </File> + <File + RelativePath=".\password_manager\password_store_win.cc" > </File> <File diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc index cf40a1b..26ebd4c 100644 --- a/chrome/browser/password_manager/password_form_manager.cc +++ b/chrome/browser/password_manager/password_form_manager.cc @@ -21,7 +21,7 @@ PasswordFormManager::PasswordFormManager(Profile* profile, observed_form_(observed_form), is_new_login_(true), password_manager_(password_manager), - pending_login_query_(NULL), + pending_login_query_(0), preferred_match_(NULL), state_(PRE_MATCHING_PHASE), profile_(profile) { @@ -168,25 +168,21 @@ void PasswordFormManager::FetchMatchingLoginsFromWebDatabase() { DCHECK_EQ(state_, PRE_MATCHING_PHASE); DCHECK(!pending_login_query_); state_ = MATCHING_PHASE; - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::EXPLICIT_ACCESS); - if (!web_data_service) { + PasswordStore* password_store = + profile_->GetPasswordStore(Profile::EXPLICIT_ACCESS); + if (!password_store) { NOTREACHED(); return; } - pending_login_query_ = web_data_service->GetLogins(observed_form_, this); + pending_login_query_ = password_store->GetLogins(observed_form_, this); } bool PasswordFormManager::HasCompletedMatching() { return state_ == POST_MATCHING_PHASE; } -void PasswordFormManager::OnRequestDone(WebDataService::Handle h, - const WDTypedResult* result) { - // Get the result from the database into a usable form. - const WDResult<std::vector<PasswordForm*> >* r = - static_cast<const WDResult<std::vector<PasswordForm*> >*>(result); - std::vector<PasswordForm*> logins_result = r->GetValue(); +void PasswordFormManager::OnRequestDone(int handle, + const std::vector<PasswordForm*>& logins_result) { // Note that the result gets deleted after this call completes, but we own // the PasswordForm objects pointed to by the result vector, thus we keep // copies to a minimum here. @@ -239,14 +235,6 @@ void PasswordFormManager::OnRequestDone(WebDataService::Handle h, // We're done matching now. state_ = POST_MATCHING_PHASE; - if (best_score <= 0) { -#if defined(OS_WIN) - state_ = PRE_MATCHING_PHASE; - FetchMatchingIE7LoginFromWebDatabase(); -#endif - return; - } - for (std::vector<PasswordForm>::const_iterator it = empties.begin(); it != empties.end(); ++it) { // If we don't already have a result with the same username, add the @@ -272,30 +260,17 @@ void PasswordFormManager::OnRequestDone(WebDataService::Handle h, } } -void PasswordFormManager::OnWebDataServiceRequestDone(WebDataService::Handle h, - const WDTypedResult* result) { +void PasswordFormManager::OnPasswordStoreRequestDone( + int handle, const std::vector<PasswordForm*>& result) { DCHECK_EQ(state_, MATCHING_PHASE); - DCHECK_EQ(pending_login_query_, h); - DCHECK(result); - pending_login_query_ = NULL; + DCHECK_EQ(pending_login_query_, handle); - if (!result) + if (result.empty()) { + state_ = POST_MATCHING_PHASE; return; - - switch (result->GetType()) { - case PASSWORD_RESULT: { - OnRequestDone(h, result); - break; - } -#if defined(OS_WIN) - case PASSWORD_IE7_RESULT: { - OnIE7RequestDone(h, result); - break; - } -#endif - default: - NOTREACHED(); } + + OnRequestDone(handle, result); } bool PasswordFormManager::IgnoreResult(const PasswordForm& form) const { @@ -322,14 +297,15 @@ void PasswordFormManager::SaveAsNewLogin() { DCHECK(!profile_->IsOffTheRecord()); - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::IMPLICIT_ACCESS); - if (!web_data_service) { + PasswordStore* password_store = + profile_->GetPasswordStore(Profile::IMPLICIT_ACCESS); + if (!password_store) { NOTREACHED(); return; } + pending_credentials_.date_created = Time::Now(); - web_data_service->AddLogin(pending_credentials_); + password_store->AddLogin(pending_credentials_); } void PasswordFormManager::UpdateLogin() { @@ -341,9 +317,9 @@ void PasswordFormManager::UpdateLogin() { DCHECK(!IsNewLogin() && pending_credentials_.preferred); DCHECK(!profile_->IsOffTheRecord()); - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::IMPLICIT_ACCESS); - if (!web_data_service) { + PasswordStore* password_store = + profile_->GetPasswordStore(Profile::IMPLICIT_ACCESS); + if (!password_store) { NOTREACHED(); return; } @@ -355,7 +331,7 @@ void PasswordFormManager::UpdateLogin() { iter->second->preferred) { // This wasn't the selected login but it used to be preferred. iter->second->preferred = false; - web_data_service->UpdateLogin(*iter->second); + password_store->UpdateLogin(*iter->second); } } // Update the new preferred login. @@ -380,23 +356,20 @@ void PasswordFormManager::UpdateLogin() { PasswordForm copy(pending_credentials_); copy.origin = observed_form_.origin; copy.action = observed_form_.action; - web_data_service->AddLogin(copy); + password_store->AddLogin(copy); } else { - web_data_service->UpdateLogin(pending_credentials_); + password_store->UpdateLogin(pending_credentials_); } } void PasswordFormManager::CancelLoginsQuery() { - if (!pending_login_query_) - return; - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::EXPLICIT_ACCESS); - if (!web_data_service) { - NOTREACHED(); + PasswordStore* password_store = + profile_->GetPasswordStore(Profile::IMPLICIT_ACCESS); + if (!password_store) { + // Can be NULL in unit tests. return; } - web_data_service->CancelRequest(pending_login_query_); - pending_login_query_ = NULL; + password_store->CancelLoginsQuery(pending_login_query_); } int PasswordFormManager::ScoreResult(const PasswordForm& candidate) const { diff --git a/chrome/browser/password_manager/password_form_manager.h b/chrome/browser/password_manager/password_form_manager.h index 332eea3..90ffc86 100644 --- a/chrome/browser/password_manager/password_form_manager.h +++ b/chrome/browser/password_manager/password_form_manager.h @@ -8,6 +8,7 @@ #include "build/build_config.h" #include "base/stl_util-inl.h" +#include "chrome/browser/password_manager/password_store.h" #include "chrome/browser/webdata/web_data_service.h" #include "webkit/glue/password_form.h" @@ -16,7 +17,7 @@ class Profile; // Per-password-form-{on-page, dialog} class responsible for interactions // between a given form, the per-tab PasswordManager, and the web database. -class PasswordFormManager : public WebDataServiceConsumer { +class PasswordFormManager : public PasswordStoreConsumer { public: // web_data_service allows access to current profile's Web Data // password_manager owns this object @@ -34,9 +35,6 @@ class PasswordFormManager : public WebDataServiceConsumer { // Retrieves potential matching logins from the database. void FetchMatchingLoginsFromWebDatabase(); -#if defined(OS_WIN) - void FetchMatchingIE7LoginFromWebDatabase(); -#endif // Simple state-check to verify whether this object as received a callback // from the web database and completed its matching phase. Note that the @@ -60,19 +58,12 @@ class PasswordFormManager : public WebDataServiceConsumer { // managed by this. bool IsNewLogin(); - // WebDataServiceConsumer implementation. If matches were found - // (in *result), this is where we determine we need to autofill. - virtual void OnWebDataServiceRequestDone(WebDataService::Handle h, - const WDTypedResult* result); - // Determines if we need to autofill given the results of the query. - void OnRequestDone(WebDataService::Handle h, const WDTypedResult* result); + void OnRequestDone(int handle, const std::vector<PasswordForm*>& result); -#if defined(OS_WIN) - // Determines if we need to autofill given the results of the query in the - // ie7_password table. - void OnIE7RequestDone(WebDataService::Handle h, const WDTypedResult* result); -#endif + // PasswordStoreConsumer implementation. + virtual void OnPasswordStoreRequestDone( + int handle, const std::vector<PasswordForm*>& result); // A user opted to 'never remember' passwords for this form. // Blacklist it so that from now on when it is seen we ignore it. @@ -141,7 +132,7 @@ class PasswordFormManager : public WebDataServiceConsumer { const PasswordManager* const password_manager_; // Handle to any pending WebDataService::GetLogins query. - WebDataService::Handle pending_login_query_; + int pending_login_query_; // Convenience pointer to entry in best_matches_ that is marked // as preferred. This is only allowed to be null if there are no best matches diff --git a/chrome/browser/password_manager/password_form_manager_win.cc b/chrome/browser/password_manager/password_form_manager_win.cc index a075617..e69de29 100644 --- a/chrome/browser/password_manager/password_form_manager_win.cc +++ b/chrome/browser/password_manager/password_form_manager_win.cc @@ -1,77 +0,0 @@ -// Copyright (c) 2009 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 "base/string_util.h" -#include "chrome/browser/password_manager/ie7_password.h" -#include "chrome/browser/password_manager/password_manager.h" -#include "chrome/browser/profile.h" - -void PasswordFormManager::FetchMatchingIE7LoginFromWebDatabase() { - DCHECK_EQ(state_, PRE_MATCHING_PHASE); - DCHECK(!pending_login_query_); - state_ = MATCHING_PHASE; - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::EXPLICIT_ACCESS); - if (!web_data_service) { - NOTREACHED(); - return; - } - - IE7PasswordInfo info; - std::wstring url = ASCIIToWide(observed_form_.origin.spec()); - info.url_hash = ie7_password::GetUrlHash(url); - pending_login_query_ = web_data_service->GetIE7Login(info, this); -} - -void PasswordFormManager::OnIE7RequestDone(WebDataService::Handle h, - const WDTypedResult* result) { - // Get the result from the database into a usable form. - const WDResult<IE7PasswordInfo>* r = - static_cast<const WDResult<IE7PasswordInfo>*>(result); - IE7PasswordInfo result_value = r->GetValue(); - - state_ = POST_MATCHING_PHASE; - - if (!result_value.encrypted_data.empty()) { - // We got a result. - // Delete the entry. If it's good we will add it to the real saved password - // table. - WebDataService* web_data_service = - profile_->GetWebDataService(Profile::EXPLICIT_ACCESS); - if (!web_data_service) { - NOTREACHED(); - return; - } - web_data_service->RemoveIE7Login(result_value); - - std::wstring username; - std::wstring password; - std::wstring url = ASCIIToWide(observed_form_.origin.spec()); - if (!ie7_password::DecryptPassword(url, result_value.encrypted_data, - &username, &password)) { - return; - } - - PasswordForm* auto_fill = new PasswordForm(observed_form_); - auto_fill->username_value = username; - auto_fill->password_value = password; - auto_fill->preferred = true; - auto_fill->ssl_valid = observed_form_.origin.SchemeIsSecure(); - auto_fill->date_created = result_value.date_created; - // Add this PasswordForm to the saved password table. - web_data_service->AddLogin(*auto_fill); - - if (IgnoreResult(*auto_fill)) { - delete auto_fill; - return; - } - - best_matches_[auto_fill->username_value] = auto_fill; - preferred_match_ = auto_fill; - password_manager_->Autofill(observed_form_, best_matches_, - preferred_match_); - } -} diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc index b558bf94..e09aa08 100644 --- a/chrome/browser/password_manager/password_manager.cc +++ b/chrome/browser/password_manager/password_manager.cc @@ -8,6 +8,7 @@ #include "app/resource_bundle.h" #include "base/stl_util-inl.h" #include "base/string_util.h" +#include "chrome/browser/password_manager/password_form_manager.h" #include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/notification_registrar.h" @@ -234,8 +235,9 @@ void PasswordManager::Autofill( return; } default: - if (observer_) + if (observer_) { observer_->OnAutofillDataAvailable(preferred_match->username_value, preferred_match->password_value); + } } } diff --git a/chrome/browser/password_manager/password_manager.h b/chrome/browser/password_manager/password_manager.h index 9c80714..175e4cd 100644 --- a/chrome/browser/password_manager/password_manager.h +++ b/chrome/browser/password_manager/password_manager.h @@ -7,13 +7,13 @@ #include "base/scoped_ptr.h" #include "base/stl_util-inl.h" -#include "chrome/browser/password_manager/password_form_manager.h" #include "chrome/browser/tab_contents/infobar_delegate.h" #include "chrome/browser/views/login_view.h" #include "chrome/common/pref_member.h" #include "webkit/glue/password_form.h" #include "webkit/glue/password_form_dom_manager.h" +class PasswordFormManager; class PrefService; class TabContents; diff --git a/chrome/browser/password_manager/password_store.h b/chrome/browser/password_manager/password_store.h index 7b2144c..1850e48 100644 --- a/chrome/browser/password_manager/password_store.h +++ b/chrome/browser/password_manager/password_store.h @@ -35,17 +35,20 @@ class PasswordStore : public base::RefCountedThreadSafe<PasswordStore> { // Reimplement this to add custom initialization. Always call this too. virtual bool Init(); + // TODO(stuartmorgan): These are only virtual to support the shim version of + // password_store_default; once that is fixed, they can become non-virtual. + // Adds the given PasswordForm to the secure password store asynchronously. - void AddLogin(const PasswordForm& form); + virtual void AddLogin(const PasswordForm& form); // Updates the matching PasswordForm in the secure password store (async). - void UpdateLogin(const PasswordForm& form); + virtual void UpdateLogin(const PasswordForm& form); // Removes the matching PasswordForm from the secure password store (async). - void RemoveLogin(const PasswordForm& form); + virtual void RemoveLogin(const PasswordForm& form); // Searches for a matching PasswordForm and returns a handle so the async // request can be tracked. Implement the PasswordStoreConsumer interface to // be notified on completion. - int GetLogins(const PasswordForm& form, - PasswordStoreConsumer* consumer); + virtual int GetLogins(const PasswordForm& form, + PasswordStoreConsumer* consumer); // Cancels a previous GetLogins query (async) virtual void CancelLoginsQuery(int handle); diff --git a/chrome/browser/password_manager/password_store_default.cc b/chrome/browser/password_manager/password_store_default.cc index c7e89c5..33895b5 100644 --- a/chrome/browser/password_manager/password_store_default.cc +++ b/chrome/browser/password_manager/password_store_default.cc @@ -13,21 +13,66 @@ PasswordStoreDefault::PasswordStoreDefault(WebDataService* web_data_service) : web_data_service_(web_data_service) { } -void PasswordStoreDefault::AddLoginImpl(const PasswordForm& form) { +PasswordStoreDefault::~PasswordStoreDefault() { + for (PendingRequestMap::const_iterator it = pending_requests_.begin(); + it != pending_requests_.end(); ++it) { + web_data_service_->CancelRequest(it->first); + } +} + +// Override all the public methods to do avoid passthroughs to the Impl +// versions. Since we are calling through to WebDataService, which is +// asynchronous, we'll still behave as the caller expects. +void PasswordStoreDefault::AddLogin(const PasswordForm& form) { web_data_service_->AddLogin(form); } -void PasswordStoreDefault::RemoveLoginImpl(const PasswordForm& form) { +void PasswordStoreDefault::UpdateLogin(const PasswordForm& form) { + web_data_service_->UpdateLogin(form); +} + +void PasswordStoreDefault::RemoveLogin(const PasswordForm& form) { web_data_service_->RemoveLogin(form); } +int PasswordStoreDefault::GetLogins(const PasswordForm& form, + PasswordStoreConsumer* consumer) { + int handle = handle_++; + GetLoginsRequest* request = new GetLoginsRequest(form, consumer, handle); + + int web_data_handle = web_data_service_->GetLogins(form, this); + pending_requests_.insert(PendingRequestMap::value_type(web_data_handle, + request)); + return handle; +} + +void PasswordStoreDefault::CancelLoginsQuery(int handle) { + for (PendingRequestMap::iterator it = pending_requests_.begin(); + it != pending_requests_.end(); ++it) { + GetLoginsRequest* request = it->second; + if (request->handle == handle) { + web_data_service_->CancelRequest(it->first); + delete request; + pending_requests_.erase(it); + return; + } + } +} + +void PasswordStoreDefault::AddLoginImpl(const PasswordForm& form) { + NOTREACHED(); +} + +void PasswordStoreDefault::RemoveLoginImpl(const PasswordForm& form) { + NOTREACHED(); +} + void PasswordStoreDefault::UpdateLoginImpl(const PasswordForm& form) { - web_data_service_->UpdateLogin(form); + NOTREACHED(); } void PasswordStoreDefault::GetLoginsImpl(GetLoginsRequest* request) { - int web_data_handle = web_data_service_->GetLogins(request->form, this); - AddPendingWebDataServiceRequest(web_data_handle, request); + NOTREACHED(); } void PasswordStoreDefault::OnWebDataServiceRequestDone( @@ -36,9 +81,11 @@ void PasswordStoreDefault::OnWebDataServiceRequestDone( // Look up this handle in our request map to get the original // GetLoginsRequest. PendingRequestMap::iterator it(pending_requests_.find(h)); - DCHECK(it != pending_requests_.end()); + // If the request was cancelled, we are done. + if (it == pending_requests_.end()) + return; - GetLoginsRequest* request = it->second; + scoped_ptr<GetLoginsRequest> request(it->second); pending_requests_.erase(it); DCHECK(result); @@ -48,36 +95,6 @@ void PasswordStoreDefault::OnWebDataServiceRequestDone( const WDResult<std::vector<PasswordForm*> >* r = static_cast<const WDResult<std::vector<PasswordForm*> >*>(result); - NotifyConsumer(request, r->GetValue()); - - RemovePendingWebDataServiceRequest(h); -} - -void PasswordStoreDefault::AddPendingWebDataServiceRequest( - WebDataService::Handle handle, GetLoginsRequest* request) { - pending_requests_.insert(PendingRequestMap::value_type(handle, request)); - - // WebDataService callbacks are non-retaining. Since there would be a race - // around cancelling the requests in the desctructor vs. getting a callback - // in this worker thread, just make sure that we stick around instead. - this->AddRef(); -} - -void PasswordStoreDefault::RemovePendingWebDataServiceRequest( - WebDataService::Handle handle) { - PendingRequestMap::iterator it(pending_requests_.find(handle)); - DCHECK(it != pending_requests_.end()); - pending_requests_.erase(it); - - // Balance the AddRef in AddPendingWebDataServiceRequest. - this->Release(); -} - -PasswordStore::GetLoginsRequest* - PasswordStoreDefault::GetLoginsRequestForWebDataServiceRequest( - WebDataService::Handle handle) { - PendingRequestMap::iterator it(pending_requests_.find(handle)); - if (it == pending_requests_.end()) - return NULL; - return it->second; + request->consumer->OnPasswordStoreRequestDone(request->handle, + r->GetValue()); } diff --git a/chrome/browser/password_manager/password_store_default.h b/chrome/browser/password_manager/password_store_default.h index c979da19..5529502 100644 --- a/chrome/browser/password_manager/password_store_default.h +++ b/chrome/browser/password_manager/password_store_default.h @@ -25,7 +25,16 @@ class PasswordStoreDefault : public PasswordStore, public WebDataServiceConsumer { public: explicit PasswordStoreDefault(WebDataService* web_data_service); - virtual ~PasswordStoreDefault() {} + virtual ~PasswordStoreDefault(); + + // Overridden to bypass the threading logic in PasswordStore, since + // WebDataService's API is not threadsafe. + virtual void AddLogin(const PasswordForm& form); + virtual void UpdateLogin(const PasswordForm& form); + virtual void RemoveLogin(const PasswordForm& form); + virtual int GetLogins(const PasswordForm& form, + PasswordStoreConsumer* consumer); + virtual void CancelLoginsQuery(int handle); protected: // Implements PasswordStore interface. @@ -35,32 +44,18 @@ class PasswordStoreDefault : public PasswordStore, void GetLoginsImpl(GetLoginsRequest* request); // Called when a WebDataService method finishes. - // Overrides must call RemovePendingWebDataServiceRequest. virtual void OnWebDataServiceRequestDone(WebDataService::Handle h, const WDTypedResult* result); - // Keeps track of a pending WebDataService request, and the original - // GetLogins request it is associated with. - // Increases the reference count of |this|, so must be balanced with a - // call to RemovePendingWebDataServiceRequest. - void AddPendingWebDataServiceRequest(WebDataService::Handle handle, - GetLoginsRequest* request); - // Removes a WebDataService request from the list of pending requests, - // and reduces the reference count of |this|. - void RemovePendingWebDataServiceRequest(WebDataService::Handle handle); - // Returns the GetLoginsRequest associated with |handle|. - GetLoginsRequest* GetLoginsRequestForWebDataServiceRequest( - WebDataService::Handle handle); - scoped_refptr<WebDataService> web_data_service_; - private: // Methods in this class call async WebDataService methods. This mapping // remembers which WebDataService request corresponds to which PasswordStore // request. typedef std::map<WebDataService::Handle, GetLoginsRequest*> PendingRequestMap; PendingRequestMap pending_requests_; + private: DISALLOW_COPY_AND_ASSIGN(PasswordStoreDefault); }; diff --git a/chrome/browser/password_manager/password_store_win.cc b/chrome/browser/password_manager/password_store_win.cc index 7eea14f..fa5e4ab 100644 --- a/chrome/browser/password_manager/password_store_win.cc +++ b/chrome/browser/password_manager/password_store_win.cc @@ -21,12 +21,13 @@ void PasswordStoreWin::OnWebDataServiceRequestDone( WebDataService::Handle h, const WDTypedResult *result) { // Look up this handle in our request map to get the original // GetLoginsRequest. - GetLoginsRequest* request = GetLoginsRequestForWebDataServiceRequest(h); - DCHECK(request); - // Remove our pending request, but make sure that we won't be destroyed until - // we're done with this function. - scoped_refptr<PasswordStoreWin> life_preserver(this); - RemovePendingWebDataServiceRequest(h); + PendingRequestMap::iterator it(pending_requests_.find(h)); + // If the request was cancelled, we are done. + if (it == pending_requests_.end()) + return; + + scoped_ptr<GetLoginsRequest> request(it->second); + pending_requests_.erase(it); DCHECK(result); if (!result) @@ -41,7 +42,8 @@ void PasswordStoreWin::OnWebDataServiceRequestDone( if (result_value.size()) { // If we found some results then return them now. - NotifyConsumer(request, result_value); + request->consumer->OnPasswordStoreRequestDone(request->handle, + result_value); return; } else { // Otherwise try finding IE7 logins. @@ -52,7 +54,8 @@ void PasswordStoreWin::OnWebDataServiceRequestDone( if (web_data_service_->IsRunning()) { WebDataService::Handle web_data_handle = web_data_service_->GetIE7Login(info, this); - AddPendingWebDataServiceRequest(web_data_handle, request); + pending_requests_.insert(PendingRequestMap::value_type( + web_data_handle, request.release())); } } break; @@ -66,7 +69,8 @@ void PasswordStoreWin::OnWebDataServiceRequestDone( if (ie7_form) forms.push_back(ie7_form); - NotifyConsumer(request, forms); + request->consumer->OnPasswordStoreRequestDone(request->handle, + forms); break; } } diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc index 513eb28..5c50259 100644 --- a/chrome/browser/profile.cc +++ b/chrome/browser/profile.cc @@ -19,6 +19,7 @@ #include "chrome/browser/extensions/user_script_master.h" #include "chrome/browser/history/history.h" #include "chrome/browser/net/chrome_url_request_context.h" +#include "chrome/browser/password_manager/password_store_default.h" #include "chrome/browser/profile_manager.h" #include "chrome/browser/renderer_host/render_process_host.h" #include "chrome/browser/search_engines/template_url_fetcher.h" @@ -90,6 +91,14 @@ URLRequestContext* Profile::GetDefaultRequestContext() { return default_request_context_; } +#if defined(OS_LINUX) +// Temporarily disabled while we figure some stuff out. +// http://code.google.com/p/chromium/issues/detail?id=12351 +// #include "chrome/browser/password_manager/password_store_gnome.h" +// #include "chrome/browser/password_manager/password_store_kwallet.h" +#elif defined(OS_WIN) +#include "chrome/browser/password_manager/password_store_win.h" +#endif //////////////////////////////////////////////////////////////////////////////// // @@ -191,6 +200,15 @@ class OffTheRecordProfileImpl : public Profile, } } + virtual PasswordStore* GetPasswordStore(ServiceAccessType sat) { + if (sat == EXPLICIT_ACCESS) { + return profile_->GetPasswordStore(sat); + } else { + NOTREACHED() << "This profile is OffTheRecord"; + return NULL; + } + } + virtual PrefService* GetPrefs() { return profile_->GetPrefs(); } @@ -391,6 +409,7 @@ ProfileImpl::ProfileImpl(const FilePath& path) extensions_request_context_(NULL), history_service_created_(false), created_web_data_service_(false), + created_password_store_(false), created_download_manager_(false), created_theme_provider_(false), start_time_(Time::Now()), @@ -759,6 +778,41 @@ void ProfileImpl::CreateWebDataService() { web_data_service_.swap(wds); } +PasswordStore* ProfileImpl::GetPasswordStore(ServiceAccessType sat) { + if (!created_password_store_) + CreatePasswordStore(); + return password_store_.get(); +} + +void ProfileImpl::CreatePasswordStore() { + DCHECK(!created_password_store_ && password_store_.get() == NULL); + created_password_store_ = true; + scoped_refptr<PasswordStore> ps; +#if defined(OS_LINUX) +// Temporarily disabled while we figure some stuff out. +// http://code.google.com/p/chromium/issues/detail?id=12351 +// if (getenv("KDE_FULL_SESSION")) { +// ps = new PasswordStoreKWallet(); +// } else { +// ps = new PasswordStoreGnome(); +// } + NOTIMPLEMENTED(); +#elif defined(OS_WIN) + ps = new PasswordStoreWin(GetWebDataService(Profile::IMPLICIT_ACCESS)); +#else + NOTIMPLEMENTED(); +#endif + if (!ps || !ps->Init()) { + // Try falling back to the default password manager + LOG(WARNING) << "Could not initialise native password manager - " + "falling back to default"; + ps = new PasswordStoreDefault(GetWebDataService(Profile::IMPLICIT_ACCESS)); + if (!ps->Init()) + return; + } + password_store_.swap(ps); +} + DownloadManager* ProfileImpl::GetDownloadManager() { if (!created_download_manager_) { scoped_refptr<DownloadManager> dlm(new DownloadManager); diff --git a/chrome/browser/profile.h b/chrome/browser/profile.h index 1ee1a19..d5e800f 100644 --- a/chrome/browser/profile.h +++ b/chrome/browser/profile.h @@ -31,6 +31,7 @@ class ExtensionProcessManager; class ExtensionsService; class HistoryService; class NavigationController; +class PasswordStore; class PrefService; class SessionService; class SpellChecker; @@ -157,6 +158,9 @@ class Profile { // the ServiceAccessType definition above. virtual WebDataService* GetWebDataService(ServiceAccessType access) = 0; + // Returns the PasswordStore for this profile. This is owned by the Profile. + virtual PasswordStore* GetPasswordStore(ServiceAccessType access) = 0; + // Retrieves a pointer to the PrefService that manages the preferences // for this user profile. The PrefService is lazily created the first // time that this method is called. @@ -314,6 +318,7 @@ class ProfileImpl : public Profile, virtual ExtensionProcessManager* GetExtensionProcessManager(); virtual HistoryService* GetHistoryService(ServiceAccessType sat); virtual WebDataService* GetWebDataService(ServiceAccessType sat); + virtual PasswordStore* GetPasswordStore(ServiceAccessType sat); virtual PrefService* GetPrefs(); virtual TemplateURLModel* GetTemplateURLModel(); virtual TemplateURLFetcher* GetTemplateURLFetcher(); @@ -359,6 +364,8 @@ class ProfileImpl : public Profile, void CreateWebDataService(); FilePath GetPrefFilePath(); + void CreatePasswordStore(); + void StopCreateSessionServiceTimer(); void EnsureSessionServiceCreated() { @@ -401,10 +408,12 @@ class ProfileImpl : public Profile, scoped_refptr<DownloadManager> download_manager_; scoped_refptr<HistoryService> history_service_; scoped_refptr<WebDataService> web_data_service_; + scoped_refptr<PasswordStore> password_store_; scoped_refptr<SessionService> session_service_; scoped_refptr<BrowserThemeProvider> theme_provider_; bool history_service_created_; bool created_web_data_service_; + bool created_password_store_; bool created_download_manager_; bool created_theme_provider_; // Whether or not the last session exited cleanly. This is set only once. diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index 552dd5b..65dd331 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -1102,9 +1102,23 @@ 'browser/password_manager/ie7_password.h', 'browser/password_manager/password_form_manager.cc', 'browser/password_manager/password_form_manager.h', - 'browser/password_manager/password_form_manager_win.cc', 'browser/password_manager/password_manager.cc', 'browser/password_manager/password_manager.h', + 'browser/password_manager/password_store.cc', + 'browser/password_manager/password_store.h', + 'browser/password_manager/password_store_default.cc', + 'browser/password_manager/password_store_default.h', + # Temporarily disabled while we figure some stuff out. + # http://code.google.com/p/chromium/issues/detail?id=12351 + # 'browser/password_manager/password_store_gnome.h', + # 'browser/password_manager/password_store_gnome.cc', + # 'browser/password_manager/password_store_kwallet.h', + # 'browser/password_manager/password_store_kwallet.cc', + 'browser/password_manager/password_store_mac_internal.h', + 'browser/password_manager/password_store_mac.h', + 'browser/password_manager/password_store_mac.cc', + 'browser/password_manager/password_store_win.h', + 'browser/password_manager/password_store_win.cc', 'browser/plugin_installer.cc', 'browser/plugin_installer.h', 'browser/plugin_process_host.cc', @@ -1570,6 +1584,10 @@ }], ['OS=="linux"', { 'dependencies': [ + # Temporarily disabled while we figure some stuff out. + # http://code.google.com/p/chromium/issues/detail?id=12351 + # '../build/linux/system.gyp:dbus-glib', + # '../build/linux/system.gyp:gnome-keyring', '../build/linux/system.gyp:gtk', ], 'sources!': [ @@ -1578,6 +1596,8 @@ 'browser/extensions/extension_view.cc', 'browser/extensions/extension_view.h', # Windows-specific files. + 'browser/password_manager/password_store_win.cc', + 'browser/password_manager/password_store_win.h', ], 'conditions': [ ['linux_breakpad==1', { @@ -1620,6 +1640,12 @@ 'browser/automation/automation_provider_list_generic.cc', 'browser/bookmarks/bookmark_context_menu.cc', 'browser/bookmarks/bookmark_drop_info.cc', + 'browser/password_manager/password_store_gnome.h', + 'browser/password_manager/password_store_gnome.cc', + 'browser/password_manager/password_store_kwallet.h', + 'browser/password_manager/password_store_kwallet.cc', + 'browser/password_manager/password_store_win.cc', + 'browser/password_manager/password_store_win.h', 'browser/extensions/extension_shelf.cc', 'browser/extensions/extension_shelf.h', 'browser/extensions/extension_view.cc', @@ -1672,6 +1698,10 @@ ], 'sources!': [ 'browser/history/history_publisher_none.cc', + 'browser/password_manager/password_store_gnome.h', + 'browser/password_manager/password_store_gnome.cc', + 'browser/password_manager/password_store_kwallet.h', + 'browser/password_manager/password_store_kwallet.cc', ], 'configurations': { 'Debug': { @@ -3138,6 +3168,7 @@ 'browser/net/url_fixer_upper_unittest.cc', 'browser/password_manager/encryptor_unittest.cc', 'browser/password_manager/password_form_manager_unittest.cc', + 'browser/password_manager/password_store_mac_unittest.cc', 'browser/printing/page_number_unittest.cc', 'browser/printing/page_overlays_unittest.cc', 'browser/printing/page_range_unittest.cc', diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h index 974d31d..7abdd8f 100644 --- a/chrome/test/testing_profile.h +++ b/chrome/test/testing_profile.h @@ -102,6 +102,9 @@ class TestingProfile : public Profile { virtual WebDataService* GetWebDataService(ServiceAccessType access) { return NULL; } + virtual PasswordStore* GetPasswordStore(ServiceAccessType access) { + return NULL; + } virtual PrefService* GetPrefs() { FilePath prefs_filename; PathService::Get(base::DIR_TEMP, &prefs_filename); |