Clean up BrowsingDataAppCacheHelper

chrome/browser/browsing_data/browsing_data_appcache_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/browsing_data/browsing_data_appcache_helper.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 
+using content::BrowserContext;
 using content::BrowserThread;
-using content::BrowserContext;
 
-BrowsingDataAppCacheHelper::BrowsingDataAppCacheHelper(Profile* profile)
+BrowsingDataAppCacheHelper::BrowsingDataAppCacheHelper(
+    BrowserContext* browser_context)
     : is_fetching_(false),
-      appcache_service_(BrowserContext::GetDefaultStoragePartition(profile)->
-                            GetAppCacheService()) {
+      appcache_service_(BrowserContext::GetDefaultStoragePartition(
+                            browser_context)->GetAppCacheService()) {
 }
 
 void BrowsingDataAppCacheHelper::StartFetching(const base::Closure& 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::IO);
-  appcache_info_callback_.Reset(
+  appcache_service_->GetAllAppCacheInfo(

[Bernhard Bauer, 2014/09/05 16:15:12]

So, what happens when this object is destroyed now?

[vabr (Chromium), 2014/09/19 12:35:47]

Thanks for catching this. I was confused when first reading the
CancellableCallback definition -- it does not Cancel() itself on destruction,
and neither is Cancel() called by BrowsingDataAppCacheHelper on destruction. But
I missed that the CancellableCallback binds the wrapped callback to a weak
pointer to itself, so this effectively cancels the callback on the destruction
of CancellableCallback (and it's owner).

Therefore, putting appcache_info_callback_ back.

+      info_collection_.get(),
       base::Bind(&BrowsingDataAppCacheHelper::OnFetchComplete,
                  base::Unretained(this)));
-  appcache_service_->GetAllAppCacheInfo(info_collection_.get(),
-                                        appcache_info_callback_.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;
@@ skipping 26 matching lines @@
   }
 
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(is_fetching_);
   is_fetching_ = false;
   completion_callback_.Run();
   completion_callback_.Reset();
 }
 
 CannedBrowsingDataAppCacheHelper::CannedBrowsingDataAppCacheHelper(
-    Profile* profile)
-    : BrowsingDataAppCacheHelper(profile),
-      profile_(profile) {
+    BrowserContext* browser_context)
+    : BrowsingDataAppCacheHelper(browser_context),
+      browser_context_(browser_context) {
   info_collection_ = new content::AppCacheInfoCollection;
 }
 
 CannedBrowsingDataAppCacheHelper* CannedBrowsingDataAppCacheHelper::Clone() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   CannedBrowsingDataAppCacheHelper* clone =
-      new CannedBrowsingDataAppCacheHelper(profile_);
+      new CannedBrowsingDataAppCacheHelper(browser_context_);
 
   clone->info_collection_->infos_by_origin = info_collection_->infos_by_origin;
   return clone;
 }
 
 void CannedBrowsingDataAppCacheHelper::AddAppCache(const GURL& manifest_url) {
   if (!BrowsingDataHelper::HasWebScheme(manifest_url))
     return;  // Ignore non-websafe schemes.
 
   OriginAppCacheInfoMap& origin_map = info_collection_->infos_by_origin;
@@ skipping 41 matching lines @@
   completion_callback.Run();
 }
 
 void CannedBrowsingDataAppCacheHelper::DeleteAppCacheGroup(
     const GURL& manifest_url) {
   info_collection_->infos_by_origin.erase(manifest_url.GetOrigin());
   BrowsingDataAppCacheHelper::DeleteAppCacheGroup(manifest_url);
 }
 
 CannedBrowsingDataAppCacheHelper::~CannedBrowsingDataAppCacheHelper() {}