Make SimpleEntryImpl::Close asynchronous.

Since only one entry operation can be in flight at once, it's safe to assume that if the IO thread Entry doesn't exist on completion that the sync entry should be deleted.

This issue is downstream of https://codereview.chromium.org/12223075/ and should land after it.

R=rvargas@chromium.org
BUG=173392

Review URL: https://chromiumcodereview.appspot.com/12277004

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

2 files changed, 24 insertions, 15 deletions

diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc
index 68f65dc..14bcee3 100644
--- a/net/disk_cache/simple/simple_entry_impl.cc
+++ b/net/disk_cache/simple/simple_entry_impl.cc
@@ -86,17 +86,12 @@ void SimpleEntryImpl::Doom() {
 }
 
 void SimpleEntryImpl::Close() {
-  if (synchronous_entry_in_use_by_worker_) {
-    NOTIMPLEMENTED();
-    delete this;
-    return;
+  if (!synchronous_entry_in_use_by_worker_) {
+    WorkerPool::PostTask(FROM_HERE,
+                         base::Bind(&SimpleSynchronousEntry::Close,
+                                    base::Unretained(synchronous_entry_)),
+                         true);
   }
-  DCHECK(synchronous_entry_);
-  WorkerPool::PostTask(FROM_HERE,
-                       base::Bind(&SimpleSynchronousEntry::Close,
-                                  base::Unretained(synchronous_entry_)),
-                       true);
-  synchronous_entry_ = NULL;
   // Entry::Close() is expected to release this entry. See disk_cache.h for
   // details.
   delete this;
@@ -125,7 +120,8 @@ int SimpleEntryImpl::ReadData(int index,
                               const CompletionCallback& callback) {
   // TODO(gavinp): Add support for overlapping reads. The net::HttpCache does
   // make overlapping read requests when multiple transactions access the same
-  // entry as read only.
+  // entry as read only. This might make calling SimpleSynchronousEntry::Close()
+  // correctly more tricky (see SimpleEntryImpl::EntryOperationComplete).
   if (synchronous_entry_in_use_by_worker_) {
     NOTIMPLEMENTED();
     CHECK(false);
@@ -133,7 +129,7 @@ int SimpleEntryImpl::ReadData(int index,
   synchronous_entry_in_use_by_worker_ = true;
   SynchronousOperationCallback sync_operation_callback =
       base::Bind(&SimpleEntryImpl::EntryOperationComplete,
-                 callback, weak_ptr_factory_.GetWeakPtr());
+                 callback, weak_ptr_factory_.GetWeakPtr(), synchronous_entry_);
   WorkerPool::PostTask(FROM_HERE,
                        base::Bind(&SimpleSynchronousEntry::ReadData,
                                   base::Unretained(synchronous_entry_),
@@ -156,7 +152,7 @@ int SimpleEntryImpl::WriteData(int index,
   synchronous_entry_in_use_by_worker_ = true;
   SynchronousOperationCallback sync_operation_callback =
       base::Bind(&SimpleEntryImpl::EntryOperationComplete,
-                 callback, weak_ptr_factory_.GetWeakPtr());
+                 callback, weak_ptr_factory_.GetWeakPtr(), synchronous_entry_);
   WorkerPool::PostTask(FROM_HERE,
                        base::Bind(&SimpleSynchronousEntry::WriteData,
                                   base::Unretained(synchronous_entry_),
@@ -221,7 +217,6 @@ SimpleEntryImpl::SimpleEntryImpl(
 }
 
 SimpleEntryImpl::~SimpleEntryImpl() {
-  DCHECK(!synchronous_entry_);
 }
 
 // static
@@ -241,11 +236,20 @@ void SimpleEntryImpl::CreationOperationComplete(
 void SimpleEntryImpl::EntryOperationComplete(
     const CompletionCallback& completion_callback,
     base::WeakPtr<SimpleEntryImpl> entry,
+    SimpleSynchronousEntry* sync_entry,
     int result) {
   if (entry) {
     DCHECK(entry->synchronous_entry_in_use_by_worker_);
     entry->synchronous_entry_in_use_by_worker_ = false;
     entry->SetSynchronousData();
+  } else {
+    // |entry| must have had Close() called while this operation was in flight.
+    // Since the simple cache now only supports one pending entry operation in
+    // flight at a time, it's safe to now call Close() on |sync_entry|.
+    WorkerPool::PostTask(FROM_HERE,
+                         base::Bind(&SimpleSynchronousEntry::Close,
+                                    base::Unretained(sync_entry)),
+                         true);
   }
   completion_callback.Run(result);
 }
diff --git a/net/disk_cache/simple/simple_entry_impl.h b/net/disk_cache/simple/simple_entry_impl.h
index 4dc76dd..c0ae78c 100644
--- a/net/disk_cache/simple/simple_entry_impl.h
+++ b/net/disk_cache/simple/simple_entry_impl.h
@@ -89,9 +89,11 @@ class SimpleEntryImpl : public Entry {
 
   // Called after a SimpleSynchronousEntry has completed an asynchronous IO
   // operation, such as ReadData() or WriteData(). Calls |completion_callback|.
+  // If |entry| no longer exists, then it ensures |sync_entry| is closed.
   static void EntryOperationComplete(
       const CompletionCallback& completion_callback,
       base::WeakPtr<SimpleEntryImpl> entry,
+      SimpleSynchronousEntry* sync_entry,
       int result);
 
   // Called on construction and also after the completion of asynchronous IO to
@@ -114,7 +116,10 @@ class SimpleEntryImpl : public Entry {
   int32 data_size_[kSimpleEntryFileCount];
 
   // The |synchronous_entry_| is the worker thread object that performs IO on
-  // entries.
+  // entries. It's owned by this SimpleEntryImpl whenever
+  // |synchronous_entry_in_use_by_worker_| is false (i.e. when an operation
+  // is not pending on the worker pool). When an operation is pending on the
+  // worker pool, the |synchronous_entry_| is owned by itself.
   SimpleSynchronousEntry* synchronous_entry_;
 
   // Set to true when a worker operation is posted on the |synchronous_entry_|,