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}

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<content::AppCacheInfoCollection> 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<GURL, content::AppCacheInfoVector> 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<GURL, content::AppCacheInfoVector> 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<content::AppCacheInfoCollection> 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<BrowsingDataAppCacheHelper>;
   virtual ~BrowsingDataAppCacheHelper();
 
-  FetchCallback completion_callback_;
-  scoped_refptr<content::AppCacheInfoCollection> 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<content::AppCacheInfoCollection> 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);
 };