Deflake SimpleCache::Close()

It turns out that Close() takes a small, finite amount of time before
it is safe to open an entry again, because it must write the EOF
record before closing the file.

To reduce flakiness, do not permit OpenEntry to proceed while a Close
is pending. This is done by placing a more detailed state machine in the
SimpleEntryImpl, along with a map of all active entries in the SimpleBackendImpl.

Not all flakes are gone (especially around mass dooms), but this should be a strict
improvement.

R=pliard,pasko,felipeg
BUG=233871,236322

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

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

6 files changed, 419 insertions, 199 deletions

diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index f5b112a..1ec66eb 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -2479,6 +2479,7 @@ void DiskCacheBackendTest::BackendDoomAll() {
   MessageLoop::current()->RunUntilIdle();
 
   disk_cache::Entry *entry3, *entry4;
+  EXPECT_NE(net::OK, OpenEntry("third", &entry3));
   ASSERT_EQ(net::OK, CreateEntry("third", &entry3));
   ASSERT_EQ(net::OK, CreateEntry("fourth", &entry4));
 
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index ef5ad9b..b98d6fd 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -59,6 +59,7 @@ class DiskCacheEntryTest : public DiskCacheTestWithCache {
   void DoomSparseEntry();
   void PartialSparseEntry();
   void EvictOldEntries();
+  void SimpleCacheMakeBadChecksumEntry(const char* key);
 };
 
 // This part of the test runs on the background thread.
@@ -2351,12 +2352,7 @@ TEST_F(DiskCacheEntryTest, SimpleCacheDoomedEntry) {
   DoomedEntry();
 }
 
-// Tests that the simple cache can detect entries that have bad data.
-TEST_F(DiskCacheEntryTest, SimpleCacheBadChecksum) {
-  SetSimpleCacheMode();
-  InitCache();
-
-  const char key[] = "the first key";
+void DiskCacheEntryTest::SimpleCacheMakeBadChecksumEntry(const char* key) {
   disk_cache::Entry* entry = NULL;
 
   ASSERT_EQ(net::OK, CreateEntry(key, &entry));
@@ -2388,15 +2384,50 @@ TEST_F(DiskCacheEntryTest, SimpleCacheBadChecksum) {
             base::WritePlatformFile(entry_file0, file_offset,
                                     bad_data.data(), bad_data.size()));
   EXPECT_TRUE(base::ClosePlatformFile(entry_file0));
+}
+
+// Tests that the simple cache can detect entries that have bad data.
+TEST_F(DiskCacheEntryTest, SimpleCacheBadChecksum) {
+  SetSimpleCacheMode();
+  InitCache();
+
+  const char key[] = "the first key";
+  SimpleCacheMakeBadChecksumEntry(key);
+
+  disk_cache::Entry* entry = NULL;
 
   // Open the entry.
   EXPECT_EQ(net::OK, OpenEntry(key, &entry));
 
-  const size_t kReadBufferSize = 200;
-  EXPECT_LE(data.size(), kReadBufferSize);
+  const int kReadBufferSize = 200;
+  DCHECK_GE(kReadBufferSize, entry->GetDataSize(0));
   scoped_refptr<net::IOBuffer> read_buffer(new net::IOBuffer(kReadBufferSize));
   EXPECT_EQ(net::ERR_FAILED,
             ReadData(entry, 0, 0, read_buffer, kReadBufferSize));
+
+  entry->Close();
+}
+
+// Tests that an entry that has had an IO error occur can still be Doomed().
+TEST_F(DiskCacheEntryTest, SimpleCacheErrorThenDoom) {
+  SetSimpleCacheMode();
+  InitCache();
+
+  const char key[] = "the first key";
+  SimpleCacheMakeBadChecksumEntry(key);
+
+  disk_cache::Entry* entry = NULL;
+
+  // Open the entry, forcing an IO error.
+  EXPECT_EQ(net::OK, OpenEntry(key, &entry));
+
+  const int kReadBufferSize = 200;
+  DCHECK_GE(kReadBufferSize, entry->GetDataSize(0));
+  scoped_refptr<net::IOBuffer> read_buffer(new net::IOBuffer(kReadBufferSize));
+  EXPECT_EQ(net::ERR_FAILED,
+            ReadData(entry, 0, 0, read_buffer, kReadBufferSize));
+
+  entry->Doom();  // Should not crash.
   entry->Close();
 }
 
diff --git a/net/disk_cache/simple/simple_backend_impl.cc b/net/disk_cache/simple/simple_backend_impl.cc
index 08dcd39..59ab3f8 100644
--- a/net/disk_cache/simple/simple_backend_impl.cc
+++ b/net/disk_cache/simple/simple_backend_impl.cc
@@ -19,6 +19,7 @@
 #include "net/disk_cache/simple/simple_entry_impl.h"
 #include "net/disk_cache/simple/simple_index.h"
 #include "net/disk_cache/simple/simple_synchronous_entry.h"
+#include "net/disk_cache/simple/simple_util.h"
 
 using base::Closure;
 using base::FilePath;
@@ -168,6 +169,11 @@ bool SimpleBackendImpl::SetMaxSize(int max_bytes) {
   return index_->SetMaxSize(max_bytes);
 }
 
+void SimpleBackendImpl::OnDeactivated(const SimpleEntryImpl* entry) {
+  DCHECK_LT(0U, active_entries_.count(entry->key()));
+  active_entries_.erase(entry->key());
+}
+
 net::CacheType SimpleBackendImpl::GetCacheType() const {
   return net::DISK_CACHE;
 }
@@ -180,19 +186,21 @@ int32 SimpleBackendImpl::GetEntryCount() const {
 int SimpleBackendImpl::OpenEntry(const std::string& key,
                                  Entry** entry,
                                  const CompletionCallback& callback) {
-  return SimpleEntryImpl::OpenEntry(index_.get(), path_, key, entry, callback);
+  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
+  return simple_entry->OpenEntry(entry, callback);
 }
 
 int SimpleBackendImpl::CreateEntry(const std::string& key,
                                    Entry** entry,
                                    const CompletionCallback& callback) {
-  return SimpleEntryImpl::CreateEntry(index_.get(), path_, key, entry,
-                                      callback);
+  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
+  return simple_entry->CreateEntry(entry, callback);
 }
 
 int SimpleBackendImpl::DoomEntry(const std::string& key,
                                  const net::CompletionCallback& callback) {
-  return SimpleEntryImpl::DoomEntry(index_.get(), path_, key, callback);
+  scoped_refptr<SimpleEntryImpl> simple_entry = CreateOrFindActiveEntry(key);
+  return simple_entry->DoomEntry(callback);
 }
 
 int SimpleBackendImpl::DoomAllEntries(const CompletionCallback& callback) {
@@ -207,12 +215,41 @@ void SimpleBackendImpl::IndexReadyForDoom(Time initial_time,
     callback.Run(result);
     return;
   }
-  scoped_ptr<std::vector<uint64> > key_hashes(
+  scoped_ptr<std::vector<uint64> > removed_key_hashes(
       index_->RemoveEntriesBetween(initial_time, end_time).release());
+
+  // If any of the entries we are dooming are currently open, we need to remove
+  // them from |active_entries_|, so that attempts to create new entries will
+  // succeed and attempts to open them will fail.
+
+  // Construct a mapping by entry hash of |active_entries_|.
+  typedef std::map<uint64, SimpleEntryImpl*> EntryByHashMap;
+  EntryByHashMap active_entries_by_entry_hash;
+  for (EntryMap::const_iterator it = active_entries_.begin();
+       it != active_entries_.end(); ++it) {
+    if (SimpleEntryImpl* entry = it->second.get()) {
+      const uint64 entry_hash = simple_util::GetEntryHashKey(entry->key());
+      active_entries_by_entry_hash[entry_hash] = entry;
+    }
+  }
+
+  for (int i = removed_key_hashes->size() - 1; i >= 0; --i) {
+    EntryByHashMap::const_iterator it =
+        active_entries_by_entry_hash.find((*removed_key_hashes)[i]);
+    if (it == active_entries_by_entry_hash.end())
+      continue;
+    SimpleEntryImpl* entry = it->second;
+    entry->Doom();
+
+    (*removed_key_hashes)[i] = removed_key_hashes->back();
+    removed_key_hashes->resize(removed_key_hashes->size() - 1);
+  }
+
   scoped_ptr<int> new_result(new int());
   Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntrySet,
-                            base::Passed(&key_hashes), path_, new_result.get());
-  Closure reply = base::Bind(CallCompletionCallback,
+                            base::Passed(&removed_key_hashes), path_,
+                            new_result.get());
+  Closure reply = base::Bind(&CallCompletionCallback,
                              callback, base::Passed(&new_result));
   WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
 }
@@ -289,4 +326,21 @@ void SimpleBackendImpl::ProvideDirectorySuggestBetterCacheSize(
                       base::Bind(initialize_index_callback, max_size, rv));
 }
 
+scoped_refptr<SimpleEntryImpl> SimpleBackendImpl::CreateOrFindActiveEntry(
+    const std::string& key) {
+  std::pair<EntryMap::iterator, bool> insert_result =
+      active_entries_.insert(std::make_pair(key,
+                                            base::WeakPtr<SimpleEntryImpl>()));
+  EntryMap::iterator& it = insert_result.first;
+  if (insert_result.second)
+    DCHECK(!it->second);
+  if (!it->second) {
+    SimpleEntryImpl* entry = new SimpleEntryImpl(this, path_, key);
+    it->second = entry->AsWeakPtr();
+  }
+  DCHECK(it->second);
+  DCHECK_EQ(key, it->second->key());
+  return make_scoped_refptr(it->second.get());
+}
+
 }  // namespace disk_cache
diff --git a/net/disk_cache/simple/simple_backend_impl.h b/net/disk_cache/simple/simple_backend_impl.h
index cfe8133..58bbc74 100644
--- a/net/disk_cache/simple/simple_backend_impl.h
+++ b/net/disk_cache/simple/simple_backend_impl.h
@@ -5,6 +5,7 @@
 #ifndef NET_DISK_CACHE_SIMPLE_SIMPLE_BACKEND_IMPL_H_
 #define NET_DISK_CACHE_SIMPLE_SIMPLE_BACKEND_IMPL_H_
 
+#include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ -13,6 +14,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/task_runner.h"
 #include "net/base/cache_type.h"
 #include "net/disk_cache/disk_cache.h"
@@ -31,6 +33,7 @@ namespace disk_cache {
 
 // See http://www.chromium.org/developers/design-documents/network-stack/disk-cache/very-simple-backend
 
+class SimpleEntryImpl;
 class SimpleIndex;
 
 class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
@@ -43,12 +46,18 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
 
   virtual ~SimpleBackendImpl();
 
+  SimpleIndex* index() { return index_.get(); }
+
   // Must run on IO Thread.
   int Init(const CompletionCallback& completion_callback);
 
   // Sets the maximum size for the total amount of data stored by this instance.
   bool SetMaxSize(int max_bytes);
 
+  // Removes |entry| from the |active_entries_| set, forcing future Open/Create
+  // operations to construct a new object.
+  void OnDeactivated(const SimpleEntryImpl* entry);
+
   // From Backend:
   virtual net::CacheType GetCacheType() const OVERRIDE;
   virtual int32 GetEntryCount() const OVERRIDE;
@@ -72,6 +81,8 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
   virtual void OnExternalCacheHit(const std::string& key) OVERRIDE;
 
  private:
+  typedef std::map<std::string, base::WeakPtr<SimpleEntryImpl> > EntryMap;
+
   typedef base::Callback<void(uint64 max_size, int result)>
       InitializeIndexCallback;
 
@@ -95,10 +106,20 @@ class NET_EXPORT_PRIVATE SimpleBackendImpl : public Backend,
       const InitializeIndexCallback& initialize_index_callback,
       uint64 suggested_max_size);
 
+  // Searches |active_entries_| for the entry corresponding to |key|. If found,
+  // returns the found entry. Otherwise, creates a new entry and returns that.
+  scoped_refptr<SimpleEntryImpl> CreateOrFindActiveEntry(
+      const std::string& key);
+
   const base::FilePath path_;
   scoped_ptr<SimpleIndex> index_;
   const scoped_refptr<base::SingleThreadTaskRunner> cache_thread_;
+
   int orig_max_size_;
+
+  // TODO(gavinp): Store the entry_hash in SimpleEntryImpl, and index this map
+  // by hash. This will save memory, and make IndexReadyForDoom easier.
+  EntryMap active_entries_;
 };
 
 }  // namespace disk_cache
diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc
index 2f58c73..a338dd9 100644
--- a/net/disk_cache/simple/simple_entry_impl.cc
+++ b/net/disk_cache/simple/simple_entry_impl.cc
@@ -19,12 +19,15 @@
 #include "base/time.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/disk_cache/simple/simple_backend_impl.h"
 #include "net/disk_cache/simple/simple_index.h"
 #include "net/disk_cache/simple/simple_synchronous_entry.h"
 #include "third_party/zlib/zlib.h"
 
 namespace {
 
+// Short trampoline to take an owned input parameter and call a net completion
+// callback with its value.
 void CallCompletionCallback(const net::CompletionCallback& callback,
                             scoped_ptr<int> result) {
   DCHECK(result);
@@ -42,91 +45,64 @@ using base::MessageLoopProxy;
 using base::Time;
 using base::WorkerPool;
 
-// static
-int SimpleEntryImpl::OpenEntry(SimpleIndex* entry_index,
-                               const FilePath& path,
-                               const std::string& key,
-                               Entry** out_entry,
-                               const CompletionCallback& callback) {
-  // This enumeration is used in histograms, add entries only at end.
-  enum OpenEntryIndexEnum {
-    INDEX_NOEXIST = 0,
-    INDEX_MISS = 1,
-    INDEX_HIT = 2,
-    INDEX_MAX = 3,
-  };
-  OpenEntryIndexEnum open_entry_index_enum = INDEX_NOEXIST;
-  if (entry_index) {
-    if (entry_index->Has(key))
-      open_entry_index_enum = INDEX_HIT;
-    else
-      open_entry_index_enum = INDEX_MISS;
+// A helper class to insure that RunNextOperationIfNeeded() is called when
+// exiting the current stack frame.
+class SimpleEntryImpl::ScopedOperationRunner {
+ public:
+  explicit ScopedOperationRunner(SimpleEntryImpl* entry) : entry_(entry) {
   }
-  UMA_HISTOGRAM_ENUMERATION("SimpleCache.OpenEntryIndexState",
-                            open_entry_index_enum, INDEX_MAX);
-  // If entry is not known to the index, initiate fast failover to the network.
-  if (open_entry_index_enum == INDEX_MISS)
-    return net::ERR_FAILED;
 
-  const base::TimeTicks start_time = base::TimeTicks::Now();
-  // Go down to the disk to find the entry.
-  scoped_refptr<SimpleEntryImpl> new_entry =
-      new SimpleEntryImpl(entry_index, path, key);
-  typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry;
-  scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry(
-      new PointerToSimpleSynchronousEntry());
-  Closure task = base::Bind(&SimpleSynchronousEntry::OpenEntry, path, key,
-                            sync_entry.get());
-  Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete,
-                             new_entry,
-                             callback,
-                             start_time,
-                             base::Passed(&sync_entry),
-                             out_entry);
-  WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
+  ~ScopedOperationRunner() {
+    entry_->RunNextOperationIfNeeded();
+  }
+
+ private:
+  SimpleEntryImpl* const entry_;
+};
+
+SimpleEntryImpl::SimpleEntryImpl(SimpleBackendImpl* backend,
+                                 const FilePath& path,
+                                 const std::string& key)
+    : backend_(backend->AsWeakPtr()),
+      path_(path),
+      key_(key),
+      open_count_(0),
+      state_(STATE_UNINITIALIZED),
+      synchronous_entry_(NULL) {
+  COMPILE_ASSERT(arraysize(data_size_) == arraysize(crc32s_end_offset_),
+                 arrays_should_be_same_size);
+  COMPILE_ASSERT(arraysize(data_size_) == arraysize(crc32s_),
+                 arrays_should_be_same_size2);
+  COMPILE_ASSERT(arraysize(data_size_) == arraysize(have_written_),
+                 arrays_should_be_same_size3);
+
+  MakeUninitialized();
+}
+
+int SimpleEntryImpl::OpenEntry(Entry** out_entry,
+                               const CompletionCallback& callback) {
+  DCHECK(backend_);
+
+  pending_operations_.push(base::Bind(&SimpleEntryImpl::OpenEntryInternal,
+                                      this, out_entry, callback));
+  RunNextOperationIfNeeded();
   return net::ERR_IO_PENDING;
 }
 
-// static
-int SimpleEntryImpl::CreateEntry(SimpleIndex* entry_index,
-                                 const FilePath& path,
-                                 const std::string& key,
-                                 Entry** out_entry,
+int SimpleEntryImpl::CreateEntry(Entry** out_entry,
                                  const CompletionCallback& callback) {
-  const base::TimeTicks start_time = base::TimeTicks::Now();
-  // We insert the entry in the index before creating the entry files in the
-  // SimpleSynchronousEntry, because this way the worse scenario is when we
-  // have the entry in the index but we don't have the created files yet, this
-  // way we never leak files. CreationOperationComplete will remove the entry
-  // from the index if the creation fails.
-  if (entry_index)
-    entry_index->Insert(key);
-  scoped_refptr<SimpleEntryImpl> new_entry =
-      new SimpleEntryImpl(entry_index, path, key);
-
-  typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry;
-  scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry(
-      new PointerToSimpleSynchronousEntry());
-  Closure task = base::Bind(&SimpleSynchronousEntry::CreateEntry, path, key,
-                            sync_entry.get());
-  Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete,
-                             new_entry,
-                             callback,
-                             start_time,
-                             base::Passed(&sync_entry),
-                             out_entry);
-  WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
+  DCHECK(backend_);
+  pending_operations_.push(base::Bind(&SimpleEntryImpl::CreateEntryInternal,
+                                      this, out_entry, callback));
+  RunNextOperationIfNeeded();
   return net::ERR_IO_PENDING;
 }
 
-// static
-int SimpleEntryImpl::DoomEntry(SimpleIndex* entry_index,
-                               const FilePath& path,
-                               const std::string& key,
-                               const CompletionCallback& callback) {
-  entry_index->Remove(key);
+int SimpleEntryImpl::DoomEntry(const CompletionCallback& callback) {
+  MarkAsDoomed();
+
   scoped_ptr<int> result(new int());
-  Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntry, path, key,
+  Closure task = base::Bind(&SimpleSynchronousEntry::DoomEntry, path_, key_,
                             result.get());
   Closure reply = base::Bind(&CallCompletionCallback,
                              callback, base::Passed(&result));
@@ -134,26 +110,24 @@ int SimpleEntryImpl::DoomEntry(SimpleIndex* entry_index,
   return net::ERR_IO_PENDING;
 }
 
+
 void SimpleEntryImpl::Doom() {
-  DCHECK(io_thread_checker_.CalledOnValidThread());
-  DCHECK(synchronous_entry_);
-#if defined(OS_POSIX)
-  // This call to static SimpleEntryImpl::DoomEntry() will just erase the
-  // underlying files. On POSIX, this is fine; the files are still open on the
-  // SimpleSynchronousEntry, and operations can even happen on them. The files
-  // will be removed from the filesystem when they are closed.
-  DoomEntry(entry_index_, path_, key_, CompletionCallback());
-#else
-  NOTIMPLEMENTED();
-#endif
+  DoomEntry(CompletionCallback());
 }
 
 void SimpleEntryImpl::Close() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
-  // Postpone close operation.
-  // Push the close operation to the end of the line. This way we run all
-  // operations before we are able close.
+  DCHECK_LT(0, open_count_);
+
+  if (--open_count_ > 0) {
+    DCHECK(!HasOneRef());
+    Release();  // Balanced in ReturnEntryToCaller().
+    return;
+  }
+
   pending_operations_.push(base::Bind(&SimpleEntryImpl::CloseInternal, this));
+  DCHECK(!HasOneRef());
+  Release();  // Balanced in ReturnEntryToCaller().
   RunNextOperationIfNeeded();
 }
 
@@ -277,51 +251,146 @@ int SimpleEntryImpl::ReadyForSparseIO(const CompletionCallback& callback) {
   return net::ERR_FAILED;
 }
 
-SimpleEntryImpl::SimpleEntryImpl(SimpleIndex* entry_index,
-                                 const FilePath& path,
-                                 const std::string& key)
-    :  entry_index_(entry_index->AsWeakPtr()),
-       path_(path),
-       key_(key),
-       synchronous_entry_(NULL),
-       operation_running_(false) {
-  COMPILE_ASSERT(arraysize(data_size_) == arraysize(crc32s_end_offset_),
-                 arrays_should_be_same_size);
-  COMPILE_ASSERT(arraysize(data_size_) == arraysize(crc32s_),
-                 arrays_should_be_same_size2);
-  COMPILE_ASSERT(arraysize(data_size_) == arraysize(have_written_),
-                 arrays_should_be_same_size3);
+SimpleEntryImpl::~SimpleEntryImpl() {
+  DCHECK(io_thread_checker_.CalledOnValidThread());
+  DCHECK_EQ(0U, pending_operations_.size());
+  DCHECK_EQ(STATE_UNINITIALIZED, state_);
+  DCHECK(!synchronous_entry_);
+  RemoveSelfFromBackend();
+}
 
+void SimpleEntryImpl::MakeUninitialized() {
+  state_ = STATE_UNINITIALIZED;
   std::memset(crc32s_end_offset_, 0, sizeof(crc32s_end_offset_));
   std::memset(crc32s_, 0, sizeof(crc32s_));
   std::memset(have_written_, 0, sizeof(have_written_));
 }
 
-SimpleEntryImpl::~SimpleEntryImpl() {
-  DCHECK(io_thread_checker_.CalledOnValidThread());
-  DCHECK_EQ(0U, pending_operations_.size());
-  DCHECK(!operation_running_);
-  DCHECK(!synchronous_entry_);
+void SimpleEntryImpl::ReturnEntryToCaller(Entry** out_entry) {
+  ++open_count_;
+  AddRef();  // Balanced in Close()
+  *out_entry = this;
 }
 
-bool SimpleEntryImpl::RunNextOperationIfNeeded() {
+void SimpleEntryImpl::RemoveSelfFromBackend() {
+  if (!backend_)
+    return;
+  backend_->OnDeactivated(this);
+  backend_.reset();
+}
+
+void SimpleEntryImpl::MarkAsDoomed() {
+  if (!backend_)
+    return;
+  backend_->index()->Remove(key_);
+  RemoveSelfFromBackend();
+}
+
+void SimpleEntryImpl::RunNextOperationIfNeeded() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
+
   UMA_HISTOGRAM_CUSTOM_COUNTS("SimpleCache.EntryOperationsPending",
                               pending_operations_.size(), 0, 100, 20);
-  if (pending_operations_.size() <= 0 || operation_running_)
-    return false;
-  base::Closure operation = pending_operations_.front();
-  pending_operations_.pop();
-  operation.Run();
-  return true;
+
+  if (!pending_operations_.empty() && state_ != STATE_IO_PENDING) {
+    base::Closure operation = pending_operations_.front();
+    pending_operations_.pop();
+    operation.Run();
+    // |this| may have been deleted.
+  }
+}
+
+void SimpleEntryImpl::OpenEntryInternal(Entry** out_entry,
+                                        const CompletionCallback& callback) {
+  ScopedOperationRunner operation_runner(this);
+
+  if (state_ == STATE_READY) {
+    ReturnEntryToCaller(out_entry);
+    MessageLoopProxy::current()->PostTask(FROM_HERE, base::Bind(callback,
+                                                                net::OK));
+    return;
+  }
+  DCHECK_EQ(STATE_UNINITIALIZED, state_);
+
+  // This enumeration is used in histograms, add entries only at end.
+  enum OpenEntryIndexEnum {
+    INDEX_NOEXIST = 0,
+    INDEX_MISS = 1,
+    INDEX_HIT = 2,
+    INDEX_MAX = 3,
+  };
+  OpenEntryIndexEnum open_entry_index_enum = INDEX_NOEXIST;
+  if (backend_) {
+    if (backend_->index()->Has(key_))
+      open_entry_index_enum = INDEX_HIT;
+    else
+      open_entry_index_enum = INDEX_MISS;
+  }
+  UMA_HISTOGRAM_ENUMERATION("SimpleCache.OpenEntryIndexState",
+                            open_entry_index_enum, INDEX_MAX);
+  // If entry is not known to the index, initiate fast failover to the network.
+  if (open_entry_index_enum == INDEX_MISS) {
+    MessageLoopProxy::current()->PostTask(FROM_HERE,
+                                          base::Bind(callback,
+                                                     net::ERR_FAILED));
+    return;
+  }
+  state_ = STATE_IO_PENDING;
+
+  const base::TimeTicks start_time = base::TimeTicks::Now();
+  typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry;
+  scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry(
+      new PointerToSimpleSynchronousEntry());
+  Closure task = base::Bind(&SimpleSynchronousEntry::OpenEntry, path_, key_,
+                            sync_entry.get());
+  Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete, this,
+                             callback, start_time, base::Passed(&sync_entry),
+                             out_entry);
+  WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
+}
+
+void SimpleEntryImpl::CreateEntryInternal(Entry** out_entry,
+                                          const CompletionCallback& callback) {
+  ScopedOperationRunner operation_runner(this);
+
+  if (state_ == STATE_READY) {
+    // There is already an active normal entry.
+    MessageLoopProxy::current()->PostTask(FROM_HERE,
+                                          base::Bind(callback,
+                                                     net::ERR_FAILED));
+    return;
+  }
+  DCHECK_EQ(STATE_UNINITIALIZED, state_);
+
+  state_ = STATE_IO_PENDING;
+
+  // We insert the entry in the index before creating the entry files in the
+  // SimpleSynchronousEntry, because this way the worst scenario is when we
+  // have the entry in the index but we don't have the created files yet, this
+  // way we never leak files. CreationOperationComplete will remove the entry
+  // from the index if the creation fails.
+  if (backend_)
+    backend_->index()->Insert(key_);
+  const base::TimeTicks start_time = base::TimeTicks::Now();
+  typedef SimpleSynchronousEntry* PointerToSimpleSynchronousEntry;
+  scoped_ptr<PointerToSimpleSynchronousEntry> sync_entry(
+      new PointerToSimpleSynchronousEntry());
+  Closure task = base::Bind(&SimpleSynchronousEntry::CreateEntry, path_, key_,
+                            sync_entry.get());
+  Closure reply = base::Bind(&SimpleEntryImpl::CreationOperationComplete, this,
+                             callback, start_time, base::Passed(&sync_entry),
+                             out_entry);
+  WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
 }
 
 void SimpleEntryImpl::CloseInternal() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   DCHECK_EQ(0U, pending_operations_.size());
-  DCHECK(!operation_running_);
+  DCHECK_EQ(STATE_READY, state_);
   DCHECK(synchronous_entry_);
 
+  state_ = STATE_IO_PENDING;
+
   typedef SimpleSynchronousEntry::CRCRecord CRCRecord;
 
   scoped_ptr<std::vector<CRCRecord> >
@@ -334,15 +403,12 @@ void SimpleEntryImpl::CloseInternal() {
         crc32s_to_write->push_back(CRCRecord(i, false, 0));
     }
   }
-  WorkerPool::PostTask(FROM_HERE,
-                       base::Bind(&SimpleSynchronousEntry::Close,
-                                  base::Unretained(synchronous_entry_),
-                                  base::Passed(&crc32s_to_write)),
-                       true);
+  Closure task = base::Bind(&SimpleSynchronousEntry::Close,
+                            base::Unretained(synchronous_entry_),
+                            base::Passed(&crc32s_to_write));
+  Closure reply = base::Bind(&SimpleEntryImpl::CloseOperationComplete, this);
+  WorkerPool::PostTaskAndReply(FROM_HERE, task, reply, true);
   synchronous_entry_ = NULL;
-  // Entry::Close() is expected to delete this entry. See disk_cache.h for
-  // details.
-  Release();  // Balanced in CreationOperationComplete().
 }
 
 void SimpleEntryImpl::ReadDataInternal(int stream_index,
@@ -351,10 +417,10 @@ void SimpleEntryImpl::ReadDataInternal(int stream_index,
                                        int buf_len,
                                        const CompletionCallback& callback) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
-  DCHECK(!operation_running_);
-  operation_running_ = true;
-  if (entry_index_)
-      entry_index_->UseIfExists(key_);
+  DCHECK_EQ(STATE_READY, state_);
+  state_ = STATE_IO_PENDING;
+  if (backend_)
+    backend_->index()->UseIfExists(key_);
 
   scoped_ptr<uint32> read_crc32(new uint32());
   scoped_ptr<int> result(new int());
@@ -375,10 +441,10 @@ void SimpleEntryImpl::WriteDataInternal(int stream_index,
                                        const CompletionCallback& callback,
                                        bool truncate) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
-  DCHECK(!operation_running_);
-  operation_running_ = true;
-  if (entry_index_)
-    entry_index_->UseIfExists(key_);
+  DCHECK_EQ(STATE_READY, state_);
+  state_ = STATE_IO_PENDING;
+  if (backend_)
+    backend_->index()->UseIfExists(key_);
   // It is easy to incrementally compute the CRC from [0 .. |offset + buf_len|)
   // if |offset == 0| or we have already computed the CRC for [0 .. offset).
   // We rely on most write operations being sequential, start to end to compute
@@ -413,30 +479,25 @@ void SimpleEntryImpl::CreationOperationComplete(
     scoped_ptr<SimpleSynchronousEntry*> in_sync_entry,
     Entry** out_entry) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
+  DCHECK_EQ(state_, STATE_IO_PENDING);
   DCHECK(in_sync_entry);
 
-  bool creation_result = !*in_sync_entry;
-  UMA_HISTOGRAM_BOOLEAN("SimpleCache.EntryCreationResult", creation_result);
-  if (creation_result) {
+  ScopedOperationRunner operation_runner(this);
+
+  bool creation_failed = !*in_sync_entry;
+  UMA_HISTOGRAM_BOOLEAN("SimpleCache.EntryCreationResult", creation_failed);
+  if (creation_failed) {
     completion_callback.Run(net::ERR_FAILED);
-    // If OpenEntry failed, we must remove it from our index.
-    if (entry_index_)
-      entry_index_->Remove(key_);
-    // The reference held by the Callback calling us will go out of scope and
-    // delete |this| on leaving this scope.
+    MarkAsDoomed();
+    state_ = STATE_UNINITIALIZED;
     return;
   }
-
-  // The Backend interface requires us to return |this|, and keep the Entry
-  // alive until Entry::Close(). Adding a reference to self will keep |this|
-  // alive after the scope of the Callback calling us is destroyed.
-  AddRef();  // Balanced in CloseInternal().
+  state_ = STATE_READY;
   synchronous_entry_ = *in_sync_entry;
   SetSynchronousData();
-  *out_entry = this;
+  ReturnEntryToCaller(out_entry);
   UMA_HISTOGRAM_TIMES("SimpleCache.EntryCreationTime",
                       (base::TimeTicks::Now() - start_time));
-
   completion_callback.Run(net::OK);
 }
 
@@ -446,15 +507,13 @@ void SimpleEntryImpl::EntryOperationComplete(
     scoped_ptr<int> result) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   DCHECK(synchronous_entry_);
-  DCHECK(operation_running_);
+  DCHECK_EQ(STATE_IO_PENDING, state_);
   DCHECK(result);
 
-  operation_running_ = false;
+  state_ = STATE_READY;
 
   if (*result < 0) {
-    if (entry_index_)
-      entry_index_->Remove(key_);
-    entry_index_.reset();
+    MarkAsDoomed();
     crc32s_end_offset_[stream_index] = 0;
   }
   SetSynchronousData();
@@ -470,7 +529,7 @@ void SimpleEntryImpl::ReadOperationComplete(
     scoped_ptr<int> result) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   DCHECK(synchronous_entry_);
-  DCHECK(operation_running_);
+  DCHECK_EQ(STATE_IO_PENDING, state_);
   DCHECK(read_crc32);
   DCHECK(result);
 
@@ -514,16 +573,25 @@ void SimpleEntryImpl::ChecksumOperationComplete(
     scoped_ptr<int> result) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   DCHECK(synchronous_entry_);
-  DCHECK(operation_running_);
+  DCHECK_EQ(STATE_IO_PENDING, state_);
   DCHECK(result);
   if (*result == net::OK)
     *result = orig_result;
   EntryOperationComplete(stream_index, completion_callback, result.Pass());
 }
 
+void SimpleEntryImpl::CloseOperationComplete() {
+  DCHECK(!synchronous_entry_);
+  DCHECK_EQ(0, open_count_);
+  DCHECK_EQ(STATE_IO_PENDING, state_);
+
+  MakeUninitialized();
+  RunNextOperationIfNeeded();
+}
+
 void SimpleEntryImpl::SetSynchronousData() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
-  DCHECK(!operation_running_);
+  DCHECK_EQ(STATE_READY, state_);
   // TODO(felipeg): These copies to avoid data races are not optimal. While
   // adding an IO thread index (for fast misses etc...), we can store this data
   // in that structure. This also solves problems with last_used() on ext4
@@ -532,8 +600,8 @@ void SimpleEntryImpl::SetSynchronousData() {
   last_modified_ = synchronous_entry_->last_modified();
   for (int i = 0; i < kSimpleEntryFileCount; ++i)
     data_size_[i] = synchronous_entry_->data_size(i);
-  if (entry_index_)
-    entry_index_->UpdateEntrySize(key_, synchronous_entry_->GetFileSize());
+  if (backend_)
+    backend_->index()->UpdateEntrySize(key_, synchronous_entry_->GetFileSize());
 }
 
 }  // namespace disk_cache
diff --git a/net/disk_cache/simple/simple_entry_impl.h b/net/disk_cache/simple/simple_entry_impl.h
index 7ceb97e..fcc7d04 100644
--- a/net/disk_cache/simple/simple_entry_impl.h
+++ b/net/disk_cache/simple/simple_entry_impl.h
@@ -15,7 +15,6 @@
 #include "base/threading/thread_checker.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/disk_cache/simple/simple_entry_format.h"
-#include "net/disk_cache/simple/simple_index.h"
 
 namespace base {
 class MessageLoopProxy;
@@ -27,30 +26,31 @@ class IOBuffer;
 
 namespace disk_cache {
 
+class SimpleBackendImpl;
 class SimpleSynchronousEntry;
 
 // SimpleEntryImpl is the IO thread interface to an entry in the very simple
 // disk cache. It proxies for the SimpleSynchronousEntry, which performs IO
 // on the worker thread.
-class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
+class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl>,
+    public base::SupportsWeakPtr<SimpleEntryImpl> {
   friend class base::RefCounted<SimpleEntryImpl>;
  public:
-  static int OpenEntry(SimpleIndex* entry_index,
-                       const base::FilePath& path,
-                       const std::string& key,
-                       Entry** entry,
-                       const CompletionCallback& callback);
-
-  static int CreateEntry(SimpleIndex* entry_index,
-                         const base::FilePath& path,
-                         const std::string& key,
-                         Entry** entry,
-                         const CompletionCallback& callback);
+  SimpleEntryImpl(SimpleBackendImpl* backend,
+                  const base::FilePath& path,
+                  const std::string& key);
+
+  // Adds another reader/writer to this entry, if possible, returning |this| to
+  // |entry|.
+  int OpenEntry(Entry** entry, const CompletionCallback& callback);
+
+  // Creates this entry, if possible. Returns |this| to |entry|.
+  int CreateEntry(Entry** entry, const CompletionCallback& callback);
+
+  // Identical to Backend::Doom() except that it accepts a CompletionCallback.
+  int DoomEntry(const CompletionCallback& callback);
 
-  static int DoomEntry(SimpleIndex* entry_index,
-                       const base::FilePath& path,
-                       const std::string& key,
-                       const CompletionCallback& callback);
+  const std::string& key() const { return key_; }
 
   // From Entry:
   virtual void Doom() OVERRIDE;
@@ -87,15 +87,52 @@ class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
   virtual int ReadyForSparseIO(const CompletionCallback& callback) OVERRIDE;
 
  private:
-  SimpleEntryImpl(SimpleIndex* entry_index,
-                  const base::FilePath& path,
-                  const std::string& key);
+  class ScopedOperationRunner;
+  friend class ScopedOperationRunner;
+
+  enum State {
+    // The state immediately after construction, but before |synchronous_entry_|
+    // has been assigned. This is the state at construction, and is the only
+    // legal state to destruct an entry in.
+    STATE_UNINITIALIZED,
+
+    // This entry is available for regular IO.
+    STATE_READY,
+
+    // IO is currently in flight, operations must wait for completion before
+    // launching.
+    STATE_IO_PENDING,
+  };
 
   virtual ~SimpleEntryImpl();
 
+  // Sets entry o STATE_UNINITIALIZED.
+  void MakeUninitialized();
+
+  // Return this entry to a user of the API in |out_entry|. Increments the user
+  // count.
+  void ReturnEntryToCaller(Entry** out_entry);
+
+  // Ensures that |this| is no longer referenced by our |backend_|, this
+  // guarantees that this entry cannot have OpenEntry/CreateEntry called again.
+  void RemoveSelfFromBackend();
+
+  // An error occured, and the SimpleSynchronousEntry should have Doomed
+  // us at this point. We need to remove |this| from the Backend and the
+  // index.
+  void MarkAsDoomed();
+
   // Runs the next operation in the queue, if any and if there is no other
-  // operation running at the moment. Returns true if a operation has run.
-  bool RunNextOperationIfNeeded();
+  // operation running at the moment.
+  // WARNING: May delete |this|, as an operation in the queue can contain
+  // the last reference.
+  void RunNextOperationIfNeeded();
+
+  void OpenEntryInternal(Entry** entry,
+                         const CompletionCallback& callback);
+
+  void CreateEntryInternal(Entry** entry,
+                           const CompletionCallback& callback);
 
   void CloseInternal();
 
@@ -122,6 +159,11 @@ class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
       scoped_ptr<SimpleSynchronousEntry*> in_sync_entry,
       Entry** out_entry);
 
+  // Called after we've closed and written the EOF record to our entry. Until
+  // this point it hasn't been safe to OpenEntry() the same entry, but from this
+  // point it is.
+  void CloseOperationComplete();
+
   // Called after a SimpleSynchronousEntry has completed an asynchronous IO
   // operation, such as ReadData() or WriteData(). Calls |completion_callback|.
   void EntryOperationComplete(
@@ -156,7 +198,7 @@ class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
   // thread, in all cases. |io_thread_checker_| documents and enforces this.
   base::ThreadChecker io_thread_checker_;
 
-  base::WeakPtr<SimpleIndex> entry_index_;
+  base::WeakPtr<SimpleBackendImpl> backend_;
   const base::FilePath path_;
   const std::string key_;
 
@@ -166,6 +208,13 @@ class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
   base::Time last_modified_;
   int32 data_size_[kSimpleEntryFileCount];
 
+  // Number of times this object has been returned from Backend::OpenEntry() and
+  // Backend::CreateEntry() without subsequent Entry::Close() calls. Used to
+  // notify the backend when this entry not used by any callers.
+  int open_count_;
+
+  State state_;
+
   // When possible, we compute a crc32, for the data in each entry as we read or
   // write. For each stream, |crc32s_[index]| is the crc32 of that stream from
   // [0 .. |crc32s_end_offset_|). If |crc32s_end_offset_[index] == 0| then the
@@ -181,10 +230,6 @@ class SimpleEntryImpl : public Entry, public base::RefCounted<SimpleEntryImpl> {
   // is false (i.e. when an operation is not pending on the worker pool).
   SimpleSynchronousEntry* synchronous_entry_;
 
-  // Set to true when a worker operation is posted on the |synchronous_entry_|,
-  // and false after. Used to ensure thread safety by not allowing multiple
-  // threads to access the |synchronous_entry_| simultaneously.
-  bool operation_running_;
   std::queue<base::Closure> pending_operations_;
 };