diff options
author | gcasto@chromium.org <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-04 00:39:42 +0000 |
---|---|---|
committer | gcasto@chromium.org <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-04 00:39:42 +0000 |
commit | 6dcaeae807dffd3784b45625cf746bfd882f2828 (patch) | |
tree | f6d6fdcef9760c4062f0caa6f5cf1b2fbf67d675 /chrome/browser | |
parent | f6cb7cafbdde72f71c8b78179542d2392350b2ae (diff) | |
download | chromium_src-6dcaeae807dffd3784b45625cf746bfd882f2828.zip chromium_src-6dcaeae807dffd3784b45625cf746bfd882f2828.tar.gz chromium_src-6dcaeae807dffd3784b45625cf746bfd882f2828.tar.bz2 |
Enable password generation only if sync for passwords is enabled.
This is the second try at this change. The first failed because the
ProfileSyncService apparently does not always outlive the autofill_manager.
Fixed by using a WeakPtr.
BUG=114092
TEST=Ran memory try bots.
Review URL: https://chromiumcodereview.appspot.com/9950051
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130523 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/autofill/autofill_manager.cc | 45 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager.h | 28 | ||||
-rw-r--r-- | chrome/browser/autofill/autofill_manager_unittest.cc | 61 |
3 files changed, 134 insertions, 0 deletions
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc index 5b475a1..7cfb8de 100644 --- a/chrome/browser/autofill/autofill_manager.cc +++ b/chrome/browser/autofill/autofill_manager.cc @@ -35,6 +35,8 @@ #include "chrome/browser/infobars/infobar_tab_helper.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" +#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/browser_window.h" @@ -183,13 +185,17 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents) user_did_type_(false), user_did_autofill_(false), user_did_edit_autofilled_field_(false), + password_sync_enabled_(false), external_delegate_(NULL) { // |personal_data_| is NULL when using test-enabled WebContents. personal_data_ = PersonalDataManagerFactory::GetForProfile( tab_contents->profile()->GetOriginalProfile()); + RegisterWithSyncService(); } AutofillManager::~AutofillManager() { + if (sync_service_ && sync_service_->HasObserver(this)) + sync_service_->RemoveObserver(this); } // static @@ -214,6 +220,43 @@ void AutofillManager::RegisterUserPrefs(PrefService* prefs) { PrefService::UNSYNCABLE_PREF); } +void AutofillManager::RegisterWithSyncService() { + ProfileSyncService* temp_sync_service = + ProfileSyncServiceFactory::GetForProfile( + tab_contents_wrapper_->profile()); + if (temp_sync_service) { + sync_service_ = temp_sync_service->AsWeakPtr(); + sync_service_->AddObserver(this); + } +} + +void AutofillManager::SendPasswordSyncStateToRenderer( + content::RenderViewHost* host, bool enabled) { + host->Send(new AutofillMsg_PasswordSyncEnabled(host->GetRoutingID(), + enabled)); +} + +void AutofillManager::UpdatePasswordSyncState(content::RenderViewHost* host) { + if (!sync_service_) + return; + + syncable::ModelTypeSet sync_set = sync_service_->GetPreferredDataTypes(); + bool new_password_sync_enabled = (sync_service_->HasSyncSetupCompleted() && + sync_set.Has(syncable::PASSWORDS)); + if (new_password_sync_enabled != password_sync_enabled_) { + password_sync_enabled_ = new_password_sync_enabled; + SendPasswordSyncStateToRenderer(host, password_sync_enabled_); + } +} + +void AutofillManager::RenderViewCreated(content::RenderViewHost* host) { + UpdatePasswordSyncState(host); +} + +void AutofillManager::OnStateChanged() { + UpdatePasswordSyncState(web_contents()->GetRenderViewHost()); +} + void AutofillManager::DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) { @@ -756,8 +799,10 @@ AutofillManager::AutofillManager(TabContentsWrapper* tab_contents, user_did_type_(false), user_did_autofill_(false), user_did_edit_autofilled_field_(false), + password_sync_enabled_(false), external_delegate_(NULL) { DCHECK(tab_contents); + RegisterWithSyncService(); } void AutofillManager::set_metric_logger(const AutofillMetrics* metric_logger) { diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h index b750703..86203c5 100644 --- a/chrome/browser/autofill/autofill_manager.h +++ b/chrome/browser/autofill/autofill_manager.h @@ -17,11 +17,13 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" +#include "base/memory/weak_ptr.h" #include "base/string16.h" #include "base/time.h" #include "chrome/browser/autofill/autofill_download.h" #include "chrome/browser/autofill/field_types.h" #include "chrome/browser/autofill/form_structure.h" +#include "chrome/browser/sync/profile_sync_service_observer.h" #include "content/public/browser/web_contents_observer.h" class AutofillExternalDelegate; @@ -31,6 +33,7 @@ class AutofillMetrics; class CreditCard; class PersonalDataManager; class PrefService; +class ProfileSyncService; class TabContentsWrapper; struct ViewHostMsg_FrameNavigate_Params; @@ -58,6 +61,7 @@ struct FormField; // forms. class AutofillManager : public content::WebContentsObserver, public AutofillDownloadManager::Observer, + public ProfileSyncServiceObserver, public base::RefCounted<AutofillManager> { public: explicit AutofillManager(TabContentsWrapper* tab_contents); @@ -107,6 +111,11 @@ class AutofillManager : public content::WebContentsObserver, // Reset cache. void Reset(); + // Informs the renderers of the current password sync state for use in + // password generation. This is a separate function to aid with testing. + virtual void SendPasswordSyncStateToRenderer(content::RenderViewHost* host, + bool enabled); + // Logs quality metrics for the |submitted_form| and uploads the form data // to the crowdsourcing server, if appropriate. virtual void UploadFormDataAsyncCallback( @@ -143,6 +152,7 @@ class AutofillManager : public content::WebContentsObserver, private: // content::WebContentsObserver: + virtual void RenderViewCreated(content::RenderViewHost* host) OVERRIDE; virtual void DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) OVERRIDE; @@ -152,6 +162,17 @@ class AutofillManager : public content::WebContentsObserver, virtual void OnLoadedServerPredictions( const std::string& response_xml) OVERRIDE; + // ProfileSyncServiceObserver: + virtual void OnStateChanged() OVERRIDE; + + // Register as an observer with the sync service. + void RegisterWithSyncService(); + + // Determines what the current state of password sync is, and if it has + // changed from password_sync_enabled_. If it has changed, it notifies + // the renderers of this change via SendPasswordSyncStateToRenderer. + void UpdatePasswordSyncState(content::RenderViewHost* host); + void OnFormsSeen(const std::vector<webkit::forms::FormData>& forms, const base::TimeTicks& timestamp); void OnTextFieldDidChange(const webkit::forms::FormData& form, @@ -303,6 +324,13 @@ class AutofillManager : public content::WebContentsObserver, // When the user first interacted with a potentially fillable form on this // page. base::TimeTicks initial_interaction_timestamp_; + // If password sync is enabled. We cache this value so that we don't + // spam the renderer with messages during startup when the sync state + // is changing rapidly. + bool password_sync_enabled_; + // The ProfileSyncService associated with this tab. This may be NULL in + // testing. + base::WeakPtr<ProfileSyncService> sync_service_; // Our copy of the form data. ScopedVector<FormStructure> form_structures_; diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc index a50cff2..8695d44 100644 --- a/chrome/browser/autofill/autofill_manager_unittest.cc +++ b/chrome/browser/autofill/autofill_manager_unittest.cc @@ -23,6 +23,7 @@ #include "chrome/browser/autofill/test_autofill_external_delegate.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h" #include "chrome/browser/ui/tab_contents/test_tab_contents_wrapper.h" @@ -486,6 +487,19 @@ class TestAutofillManager : public AutofillManager { submitted_form_signature_ = submitted_form.FormSignature(); } + virtual void SendPasswordSyncStateToRenderer( + content::RenderViewHost* host, bool enabled) OVERRIDE { + sent_states_.push_back(enabled); + } + + const std::vector<bool>& GetSentStates() { + return sent_states_; + } + + void ClearSentStates() { + sent_states_.clear(); + } + const std::string GetSubmittedFormSignature() { return submitted_form_signature_; } @@ -527,6 +541,7 @@ class TestAutofillManager : public AutofillManager { std::string submitted_form_signature_; std::vector<FieldTypeSet> expected_submitted_field_types_; + std::vector<bool> sent_states_; DISALLOW_COPY_AND_ASSIGN(TestAutofillManager); }; @@ -567,6 +582,10 @@ class AutofillManagerTest : public TabContentsWrapperTestHarness { TabContentsWrapperTestHarness::TearDown(); } + void UpdatePasswordSyncState() { + autofill_manager_->UpdatePasswordSyncState(NULL); + } + void GetAutofillSuggestions(int query_id, const webkit::forms::FormData& form, const webkit::forms::FormField& field) { @@ -653,6 +672,10 @@ class AutofillManagerTest : public TabContentsWrapperTestHarness { return true; } + ProfileSyncService* GetProfileSyncService() { + return autofill_manager_->sync_service_; + } + protected: content::TestBrowserThread ui_thread_; content::TestBrowserThread file_thread_; @@ -2862,6 +2885,44 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesForUpload) { FormSubmitted(form); } +TEST_F(AutofillManagerTest, UpdatePasswordSyncState) { + // Allow this test to control what should get synced. + profile()->GetPrefs()->SetBoolean(prefs::kSyncKeepEverythingSynced, false); + + // Sync some things, but not passwords. Shouldn't send anything since + // password generation is disabled by default. + ProfileSyncService* sync_service = GetProfileSyncService(); + sync_service->SetSyncSetupCompleted(); + syncable::ModelTypeSet preferred_set; + preferred_set.Put(syncable::EXTENSIONS); + preferred_set.Put(syncable::PREFERENCES); + sync_service->ChangePreferredDataTypes(preferred_set); + syncable::ModelTypeSet new_set = sync_service->GetPreferredDataTypes(); + UpdatePasswordSyncState(); + EXPECT_EQ(0u, autofill_manager_->GetSentStates().size()); + + // Now sync passwords. + preferred_set.Put(syncable::PASSWORDS); + sync_service->ChangePreferredDataTypes(preferred_set); + UpdatePasswordSyncState(); + EXPECT_EQ(1u, autofill_manager_->GetSentStates().size()); + EXPECT_EQ(true, autofill_manager_->GetSentStates()[0]); + autofill_manager_->ClearSentStates(); + + // Add some additional synced state. Nothing should be sent. + preferred_set.Put(syncable::THEMES); + sync_service->ChangePreferredDataTypes(preferred_set); + UpdatePasswordSyncState(); + EXPECT_EQ(0u, autofill_manager_->GetSentStates().size()); + + // Disable syncing. This should be sent. + sync_service->DisableForUser(); + UpdatePasswordSyncState(); + EXPECT_EQ(1u, autofill_manager_->GetSentStates().size()); + EXPECT_EQ(false, autofill_manager_->GetSentStates()[0]); + autofill_manager_->ClearSentStates(); +} + namespace { class MockAutofillExternalDelegate : public TestAutofillExternalDelegate { |