base::Bind: Remove OldCompletionCallback.

webkit/appcache/appcache_disk_cache.cc

Index: webkit/appcache/appcache_disk_cache.cc
diff --git a/webkit/appcache/appcache_disk_cache.cc b/webkit/appcache/appcache_disk_cache.cc
index c0c946802fd9d7557d4741427732fc7dbd8c60bd..3d452093348abba4fc97bd67e5d170d23bc3d848 100644
--- a/webkit/appcache/appcache_disk_cache.cc
+++ b/webkit/appcache/appcache_disk_cache.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -14,6 +14,12 @@
 
 namespace appcache {
 
+// TODO(ajwong): Change disk_cache's API to return results via Callback.
+struct AppCacheDiskCache::CreateBackendDataShim {
+  CreateBackendDataShim() : backend(NULL) {}
+  disk_cache::Backend* backend;
+};
+
 // An implementation of AppCacheDiskCacheInterface::Entry that's a thin
 // wrapper around disk_cache::Entry.
 class AppCacheDiskCache::EntryImpl : public Entry {
@@ -123,10 +129,9 @@ AppCacheDiskCache::AppCacheDiskCache()
 }
 
 AppCacheDiskCache::~AppCacheDiskCache() {
-  if (create_backend_callback_) {
-    create_backend_callback_->Cancel();
-    create_backend_callback_.release();
-    OnCreateBackendComplete(net::ERR_ABORTED);
+  if (!create_backend_callback_.IsCancelled()) {
+    create_backend_callback_.Cancel();

[michaeln, 2012/01/04 02:07:17]

will this call to Cancel() delete storage for the backend ptr? But
disk_cache::CreateCacheBackend() may still access it after this call returns...
use-after-free error?

[James Hawkins, 2012/01/05 22:32:51]

I think you're right, but I also think this bug exists in the previous code.

CreateBackendCallback::backend_ptr is deleted on release().

Is that correct?

[michaeln, 2012/01/05 23:34:54]

Don't think so. Like we chatted about, the old CancelableOldCompletionCallback
infrastructure voodoo kept the CreateBackendCallback class alive till it was no
longer needed.

+    OnCreateBackendComplete(NULL, net::ERR_ABORTED);
   }
   disk_cache_.reset();
   STLDeleteElements(&active_calls_);
@@ -152,10 +157,9 @@ void AppCacheDiskCache::Disable() {
 
   is_disabled_ = true;
 
-  if (create_backend_callback_) {
-    create_backend_callback_->Cancel();
-    create_backend_callback_.release();
-    OnCreateBackendComplete(net::ERR_ABORTED);
+  if (!create_backend_callback_.IsCancelled()) {
+    create_backend_callback_.Cancel();

[michaeln, 2012/01/04 02:07:17]

ditto

+    OnCreateBackendComplete(NULL, net::ERR_ABORTED);
   }
 }
 
@@ -221,26 +225,31 @@ int AppCacheDiskCache::Init(net::CacheType cache_type,
                             const net::CompletionCallback& callback) {
   DCHECK(!is_initializing() && !disk_cache_.get());
   is_disabled_ = false;
-  create_backend_callback_ = new CreateBackendCallback(
-      this, &AppCacheDiskCache::OnCreateBackendComplete);
 
+  // TODO(ajwong): Change disk_cache's API to return results via Callback.
+  disk_cache::Backend** backend_ptr = new(disk_cache::Backend*);
+  create_backend_callback_.Reset(
+      base::Bind(&AppCacheDiskCache::OnCreateBackendComplete,
+                 base::Unretained(this), base::Owned(backend_ptr)));
   int rv = disk_cache::CreateCacheBackend(
       cache_type, cache_directory, cache_size, force, cache_thread, NULL,
-      &(create_backend_callback_->backend_ptr_),
-      base::Bind(&net::OldCompletionCallbackAdapter, create_backend_callback_));
+      backend_ptr, create_backend_callback_.callback());
   if (rv == net::ERR_IO_PENDING)
     init_callback_ = callback;
   else
-    OnCreateBackendComplete(rv);
+    OnCreateBackendComplete(backend_ptr, rv);
+
   return rv;
 }
 
-void AppCacheDiskCache::OnCreateBackendComplete(int rv) {
+void AppCacheDiskCache::OnCreateBackendComplete(
+    disk_cache::Backend** backend, int rv) {
   if (rv == net::OK) {
-    disk_cache_.reset(create_backend_callback_->backend_ptr_);
-    create_backend_callback_->backend_ptr_ = NULL;
+    DCHECK(backend);
+    disk_cache_.reset(*backend);
   }
-  create_backend_callback_ = NULL;
+
+  create_backend_callback_.Cancel();
 
   // Invoke our clients callback function.
   if (!init_callback_.is_null()) {