Http cache: It turns out that the cache destructor

may be called from within one of the notifications
of "backend construction done". This CL makes sure
that this scenario is handled properly by invoking
the callbacks one at a time, posting a task before
moving on to the next one.

BUG=52090
TEST=netunittests

Review URL: http://codereview.chromium.org/3173030

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56964 0039d316-1c4b-4281-b951-d872f2087c98

net/http/http_cache.cc

@@ -105,12 +105,16 @@ class HttpCache::WorkItem {
       trans_->io_callback()->Run(result);
   }
 
-  // Notifies the caller about the operation completion.
-  void DoCallback(int result, disk_cache::Backend* backend) {
+  // Notifies the caller about the operation completion. Returns true if the
+  // callback was invoked.
+  bool DoCallback(int result, disk_cache::Backend* backend) {
     if (backend_)
       *backend_ = backend;
-    if (callback_)
+    if (callback_) {
       callback_->Run(result);
+      return true;
+    }
+    return false;
   }
 
   WorkItemOperation operation() { return operation_; }
@@ -298,7 +302,8 @@ HttpCache::~HttpCache() {
       // deliver the callbacks.
       BackendCallback* callback =
           static_cast<BackendCallback*>(pending_op->callback);
-      callback->Cancel();
+      if (callback)
+        callback->Cancel();
     } else {
       delete pending_op->callback;
     }
@@ -1009,26 +1014,39 @@ void HttpCache::OnBackendCreated(int result, PendingOp* pending_op) {
   WorkItemOperation op = item->operation();
   DCHECK_EQ(WI_CREATE_BACKEND, op);
 
-  backend_factory_.reset();  // Reclaim memory.
-
-  if (result == OK)
-    disk_cache_.reset(temp_backend_);
+  // We don't need the callback anymore.
+  pending_op->callback = NULL;
 
-  item->DoCallback(result, temp_backend_);
+  if (backend_factory_.get()) {
+    // We may end up calling OnBackendCreated multiple times if we have pending
+    // work items. The first call saves the backend and releases the factory,
+    // and the last call clears building_backend_.
+    backend_factory_.reset();  // Reclaim memory.
+    if (result == OK)
+      disk_cache_.reset(temp_backend_);
+  }
 
-  // Notify all callers and delete all pending work items.
-  while (!pending_op->pending_queue.empty()) {
-    scoped_ptr<WorkItem> pending_item(pending_op->pending_queue.front());
+  if (!pending_op->pending_queue.empty()) {
+    WorkItem* pending_item = pending_op->pending_queue.front();
     pending_op->pending_queue.pop_front();
     DCHECK_EQ(WI_CREATE_BACKEND, pending_item->operation());
 
-    // This could be an external caller or a transaction waiting on Start().
-    pending_item->DoCallback(result, temp_backend_);
-    pending_item->NotifyTransaction(result, NULL);
+    // We want to process a single callback at a time, because the cache may
+    // go away from the callback.
+    pending_op->writer = pending_item;
+
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        task_factory_.NewRunnableMethod(&HttpCache::OnBackendCreated,
+                                        result, pending_op));
+  } else {
+    building_backend_ = false;
+    DeletePendingOp(pending_op);
   }
 
-  DeletePendingOp(pending_op);
-  building_backend_ = false;
+  // The cache may be gone when we return from the callback.
+  if (!item->DoCallback(result, temp_backend_))
+    item->NotifyTransaction(result, NULL);
 }
 
 }  // namespace net

net/http/http_cache_unittest.cc

@@ -2012,6 +2012,61 @@ TEST(HttpCache, DeleteCacheWaitingForBackend) {
   callback->Run(net::ERR_ABORTED);
 }
 
+class DeleteCacheCompletionCallback : public TestCompletionCallback {
+ public:
+  explicit DeleteCacheCompletionCallback(MockHttpCache* cache)
+      : cache_(cache) {}
+
+  virtual void RunWithParams(const Tuple1<int>& params) {
+    delete cache_;
+    TestCompletionCallback::RunWithParams(params);
+  }
+
+ private:
+  MockHttpCache* cache_;
+};
+
+// Tests that we can delete the cache while creating the backend, from within
+// one of the callbacks.
+TEST(HttpCache, DeleteCacheWaitingForBackend2) {
+  MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
+  MockHttpCache* cache = new MockHttpCache(factory);
+
+  DeleteCacheCompletionCallback cb(cache);
+  disk_cache::Backend* backend;
+  int rv = cache->http_cache()->GetBackend(&backend, &cb);
+  EXPECT_EQ(net::ERR_IO_PENDING, rv);
+
+  // Now let's queue a regular transaction
+  MockHttpRequest request(kSimpleGET_Transaction);
+
+  scoped_ptr<Context> c(new Context());
+  c->result = cache->http_cache()->CreateTransaction(&c->trans);
+  EXPECT_EQ(net::OK, c->result);
+
+  c->trans->Start(&request, &c->callback, net::BoundNetLog());
+
+  // And another direct backend request.
+  TestCompletionCallback cb2;
+  rv = cache->http_cache()->GetBackend(&backend, &cb2);
+  EXPECT_EQ(net::ERR_IO_PENDING, rv);
+
+  // Just to make sure that everything is still pending.
+  MessageLoop::current()->RunAllPending();
+
+  // The request should be queued.
+  EXPECT_FALSE(c->callback.have_result());
+
+  // Generate the callback.
+  factory->FinishCreation();
+  rv = cb.WaitForResult();
+
+  // The cache should be gone by now.
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(net::OK, c->callback.GetResult(c->result));
+  EXPECT_FALSE(cb2.have_result());
+}
+
 TEST(HttpCache, TypicalGET_ConditionalRequest) {
   MockHttpCache cache;