From 28e7d96b3ce30a098e4a6839455a232bc843c915 Mon Sep 17 00:00:00 2001 From: "pkasting@chromium.org" Date: Tue, 14 Jan 2014 00:20:20 +0000 Subject: [PATCH] Ensure WeakPtrFactories are at the end of their owning classes and refer directly to their owning class types. Also does a little bit of other non-functional cleanup, mostly eliminating unnecessary get() calls and fixing violations of the "don't handle DCHECK failure" style rule. BUG=303818 TEST=none R=dmichael@chromium.org Review URL: https://codereview.chromium.org/122543002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244618 0039d316-1c4b-4281-b951-d872f2087c98 --- .../download_request_infobar_delegate_unittest.cc | 27 ++++++----- chrome/browser/google/google_url_tracker.cc | 6 +-- chrome/browser/google/google_url_tracker.h | 2 +- chrome/browser/intranet_redirect_detector.cc | 8 ++-- chrome/browser/intranet_redirect_detector.h | 2 +- components/webdata/common/web_database_service.cc | 52 ++++++++-------------- components/webdata/common/web_database_service.h | 6 +-- 7 files changed, 46 insertions(+), 57 deletions(-) diff --git a/chrome/browser/download/download_request_infobar_delegate_unittest.cc b/chrome/browser/download/download_request_infobar_delegate_unittest.cc index c45dd691f9f7..e0b657b631af 100644 --- a/chrome/browser/download/download_request_infobar_delegate_unittest.cc +++ b/chrome/browser/download/download_request_infobar_delegate_unittest.cc @@ -15,10 +15,10 @@ class MockTabDownloadState : public DownloadRequestLimiter::TabDownloadState { MockTabDownloadState(); virtual ~MockTabDownloadState(); - // DownloadRequestLimiter::TabDownloadState + // DownloadRequestLimiter::TabDownloadState: virtual void Cancel() OVERRIDE; virtual void Accept() OVERRIDE; - virtual void CancelOnce() OVERRIDE { Cancel(); } + virtual void CancelOnce() OVERRIDE; ConfirmInfoBarDelegate* infobar_delegate() { return infobar_delegate_.get(); } void delete_infobar_delegate() { infobar_delegate_.reset(); } @@ -26,9 +26,6 @@ class MockTabDownloadState : public DownloadRequestLimiter::TabDownloadState { bool accepted() const { return accepted_; } private: - // To produce weak pointers for infobar_ construction. - base::WeakPtrFactory factory_; - // The actual infobar delegate we're listening to. scoped_ptr infobar_delegate_; @@ -39,14 +36,18 @@ class MockTabDownloadState : public DownloadRequestLimiter::TabDownloadState { // not true. bool accepted_; + // To produce weak pointers for infobar_ construction. + base::WeakPtrFactory weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(MockTabDownloadState); }; MockTabDownloadState::MockTabDownloadState() - : factory_(this), - infobar_delegate_( - DownloadRequestInfoBarDelegate::Create(factory_.GetWeakPtr())), - responded_(false), - accepted_(false) { + : responded_(false), + accepted_(false), + weak_ptr_factory_(this) { + infobar_delegate_ = + DownloadRequestInfoBarDelegate::Create(weak_ptr_factory_.GetWeakPtr()); } MockTabDownloadState::~MockTabDownloadState() { @@ -63,7 +64,11 @@ void MockTabDownloadState::Accept() { EXPECT_FALSE(responded_); responded_ = true; accepted_ = true; - factory_.InvalidateWeakPtrs(); + weak_ptr_factory_.InvalidateWeakPtrs(); +} + +void MockTabDownloadState::CancelOnce() { + Cancel(); } diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc index 81cb919aa87d..d971caf21025 100644 --- a/chrome/browser/google/google_url_tracker.cc +++ b/chrome/browser/google/google_url_tracker.cc @@ -41,13 +41,13 @@ GoogleURLTracker::GoogleURLTracker( infobar_creator_(base::Bind(&GoogleURLTrackerInfoBarDelegate::Create)), google_url_(mode == UNIT_TEST_MODE ? kDefaultGoogleHomepage : profile->GetPrefs()->GetString(prefs::kLastKnownGoogleURL)), - weak_ptr_factory_(this), fetcher_id_(0), in_startup_sleep_(true), already_fetched_(false), need_to_fetch_(false), need_to_prompt_(false), - search_committed_(false) { + search_committed_(false), + weak_ptr_factory_(this) { net::NetworkChangeNotifier::AddIPAddressObserver(this); nav_helper_->SetGoogleURLTracker(this); @@ -202,8 +202,8 @@ void GoogleURLTracker::OnIPAddressChanged() { void GoogleURLTracker::Shutdown() { nav_helper_.reset(); - weak_ptr_factory_.InvalidateWeakPtrs(); fetcher_.reset(); + weak_ptr_factory_.InvalidateWeakPtrs(); net::NetworkChangeNotifier::RemoveIPAddressObserver(this); } diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h index f5723220e3e1..e71dbecadea4 100644 --- a/chrome/browser/google/google_url_tracker.h +++ b/chrome/browser/google/google_url_tracker.h @@ -184,7 +184,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate, GURL google_url_; GURL fetched_google_url_; - base::WeakPtrFactory weak_ptr_factory_; scoped_ptr fetcher_; int fetcher_id_; bool in_startup_sleep_; // True if we're in the five-second "no fetching" @@ -202,6 +201,7 @@ class GoogleURLTracker : public net::URLFetcherDelegate, bool search_committed_; // True when we're expecting a notification of a new // pending search navigation. EntryMap entry_map_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(GoogleURLTracker); }; diff --git a/chrome/browser/intranet_redirect_detector.cc b/chrome/browser/intranet_redirect_detector.cc index bb9376d9a05d..0e2aa1864d15 100644 --- a/chrome/browser/intranet_redirect_detector.cc +++ b/chrome/browser/intranet_redirect_detector.cc @@ -26,8 +26,8 @@ const size_t IntranetRedirectDetector::kNumCharsInHostnames = 10; IntranetRedirectDetector::IntranetRedirectDetector() : redirect_origin_(g_browser_process->local_state()->GetString( prefs::kLastKnownIntranetRedirectOrigin)), - weak_factory_(this), - in_sleep_(true) { + in_sleep_(true), + weak_ptr_factory_(this) { // Because this function can be called during startup, when kicking off a URL // fetch can eat up 20 ms of time, we delay seven seconds, which is hopefully // long enough to be after startup, but still get results back quickly. @@ -37,7 +37,7 @@ IntranetRedirectDetector::IntranetRedirectDetector() static const int kStartFetchDelaySeconds = 7; base::MessageLoop::current()->PostDelayedTask(FROM_HERE, base::Bind(&IntranetRedirectDetector::FinishSleep, - weak_factory_.GetWeakPtr()), + weak_ptr_factory_.GetWeakPtr()), base::TimeDelta::FromSeconds(kStartFetchDelaySeconds)); net::NetworkChangeNotifier::AddIPAddressObserver(this); @@ -159,7 +159,7 @@ void IntranetRedirectDetector::OnIPAddressChanged() { static const int kNetworkSwitchDelayMS = 1000; base::MessageLoop::current()->PostDelayedTask(FROM_HERE, base::Bind(&IntranetRedirectDetector::FinishSleep, - weak_factory_.GetWeakPtr()), + weak_ptr_factory_.GetWeakPtr()), base::TimeDelta::FromMilliseconds(kNetworkSwitchDelayMS)); } diff --git a/chrome/browser/intranet_redirect_detector.h b/chrome/browser/intranet_redirect_detector.h index a39b9e683da8..a0343e8fc717 100644 --- a/chrome/browser/intranet_redirect_detector.h +++ b/chrome/browser/intranet_redirect_detector.h @@ -72,12 +72,12 @@ class IntranetRedirectDetector virtual void OnIPAddressChanged() OVERRIDE; GURL redirect_origin_; - base::WeakPtrFactory weak_factory_; Fetchers fetchers_; std::vector resulting_origins_; bool in_sleep_; // True if we're in the seven-second "no fetching" period // that begins at browser start, or the one-second "no // fetching" period that begins after network switches. + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(IntranetRedirectDetector); }; diff --git a/components/webdata/common/web_database_service.cc b/components/webdata/common/web_database_service.cc index 11eb30b8bf11..599ca03e6b54 100644 --- a/components/webdata/common/web_database_service.cc +++ b/components/webdata/common/web_database_service.cc @@ -43,9 +43,9 @@ WebDatabaseService::WebDatabaseService( const scoped_refptr& db_thread) : base::RefCountedDeleteOnMessageLoop(ui_thread), path_(path), - weak_ptr_factory_(this), db_loaded_(false), - db_thread_(db_thread) { + db_thread_(db_thread), + weak_ptr_factory_(this) { // WebDatabaseService should be instantiated on UI thread. DCHECK(ui_thread->BelongsToCurrentThread()); // WebDatabaseService requires DB thread if instantiated. @@ -56,7 +56,7 @@ WebDatabaseService::~WebDatabaseService() { } void WebDatabaseService::AddTable(scoped_ptr table) { - if (!wds_backend_.get()) { + if (!wds_backend_) { wds_backend_ = new WebDataServiceBackend( path_, new BackendDelegate(weak_ptr_factory_.GetWeakPtr()), db_thread_); @@ -65,8 +65,7 @@ void WebDatabaseService::AddTable(scoped_ptr table) { } void WebDatabaseService::LoadDatabase() { - DCHECK(wds_backend_.get()); - + DCHECK(wds_backend_); db_thread_->PostTask( FROM_HERE, Bind(&WebDataServiceBackend::InitDatabase, wds_backend_)); @@ -74,19 +73,19 @@ void WebDatabaseService::LoadDatabase() { void WebDatabaseService::UnloadDatabase() { db_loaded_ = false; - if (!wds_backend_.get()) + if (!wds_backend_) return; db_thread_->PostTask(FROM_HERE, - Bind(&WebDataServiceBackend::ShutdownDatabase, - wds_backend_, true)); + Bind(&WebDataServiceBackend::ShutdownDatabase, + wds_backend_, true)); } void WebDatabaseService::ShutdownDatabase() { db_loaded_ = false; - weak_ptr_factory_.InvalidateWeakPtrs(); loaded_callbacks_.clear(); error_callbacks_.clear(); - if (!wds_backend_.get()) + weak_ptr_factory_.InvalidateWeakPtrs(); + if (!wds_backend_) return; db_thread_->PostTask(FROM_HERE, Bind(&WebDataServiceBackend::ShutdownDatabase, @@ -95,9 +94,7 @@ void WebDatabaseService::ShutdownDatabase() { WebDatabase* WebDatabaseService::GetDatabaseOnDB() const { DCHECK(db_thread_->BelongsToCurrentThread()); - if (!wds_backend_.get()) - return NULL; - return wds_backend_->database(); + return wds_backend_ ? wds_backend_->database() : NULL; } scoped_refptr WebDatabaseService::GetBackend() const { @@ -107,17 +104,12 @@ scoped_refptr WebDatabaseService::GetBackend() const { void WebDatabaseService::ScheduleDBTask( const tracked_objects::Location& from_here, const WriteTask& task) { - if (!wds_backend_.get()) { - NOTREACHED() << "Task scheduled after Shutdown()"; - return; - } - + DCHECK(wds_backend_); scoped_ptr request( new WebDataRequest(NULL, wds_backend_->request_manager().get())); - db_thread_->PostTask(from_here, - Bind(&WebDataServiceBackend::DBWriteTaskWrapper, wds_backend_, - task, base::Passed(&request))); + Bind(&WebDataServiceBackend::DBWriteTaskWrapper, + wds_backend_, task, base::Passed(&request))); } WebDataServiceBase::Handle WebDatabaseService::ScheduleDBTaskWithResult( @@ -125,26 +117,18 @@ WebDataServiceBase::Handle WebDatabaseService::ScheduleDBTaskWithResult( const ReadTask& task, WebDataServiceConsumer* consumer) { DCHECK(consumer); - WebDataServiceBase::Handle handle = 0; - - if (!wds_backend_.get()) { - NOTREACHED() << "Task scheduled after Shutdown()"; - return handle; - } - + DCHECK(wds_backend_); scoped_ptr request( new WebDataRequest(consumer, wds_backend_->request_manager().get())); - handle = request->GetHandle(); - + WebDataServiceBase::Handle handle = request->GetHandle(); db_thread_->PostTask(from_here, - Bind(&WebDataServiceBackend::DBReadTaskWrapper, wds_backend_, - task, base::Passed(&request))); - + Bind(&WebDataServiceBackend::DBReadTaskWrapper, + wds_backend_, task, base::Passed(&request))); return handle; } void WebDatabaseService::CancelRequest(WebDataServiceBase::Handle h) { - if (!wds_backend_.get()) + if (!wds_backend_) return; wds_backend_->request_manager()->CancelRequest(h); } diff --git a/components/webdata/common/web_database_service.h b/components/webdata/common/web_database_service.h index 95ef5e2c7a18..4b30876055dd 100644 --- a/components/webdata/common/web_database_service.h +++ b/components/webdata/common/web_database_service.h @@ -135,9 +135,6 @@ class WEBDATA_EXPORT WebDatabaseService // PostTask on DB thread may outlive us. scoped_refptr wds_backend_; - // All vended weak pointers are invalidated in ShutdownDatabase(). - base::WeakPtrFactory weak_ptr_factory_; - // Callbacks to be called once the DB has loaded. LoadedCallbacks loaded_callbacks_; @@ -149,6 +146,9 @@ class WEBDATA_EXPORT WebDatabaseService scoped_refptr db_thread_; + // All vended weak pointers are invalidated in ShutdownDatabase(). + base::WeakPtrFactory weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(WebDatabaseService); }; -- 2.11.4.GIT