diff options
| author | cpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-08 12:33:46 +0000 |
|---|---|---|
| committer | cpu@chromium.org <cpu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-08 12:33:46 +0000 |
| commit | e4d0ed2731bd622c36ded53ae2c9957f6bfc4241 (patch) | |
| tree | 8eb674b89678c38605c52f5e4ff6125100db8ab9 /components/autofill/browser | |
| parent | 99dbb184ea5182e07a738a9c901494ccb970d01f (diff) | |
| download | chromium_src-e4d0ed2731bd622c36ded53ae2c9957f6bfc4241.zip chromium_src-e4d0ed2731bd622c36ded53ae2c9957f6bfc4241.tar.gz chromium_src-e4d0ed2731bd622c36ded53ae2c9957f6bfc4241.tar.bz2 | |
Revert 205038 "Un-refcount AutofillWebData and TokenWebData"
Not sure if this is the culprit but lots of bots are red
unittest runs in linux mac and chromeos are not completing (timeout)
no output. Only for CL common to all, this is one.
I'll unrevert if it does not help.
> Un-refcount AutofillWebData and TokenWebData
>
> depends on https://codereview.chromium.org/15927029/
>
> BUG=230920
>
> Review URL: https://chromiumcodereview.appspot.com/16154031
TBR=caitkp@chromium.org
Review URL: https://codereview.chromium.org/15937020
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205055 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'components/autofill/browser')
7 files changed, 92 insertions, 74 deletions
diff --git a/components/autofill/browser/autocomplete_history_manager.cc b/components/autofill/browser/autocomplete_history_manager.cc index 7ad0cc3..f3f6f3f 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_) { + if (autofill_data_.get()) { 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_) + if (!values.empty() && autofill_data_.get()) autofill_data_->AddFormFields(values); } void AutocompleteHistoryManager::OnRemoveAutocompleteEntry( const base::string16& name, const base::string16& value) { - if (autofill_data_) + if (autofill_data_.get()) autofill_data_->RemoveFormValueForElementName(name, value); } @@ -170,7 +170,7 @@ void AutocompleteHistoryManager::SetExternalDelegate( void AutocompleteHistoryManager::CancelPendingQuery() { if (pending_query_handle_) { - if (autofill_data_) + if (autofill_data_.get()) 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 4bba83a..9a55fa4 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_; - AutofillWebDataService* autofill_data_; + scoped_refptr<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 cc6707f..57da55a 100644 --- a/components/autofill/browser/autocomplete_history_manager_unittest.cc +++ b/components/autofill/browser/autocomplete_history_manager_unittest.cc @@ -38,11 +38,43 @@ class MockWebDataService : public AutofillWebDataService { public: MockWebDataService() : AutofillWebDataService() { + current_mock_web_data_service_ = this; } MOCK_METHOD1(AddFormFields, void(const std::vector<FormFieldData>&)); + static scoped_refptr<MockWebDataService> 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<AutofillWebDataService> GetAutofillWebData() OVERRIDE { + return MockWebDataService::GetCurrent(); + } + + private: + DISALLOW_COPY_AND_ASSIGN(MockWebDataServiceWrapperCurrent); }; class MockAutofillManagerDelegate @@ -58,37 +90,25 @@ 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(); - MockWebDataServiceWrapper* wrapper = - static_cast<MockWebDataServiceWrapper*>( - WebDataServiceFactory::GetInstance()->SetTestingFactoryAndUse( - profile(), BuildMockWebDataServiceWrapper)); - web_data_service_ = - static_cast<MockWebDataService*>(wrapper->GetAutofillWebData()); + web_data_service_ = new MockWebDataService(); + WebDataServiceFactory::GetInstance()->SetTestingFactory( + profile(), MockWebDataServiceWrapperCurrent::Build); autocomplete_manager_.reset(new AutocompleteHistoryManager(web_contents())); } virtual void TearDown() OVERRIDE { autocomplete_manager_.reset(); - web_data_service_->ShutdownOnUIThread(); web_data_service_ = NULL; ChromeRenderViewHostTestHarness::TearDown(); } - MockWebDataService* web_data_service_; + scoped_refptr<MockWebDataService> web_data_service_; scoped_ptr<AutocompleteHistoryManager> autocomplete_manager_; MockAutofillManagerDelegate manager_delegate; }; @@ -110,7 +130,7 @@ TEST_F(AutocompleteHistoryManagerTest, CreditCardNumberValue) { valid_cc.form_control_type = "text"; form.fields.push_back(valid_cc); - EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0); + EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0); autocomplete_manager_->OnFormSubmitted(form); } @@ -133,7 +153,7 @@ TEST_F(AutocompleteHistoryManagerTest, NonCreditCardNumberValue) { invalid_cc.form_control_type = "text"; form.fields.push_back(invalid_cc); - EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1); + EXPECT_CALL(*(web_data_service_.get()), AddFormFields(_)).Times(1); autocomplete_manager_->OnFormSubmitted(form); } @@ -153,7 +173,7 @@ TEST_F(AutocompleteHistoryManagerTest, SSNValue) { ssn.form_control_type = "text"; form.fields.push_back(ssn); - EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0); + EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0); autocomplete_manager_->OnFormSubmitted(form); } @@ -174,7 +194,7 @@ TEST_F(AutocompleteHistoryManagerTest, SearchField) { search_field.form_control_type = "search"; form.fields.push_back(search_field); - EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1); + EXPECT_CALL(*(web_data_service_.get()), 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 e2afca4..a399548 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()); - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); // WebDataService may not be available in tests. - if (!autofill_data) + if (!autofill_data.get()) return; LoadProfiles(); @@ -166,9 +166,9 @@ PersonalDataManager::~PersonalDataManager() { if (!browser_context_) return; - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (autofill_data.get()) autofill_data->RemoveObserver(this); } @@ -375,9 +375,9 @@ void PersonalDataManager::AddProfile(const AutofillProfile& profile) { if (FindByGUID<AutofillProfile>(web_profiles_, profile.guid())) return; - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; // Don't add a duplicate. @@ -408,9 +408,9 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) { return; } - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; // Make the update. @@ -441,9 +441,9 @@ void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) { if (FindByGUID<CreditCard>(credit_cards_, credit_card.guid())) return; - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; // Don't add a duplicate. @@ -474,9 +474,9 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) { return; } - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; // Make the update. @@ -496,9 +496,9 @@ void PersonalDataManager::RemoveByGUID(const std::string& guid) { if (!is_credit_card && !is_profile) return; - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; if (is_credit_card) @@ -784,9 +784,9 @@ void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) { address_of<AutofillProfile>); AutofillProfile::AdjustInferredLabels(&profile_pointers); - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) return; // Any profiles that are not in the new profile list should be removed from @@ -838,9 +838,9 @@ void PersonalDataManager::SetCreditCards( it++; } - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) 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() { - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) { + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) { NOTREACHED(); return; } @@ -898,9 +898,9 @@ void PersonalDataManager::LoadAuxiliaryProfiles() { #endif void PersonalDataManager::LoadCreditCards() { - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) { + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) { NOTREACHED(); return; } @@ -949,9 +949,9 @@ void PersonalDataManager::ReceiveLoadedCreditCards( void PersonalDataManager::CancelPendingQuery( WebDataServiceBase::Handle* handle) { if (*handle) { - AutofillWebDataService* autofill_data = - AutofillWebDataService::FromBrowserContext(browser_context_); - if (!autofill_data) { + scoped_refptr<AutofillWebDataService> autofill_data( + AutofillWebDataService::FromBrowserContext(browser_context_)); + if (!autofill_data.get()) { NOTREACHED(); return; } diff --git a/components/autofill/browser/personal_data_manager_unittest.cc b/components/autofill/browser/personal_data_manager_unittest.cc index 1a4ffd5..6c20883 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); - AutofillWebDataService* wds = + scoped_refptr<AutofillWebDataService> wds = AutofillWebDataService::FromBrowserContext(profile_.get()); - ASSERT_TRUE(wds); + ASSERT_TRUE(wds.get()); 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 a85896d..b861c64 100644 --- a/components/autofill/browser/webdata/autofill_webdata_service.h +++ b/components/autofill/browser/webdata/autofill_webdata_service.h @@ -43,11 +43,9 @@ class AutofillWebDataService : public AutofillWebData, AutofillWebDataService(scoped_refptr<WebDatabaseService> wdbs, const ProfileErrorCallback& callback); - virtual ~AutofillWebDataService(); - // Retrieve an AutofillWebDataService for the given context. // Can return NULL in some contexts. - static AutofillWebDataService* FromBrowserContext( + static scoped_refptr<AutofillWebDataService> FromBrowserContext( content::BrowserContext* context); // WebDataServiceBase implementation. @@ -103,6 +101,8 @@ class AutofillWebDataService : public AutofillWebData, const base::Callback<void(AutofillWebDataBackend*)>& callback); protected: + virtual ~AutofillWebDataService(); + virtual void NotifyAutofillMultipleChangedOnUIThread(); base::WeakPtr<AutofillWebDataService> AsWeakPtr() { diff --git a/components/autofill/browser/webdata/web_data_service_unittest.cc b/components/autofill/browser/webdata/web_data_service_unittest.cc index b12326b..128ac1c 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<WebDatabaseTable>(new AutofillTable("en-US"))); wdbs_->LoadDatabase(); - wds_.reset(new AutofillWebDataService( - wdbs_, WebDataServiceBase::ProfileErrorCallback())); + wds_ = new AutofillWebDataService( + wdbs_, WebDataServiceBase::ProfileErrorCallback()); wds_->Init(); } virtual void TearDown() { wds_->ShutdownOnUIThread(); wdbs_->ShutdownDatabase(); - wds_.reset(); + wds_ = NULL; 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_ptr<AutofillWebDataService> wds_; + scoped_refptr<AutofillWebDataService> wds_; scoped_refptr<WebDatabaseService> wdbs_; base::ScopedTempDir temp_dir_; }; @@ -135,8 +135,7 @@ class WebDataServiceAutofillTest : public WebDataServiceTest { BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(add_observer_func, - base::Unretained(wds_.get()), &observer_)); + base::Bind(add_observer_func, wds_, &observer_)); WaitForDatabaseThread(); } @@ -147,8 +146,7 @@ class WebDataServiceAutofillTest : public WebDataServiceTest { BrowserThread::PostTask( BrowserThread::DB, FROM_HERE, - base::Bind(remove_observer_func, - base::Unretained(wds_.get()), &observer_)); + base::Bind(remove_observer_func, wds_, &observer_)); WaitForDatabaseThread(); WebDataServiceTest::TearDown(); |