diff options
3 files changed, 33 insertions, 14 deletions
diff --git a/chrome/browser/search_engines/search_provider_install_data.cc b/chrome/browser/search_engines/search_provider_install_data.cc index b137ece..10ac1c2 100644 --- a/chrome/browser/search_engines/search_provider_install_data.cc +++ b/chrome/browser/search_engines/search_provider_install_data.cc @@ -9,6 +9,7 @@ #include "base/basictypes.h" #include "base/logging.h" #include "base/task.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/browser/search_engines/search_host_to_urls_map.h" #include "chrome/browser/search_engines/search_terms_data.h" #include "chrome/browser/search_engines/template_url.h" @@ -72,6 +73,8 @@ SearchProviderInstallData::SearchProviderInstallData( } SearchProviderInstallData::~SearchProviderInstallData() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + if (load_handle_) { DCHECK(web_service_.get()); web_service_->CancelRequest(load_handle_); @@ -79,6 +82,8 @@ SearchProviderInstallData::~SearchProviderInstallData() { } void SearchProviderInstallData::CallWhenLoaded(Task* task) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + if (provider_map_.get()) { task->Run(); delete task; @@ -97,6 +102,7 @@ void SearchProviderInstallData::CallWhenLoaded(Task* task) { SearchProviderInstallData::State SearchProviderInstallData::GetInstallState( const GURL& requested_origin) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); DCHECK(provider_map_.get()); // First check to see if the origin is the default search provider. @@ -122,6 +128,8 @@ SearchProviderInstallData::State SearchProviderInstallData::GetInstallState( void SearchProviderInstallData::OnWebDataServiceRequestDone( WebDataService::Handle h, const WDTypedResult* result) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + // Reset the load_handle so that we don't try and cancel the load in // the destructor. load_handle_ = 0; @@ -153,6 +161,8 @@ void SearchProviderInstallData::OnWebDataServiceRequestDone( } void SearchProviderInstallData::SetDefault(const TemplateURL* template_url) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + if (!template_url) { default_search_origin_.clear(); return; @@ -169,6 +179,8 @@ void SearchProviderInstallData::SetDefault(const TemplateURL* template_url) { } void SearchProviderInstallData::OnLoadFailed() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + provider_map_.reset(new SearchHostToURLsMap()); IOThreadSearchTermsData search_terms_data; provider_map_->Init(template_urls_.get(), search_terms_data); @@ -177,6 +189,8 @@ void SearchProviderInstallData::OnLoadFailed() { } void SearchProviderInstallData::NotifyLoaded() { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + task_queue_.Run(); // Since we expect this request to be rare, clear out the information. This diff --git a/chrome/browser/search_engines/search_provider_install_data_unittest.cc b/chrome/browser/search_engines/search_provider_install_data_unittest.cc index 1989a4c..f64f671 100644 --- a/chrome/browser/search_engines/search_provider_install_data_unittest.cc +++ b/chrome/browser/search_engines/search_provider_install_data_unittest.cc @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "base/message_loop.h" #include "base/ref_counted.h" +#include "base/task.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/search_engines/search_provider_install_data.h" #include "chrome/browser/search_engines/template_url.h" @@ -28,8 +29,8 @@ static TemplateURL* CreateTemplateURL(const std::string& url, class TestGetInstallState : public base::RefCountedThreadSafe<TestGetInstallState> { public: - explicit TestGetInstallState(WebDataService* web_data_service) - : install_data_(web_data_service), + explicit TestGetInstallState(SearchProviderInstallData* install_data) + : install_data_(install_data), main_loop_(NULL), passed_(false) { } @@ -62,7 +63,7 @@ class TestGetInstallState : void VerifyInstallState(SearchProviderInstallData::State expected_state, const std::string& url); - SearchProviderInstallData install_data_; + SearchProviderInstallData* install_data_; MessageLoop* main_loop_; // A host which should be a search provider but not the default. @@ -98,7 +99,7 @@ TestGetInstallState::~TestGetInstallState() { } void TestGetInstallState::StartTestOnIOThread() { - install_data_.CallWhenLoaded( + install_data_->CallWhenLoaded( NewRunnableMethod(this, &TestGetInstallState::DoInstallStateTests)); } @@ -137,7 +138,7 @@ void TestGetInstallState::VerifyInstallState( const std::string& url) { SearchProviderInstallData::State actual_state = - install_data_.GetInstallState(GURL(url)); + install_data_->GetInstallState(GURL(url)); if (expected_state == actual_state) return; @@ -150,16 +151,23 @@ void TestGetInstallState::VerifyInstallState( // that use TemplateURLModelTestUtil. class SearchProviderInstallDataTest : public testing::Test { public: - SearchProviderInstallDataTest() {} + SearchProviderInstallDataTest() + : install_data_(NULL) {} virtual void SetUp() { testing::Test::SetUp(); util_.SetUp(); + install_data_ = new SearchProviderInstallData(util_.GetWebDataService()); io_thread_.reset(new ChromeThread(ChromeThread::IO)); io_thread_->Start(); } virtual void TearDown() { + io_thread_->message_loop()->PostTask( + FROM_HERE, + new DeleteTask<SearchProviderInstallData>(install_data_)); + install_data_ = NULL; + util_.BlockTillIOThreadProcessesRequests(); io_thread_->Stop(); io_thread_.reset(); util_.TearDown(); @@ -170,6 +178,10 @@ class SearchProviderInstallDataTest : public testing::Test { TemplateURLModelTestUtil util_; scoped_ptr<ChromeThread> io_thread_; + // Provides the search provider install state on the I/O thread. It must be + // deleted on the I/O thread, which is why it isn't a scoped_ptr. + SearchProviderInstallData* install_data_; + DISALLOW_COPY_AND_ASSIGN(SearchProviderInstallDataTest); }; @@ -186,7 +198,7 @@ TEST_F(SearchProviderInstallDataTest, GetInstallState) { // Verify the search providers install state (with no default set). scoped_refptr<TestGetInstallState> test_get_install_state( - new TestGetInstallState(util_.GetWebDataService())); + new TestGetInstallState(install_data_)); test_get_install_state->set_search_provider_host(host); EXPECT_TRUE(test_get_install_state->RunTests(*io_thread_.get())); diff --git a/tools/valgrind/tsan/suppressions.txt b/tools/valgrind/tsan/suppressions.txt index ba8fb1e..3400d38 100644 --- a/tools/valgrind/tsan/suppressions.txt +++ b/tools/valgrind/tsan/suppressions.txt @@ -39,13 +39,6 @@ fun:*media*PipelineImpl*GetCurrentTime* } -{ - bug_56002 - ThreadSanitizer:Race - fun:*scoped_ptr* - fun:*SearchProviderInstallData* -} - ############################ # Real races in third_party { |