From 50ab33e2b381c27152fb5eb8e0ef1385199d2865 Mon Sep 17 00:00:00 2001 From: "caitkp@chromium.org" Date: Tue, 11 Jun 2013 23:11:13 +0000 Subject: Un-refcount AutofillWebData and TokenWebData depends on https://codereview.chromium.org/15927029/ BUG=230920 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205038 Review URL: https://chromiumcodereview.appspot.com/16154031 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205670 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/autocomplete_history_manager.cc | 8 +-- .../browser/autocomplete_history_manager.h | 2 +- .../autocomplete_history_manager_unittest.cc | 60 ++++++------------ .../autofill/browser/personal_data_manager.cc | 72 +++++++++++----------- .../browser/personal_data_manager_unittest.cc | 4 +- .../browser/webdata/autofill_webdata_service.h | 6 +- .../browser/webdata/web_data_service_unittest.cc | 14 +++-- 7 files changed, 74 insertions(+), 92 deletions(-) (limited to 'components/autofill/browser') diff --git a/components/autofill/browser/autocomplete_history_manager.cc b/components/autofill/browser/autocomplete_history_manager.cc index 2f6ce63..2ff85ca 100644 --- a/components/autofill/browser/autocomplete_history_manager.cc +++ b/components/autofill/browser/autocomplete_history_manager.cc @@ -117,7 +117,7 @@ void AutocompleteHistoryManager::OnGetAutocompleteSuggestions( return; } - if (autofill_data_.get()) { + if (autofill_data_) { pending_query_handle_ = autofill_data_->GetFormValuesForElementName( name, prefix, kMaxAutocompleteMenuItems, this); } @@ -153,13 +153,13 @@ void AutocompleteHistoryManager::OnFormSubmitted(const FormData& form) { } } - if (!values.empty() && autofill_data_.get()) + if (!values.empty() && autofill_data_) autofill_data_->AddFormFields(values); } void AutocompleteHistoryManager::OnRemoveAutocompleteEntry( const base::string16& name, const base::string16& value) { - if (autofill_data_.get()) + if (autofill_data_) autofill_data_->RemoveFormValueForElementName(name, value); } @@ -170,7 +170,7 @@ void AutocompleteHistoryManager::SetExternalDelegate( void AutocompleteHistoryManager::CancelPendingQuery() { if (pending_query_handle_) { - if (autofill_data_.get()) + if (autofill_data_) autofill_data_->CancelRequest(pending_query_handle_); pending_query_handle_ = 0; } diff --git a/components/autofill/browser/autocomplete_history_manager.h b/components/autofill/browser/autocomplete_history_manager.h index 9a55fa4..4bba83a 100644 --- a/components/autofill/browser/autocomplete_history_manager.h +++ b/components/autofill/browser/autocomplete_history_manager.h @@ -69,7 +69,7 @@ class AutocompleteHistoryManager : public content::WebContentsObserver, void CancelPendingQuery(); content::BrowserContext* browser_context_; - scoped_refptr autofill_data_; + AutofillWebDataService* autofill_data_; BooleanPrefMember autofill_enabled_; diff --git a/components/autofill/browser/autocomplete_history_manager_unittest.cc b/components/autofill/browser/autocomplete_history_manager_unittest.cc index 0a3680d..39d1c58 100644 --- a/components/autofill/browser/autocomplete_history_manager_unittest.cc +++ b/components/autofill/browser/autocomplete_history_manager_unittest.cc @@ -38,43 +38,11 @@ class MockWebDataService : public AutofillWebDataService { public: MockWebDataService() : AutofillWebDataService() { - current_mock_web_data_service_ = this; } MOCK_METHOD1(AddFormFields, void(const std::vector&)); - static scoped_refptr GetCurrent() { - if (!current_mock_web_data_service_) { - return new MockWebDataService(); - } - return current_mock_web_data_service_; - } - - protected: virtual ~MockWebDataService() {} - - private: - // Keep track of the most recently created instance, so that it can be - // associated with the current profile when Build() is called. - static MockWebDataService* current_mock_web_data_service_; -}; - -MockWebDataService* MockWebDataService::current_mock_web_data_service_ = NULL; - -class MockWebDataServiceWrapperCurrent : public MockWebDataServiceWrapperBase { - public: - static BrowserContextKeyedService* Build(content::BrowserContext* profile) { - return new MockWebDataServiceWrapperCurrent(); - } - - MockWebDataServiceWrapperCurrent() {} - - virtual scoped_refptr GetAutofillWebData() OVERRIDE { - return MockWebDataService::GetCurrent(); - } - - private: - DISALLOW_COPY_AND_ASSIGN(MockWebDataServiceWrapperCurrent); }; class MockAutofillManagerDelegate @@ -90,25 +58,37 @@ class MockAutofillManagerDelegate DISALLOW_COPY_AND_ASSIGN(MockAutofillManagerDelegate); }; +BrowserContextKeyedService* BuildMockWebDataServiceWrapper( + content::BrowserContext* profile) { + return new MockWebDataServiceWrapper( + NULL, + new MockWebDataService(), + NULL); +} + } // namespace class AutocompleteHistoryManagerTest : public ChromeRenderViewHostTestHarness { protected: virtual void SetUp() OVERRIDE { ChromeRenderViewHostTestHarness::SetUp(); - web_data_service_ = new MockWebDataService(); - WebDataServiceFactory::GetInstance()->SetTestingFactory( - profile(), MockWebDataServiceWrapperCurrent::Build); + MockWebDataServiceWrapper* wrapper = + static_cast( + WebDataServiceFactory::GetInstance()->SetTestingFactoryAndUse( + profile(), BuildMockWebDataServiceWrapper)); + web_data_service_ = + static_cast(wrapper->GetAutofillWebData()); autocomplete_manager_.reset(new AutocompleteHistoryManager(web_contents())); } virtual void TearDown() OVERRIDE { autocomplete_manager_.reset(); + web_data_service_->ShutdownOnUIThread(); web_data_service_ = NULL; ChromeRenderViewHostTestHarness::TearDown(); } - scoped_refptr web_data_service_; + MockWebDataService* web_data_service_; scoped_ptr autocomplete_manager_; MockAutofillManagerDelegate manager_delegate; }; @@ -130,7 +110,7 @@ TEST_F(AutocompleteHistoryManagerTest, CreditCardNumberValue) { valid_cc.form_control_type = "text"; form.fields.push_back(valid_cc); - EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0); + EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0); autocomplete_manager_->OnFormSubmitted(form); } @@ -153,7 +133,7 @@ TEST_F(AutocompleteHistoryManagerTest, NonCreditCardNumberValue) { invalid_cc.form_control_type = "text"; form.fields.push_back(invalid_cc); - EXPECT_CALL(*(web_data_service_.get()), AddFormFields(_)).Times(1); + EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1); autocomplete_manager_->OnFormSubmitted(form); } @@ -173,7 +153,7 @@ TEST_F(AutocompleteHistoryManagerTest, SSNValue) { ssn.form_control_type = "text"; form.fields.push_back(ssn); - EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0); + EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0); autocomplete_manager_->OnFormSubmitted(form); } @@ -194,7 +174,7 @@ TEST_F(AutocompleteHistoryManagerTest, SearchField) { search_field.form_control_type = "search"; form.fields.push_back(search_field); - EXPECT_CALL(*(web_data_service_.get()), AddFormFields(_)).Times(1); + EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1); autocomplete_manager_->OnFormSubmitted(form); } diff --git a/components/autofill/browser/personal_data_manager.cc b/components/autofill/browser/personal_data_manager.cc index 6a7fb1f..07e5527 100644 --- a/components/autofill/browser/personal_data_manager.cc +++ b/components/autofill/browser/personal_data_manager.cc @@ -146,11 +146,11 @@ void PersonalDataManager::Init(BrowserContext* browser_context) { if (!browser_context_->IsOffTheRecord()) metric_logger_->LogIsAutofillEnabledAtStartup(IsAutofillEnabled()); - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); // WebDataService may not be available in tests. - if (!autofill_data.get()) + if (!autofill_data) return; LoadProfiles(); @@ -166,9 +166,9 @@ PersonalDataManager::~PersonalDataManager() { if (!browser_context_) return; - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (autofill_data) autofill_data->RemoveObserver(this); } @@ -375,9 +375,9 @@ void PersonalDataManager::AddProfile(const AutofillProfile& profile) { if (FindByGUID(web_profiles_, profile.guid())) return; - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Don't add a duplicate. @@ -408,9 +408,9 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) { return; } - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Make the update. @@ -441,9 +441,9 @@ void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) { if (FindByGUID(credit_cards_, credit_card.guid())) return; - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Don't add a duplicate. @@ -474,9 +474,9 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) { return; } - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Make the update. @@ -496,9 +496,9 @@ void PersonalDataManager::RemoveByGUID(const std::string& guid) { if (!is_credit_card && !is_profile) return; - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; if (is_credit_card) @@ -784,9 +784,9 @@ void PersonalDataManager::SetProfiles(std::vector* profiles) { address_of); AutofillProfile::AdjustInferredLabels(&profile_pointers); - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Any profiles that are not in the new profile list should be removed from @@ -838,9 +838,9 @@ void PersonalDataManager::SetCreditCards( it++; } - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) return; // Any credit cards that are not in the new credit card list should be @@ -878,9 +878,9 @@ void PersonalDataManager::SetCreditCards( } void PersonalDataManager::LoadProfiles() { - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) { + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) { NOTREACHED(); return; } @@ -898,9 +898,9 @@ void PersonalDataManager::LoadAuxiliaryProfiles() { #endif void PersonalDataManager::LoadCreditCards() { - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) { + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) { NOTREACHED(); return; } @@ -949,9 +949,9 @@ void PersonalDataManager::ReceiveLoadedCreditCards( void PersonalDataManager::CancelPendingQuery( WebDataServiceBase::Handle* handle) { if (*handle) { - scoped_refptr autofill_data( - AutofillWebDataService::FromBrowserContext(browser_context_)); - if (!autofill_data.get()) { + AutofillWebDataService* autofill_data = + AutofillWebDataService::FromBrowserContext(browser_context_); + if (!autofill_data) { NOTREACHED(); return; } diff --git a/components/autofill/browser/personal_data_manager_unittest.cc b/components/autofill/browser/personal_data_manager_unittest.cc index 6c20883..1a4ffd5 100644 --- a/components/autofill/browser/personal_data_manager_unittest.cc +++ b/components/autofill/browser/personal_data_manager_unittest.cc @@ -539,9 +539,9 @@ TEST_F(PersonalDataManagerTest, Refresh) { profile_pointers.push_back(&profile2); AutofillProfile::AdjustInferredLabels(&profile_pointers); - scoped_refptr wds = + AutofillWebDataService* wds = AutofillWebDataService::FromBrowserContext(profile_.get()); - ASSERT_TRUE(wds.get()); + ASSERT_TRUE(wds); wds->AddAutofillProfile(profile2); personal_data_->Refresh(); diff --git a/components/autofill/browser/webdata/autofill_webdata_service.h b/components/autofill/browser/webdata/autofill_webdata_service.h index b861c64..a85896d 100644 --- a/components/autofill/browser/webdata/autofill_webdata_service.h +++ b/components/autofill/browser/webdata/autofill_webdata_service.h @@ -43,9 +43,11 @@ class AutofillWebDataService : public AutofillWebData, AutofillWebDataService(scoped_refptr wdbs, const ProfileErrorCallback& callback); + virtual ~AutofillWebDataService(); + // Retrieve an AutofillWebDataService for the given context. // Can return NULL in some contexts. - static scoped_refptr FromBrowserContext( + static AutofillWebDataService* FromBrowserContext( content::BrowserContext* context); // WebDataServiceBase implementation. @@ -101,8 +103,6 @@ class AutofillWebDataService : public AutofillWebData, const base::Callback& callback); protected: - virtual ~AutofillWebDataService(); - virtual void NotifyAutofillMultipleChangedOnUIThread(); base::WeakPtr AsWeakPtr() { diff --git a/components/autofill/browser/webdata/web_data_service_unittest.cc b/components/autofill/browser/webdata/web_data_service_unittest.cc index ed64064..fefb8bb 100644 --- a/components/autofill/browser/webdata/web_data_service_unittest.cc +++ b/components/autofill/browser/webdata/web_data_service_unittest.cc @@ -76,15 +76,15 @@ class WebDataServiceTest : public testing::Test { wdbs_->AddTable(scoped_ptr(new AutofillTable("en-US"))); wdbs_->LoadDatabase(); - wds_ = new AutofillWebDataService( - wdbs_, WebDataServiceBase::ProfileErrorCallback()); + wds_.reset(new AutofillWebDataService( + wdbs_, WebDataServiceBase::ProfileErrorCallback())); wds_->Init(); } virtual void TearDown() { wds_->ShutdownOnUIThread(); wdbs_->ShutdownDatabase(); - wds_ = NULL; + wds_.reset(); wdbs_ = NULL; WaitForDatabaseThread(); @@ -107,7 +107,7 @@ class WebDataServiceTest : public testing::Test { content::TestBrowserThread ui_thread_; content::TestBrowserThread db_thread_; base::FilePath profile_dir_; - scoped_refptr wds_; + scoped_ptr wds_; scoped_refptr wdbs_; base::ScopedTempDir temp_dir_; }; @@ -135,7 +135,8 @@ class WebDataServiceAutofillTest : public WebDataServiceTest { BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(add_observer_func, wds_, &observer_)); + base::Bind(add_observer_func, + base::Unretained(wds_.get()), &observer_)); WaitForDatabaseThread(); } @@ -146,7 +147,8 @@ class WebDataServiceAutofillTest : public WebDataServiceTest { BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(remove_observer_func, wds_, &observer_)); + base::Bind(remove_observer_func, + base::Unretained(wds_.get()), &observer_)); WaitForDatabaseThread(); WebDataServiceTest::TearDown(); -- cgit v1.1