From ae0534ec3d8d054ee0578cf96da1486eb691f4c0 Mon Sep 17 00:00:00 2001 From: jsbell Date: Wed, 26 Aug 2015 18:25:43 -0700 Subject: Simplify browsing data helper for AppCache Remove local state from the helper during fetch in favor of passing everything along in closures. Also, follow modern conventions and add XXXOnIOThread methods rather than having XXX check which thread its on and PostTask. BUG=524222 Review URL: https://codereview.chromium.org/1307163003 Cr-Commit-Position: refs/heads/master@{#345756} --- .../browsing_data/browsing_data_appcache_helper.cc | 107 +++++++++++---------- .../browsing_data/browsing_data_appcache_helper.h | 9 +- .../mock_browsing_data_appcache_helper.h | 2 + 3 files changed, 62 insertions(+), 56 deletions(-) diff --git a/chrome/browser/browsing_data/browsing_data_appcache_helper.cc b/chrome/browser/browsing_data/browsing_data_appcache_helper.cc index 1a38977..88ce531 100644 --- a/chrome/browser/browsing_data/browsing_data_appcache_helper.cc +++ b/chrome/browser/browsing_data/browsing_data_appcache_helper.cc @@ -16,6 +16,33 @@ using content::BrowserContext; using content::BrowserThread; +namespace { + +void OnFetchComplete( + const BrowsingDataAppCacheHelper::FetchCallback& callback, + scoped_refptr info_collection, + int /*rv*/) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + DCHECK(!callback.is_null()); + + // Filter out appcache info entries for non-websafe schemes. Extension state + // and DevTools, for example, are not considered browsing data. + typedef std::map InfoByOrigin; + InfoByOrigin& origin_map = info_collection->infos_by_origin; + for (InfoByOrigin::iterator it = origin_map.begin(); + it != origin_map.end();) { + if (!BrowsingDataHelper::HasWebScheme(it->first)) + origin_map.erase(it++); + else + ++it; + } + + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(callback, info_collection)); +} + +} // namespace + BrowsingDataAppCacheHelper::BrowsingDataAppCacheHelper( BrowserContext* browser_context) : appcache_service_( @@ -23,65 +50,43 @@ BrowsingDataAppCacheHelper::BrowsingDataAppCacheHelper( ->GetAppCacheService()) {} void BrowsingDataAppCacheHelper::StartFetching(const FetchCallback& callback) { - if (BrowserThread::CurrentlyOn(BrowserThread::UI)) { - DCHECK(!is_fetching_); - DCHECK(!callback.is_null()); - is_fetching_ = true; - info_collection_ = new content::AppCacheInfoCollection; - completion_callback_ = callback; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&BrowsingDataAppCacheHelper::StartFetching, this, callback)); - return; - } + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(!callback.is_null()); - DCHECK_CURRENTLY_ON(BrowserThread::IO); - appcache_service_->GetAllAppCacheInfo( - info_collection_.get(), - base::Bind(&BrowsingDataAppCacheHelper::OnFetchComplete, this)); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&BrowsingDataAppCacheHelper::StartFetchingOnIOThread, this, + callback)); } -void BrowsingDataAppCacheHelper::DeleteAppCacheGroup( - const GURL& manifest_url) { - if (BrowserThread::CurrentlyOn(BrowserThread::UI)) { - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(&BrowsingDataAppCacheHelper::DeleteAppCacheGroup, this, - manifest_url)); - return; - } - - appcache_service_->DeleteAppCacheGroup(manifest_url, - net::CompletionCallback()); +void BrowsingDataAppCacheHelper::DeleteAppCacheGroup(const GURL& manifest_url) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + base::Bind(&BrowsingDataAppCacheHelper::DeleteAppCacheGroupOnIOThread, + this, manifest_url)); } BrowsingDataAppCacheHelper::~BrowsingDataAppCacheHelper() {} -void BrowsingDataAppCacheHelper::OnFetchComplete(int rv) { - if (BrowserThread::CurrentlyOn(BrowserThread::IO)) { - // Filter out appcache info entries for non-websafe schemes. Extension state - // and DevTools, for example, are not considered browsing data. - typedef std::map InfoByOrigin; - InfoByOrigin& origin_map = info_collection_->infos_by_origin; - for (InfoByOrigin::iterator origin = origin_map.begin(); - origin != origin_map.end();) { - InfoByOrigin::iterator current = origin; - ++origin; - if (!BrowsingDataHelper::HasWebScheme(current->first)) - origin_map.erase(current); - } - - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&BrowsingDataAppCacheHelper::OnFetchComplete, this, rv)); - return; - } +void BrowsingDataAppCacheHelper::StartFetchingOnIOThread( + const FetchCallback& callback) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + DCHECK(!callback.is_null()); - DCHECK_CURRENTLY_ON(BrowserThread::UI); - DCHECK(is_fetching_); - is_fetching_ = false; - completion_callback_.Run(info_collection_); - completion_callback_.Reset(); + scoped_refptr info_collection = + new content::AppCacheInfoCollection(); + + appcache_service_->GetAllAppCacheInfo( + info_collection.get(), + base::Bind(&OnFetchComplete, callback, info_collection)); +} + +void BrowsingDataAppCacheHelper::DeleteAppCacheGroupOnIOThread( + const GURL& manifest_url) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + appcache_service_->DeleteAppCacheGroup(manifest_url, + net::CompletionCallback()); } CannedBrowsingDataAppCacheHelper::CannedBrowsingDataAppCacheHelper( diff --git a/chrome/browser/browsing_data/browsing_data_appcache_helper.h b/chrome/browser/browsing_data/browsing_data_appcache_helper.h index ddcdb78..15d1797 100644 --- a/chrome/browser/browsing_data/browsing_data_appcache_helper.h +++ b/chrome/browser/browsing_data/browsing_data_appcache_helper.h @@ -35,13 +35,10 @@ class BrowsingDataAppCacheHelper friend class base::RefCountedThreadSafe; virtual ~BrowsingDataAppCacheHelper(); - FetchCallback completion_callback_; - scoped_refptr info_collection_; - private: - void OnFetchComplete(int rv); + void StartFetchingOnIOThread(const FetchCallback& completion_callback); + void DeleteAppCacheGroupOnIOThread(const GURL& manifest_url); - bool is_fetching_ = false; content::AppCacheService* appcache_service_; DISALLOW_COPY_AND_ASSIGN(BrowsingDataAppCacheHelper); @@ -78,6 +75,8 @@ class CannedBrowsingDataAppCacheHelper : public BrowsingDataAppCacheHelper { private: ~CannedBrowsingDataAppCacheHelper() override; + scoped_refptr info_collection_; + DISALLOW_COPY_AND_ASSIGN(CannedBrowsingDataAppCacheHelper); }; diff --git a/chrome/browser/browsing_data/mock_browsing_data_appcache_helper.h b/chrome/browser/browsing_data/mock_browsing_data_appcache_helper.h index 0246ffd..e81d92a 100644 --- a/chrome/browser/browsing_data/mock_browsing_data_appcache_helper.h +++ b/chrome/browser/browsing_data/mock_browsing_data_appcache_helper.h @@ -20,6 +20,8 @@ class MockBrowsingDataAppCacheHelper private: ~MockBrowsingDataAppCacheHelper() override; + FetchCallback completion_callback_; + DISALLOW_COPY_AND_ASSIGN(MockBrowsingDataAppCacheHelper); }; -- cgit v1.1