Don't doom the wrong simple cache entry.

Previously, dooming a previously doomed simple cache entry could doom an
existing entry. Now fixed.

R=pliard,pasko
BUG=289542

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

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

3 files changed, 41 insertions, 5 deletions

diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 5637143..188a589 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -2908,9 +2908,7 @@ TEST_F(DiskCacheEntryTest, SimpleCacheOptimistic5) {
       static_cast<disk_cache::SimpleEntryImpl*>(entry)->HasOneRef());
 }
 
-// TODO(gavinp): Fix this, perhaps by landing
-// https://codereview.chromium.org/23823002/
-TEST_F(DiskCacheEntryTest, DISABLED_SimpleCacheOptimistic6) {
+TEST_F(DiskCacheEntryTest, SimpleCacheOptimistic6) {
   // Test sequence:
   // Create, Write, Doom, Doom, Read, Doom, Close.
   SetSimpleCacheMode();
@@ -3053,6 +3051,40 @@ TEST_F(DiskCacheEntryTest, SimpleCacheDoomCreateRace) {
   EXPECT_EQ(net::OK, doom_callback.GetResult(net::ERR_IO_PENDING));
 }
 
+TEST_F(DiskCacheEntryTest, SimpleCacheDoomDoom) {
+  // Test sequence:
+  // Create, Doom, Create, Doom (1st entry), Open.
+  SetSimpleCacheMode();
+  InitCache();
+  disk_cache::Entry* null = NULL;
+
+  const char key[] = "the first key";
+
+  disk_cache::Entry* entry1 = NULL;
+  ASSERT_EQ(net::OK, CreateEntry(key, &entry1));
+  ScopedEntryPtr entry1_closer(entry1);
+  EXPECT_NE(null, entry1);
+
+  EXPECT_EQ(net::OK, DoomEntry(key));
+
+  disk_cache::Entry* entry2 = NULL;
+  ASSERT_EQ(net::OK, CreateEntry(key, &entry2));
+  ScopedEntryPtr entry2_closer(entry2);
+  EXPECT_NE(null, entry2);
+
+  // Redundantly dooming entry1 should not delete entry2.
+  disk_cache::SimpleEntryImpl* simple_entry1 =
+      static_cast<disk_cache::SimpleEntryImpl*>(entry1);
+  net::TestCompletionCallback cb;
+  EXPECT_EQ(net::OK,
+            cb.GetResult(simple_entry1->DoomEntry(cb.callback())));
+
+  disk_cache::Entry* entry3 = NULL;
+  ASSERT_EQ(net::OK, OpenEntry(key, &entry3));
+  ScopedEntryPtr entry3_closer(entry3);
+  EXPECT_NE(null, entry3);
+}
+
 // Checks that an optimistic Create would fail later on a racing Open.
 TEST_F(DiskCacheEntryTest, SimpleCacheOptimisticCreateFailsOnOpen) {
   SetSimpleCacheMode();
diff --git a/net/disk_cache/simple/simple_entry_impl.cc b/net/disk_cache/simple/simple_entry_impl.cc
index 4823dda..3a54a1f 100644
--- a/net/disk_cache/simple/simple_entry_impl.cc
+++ b/net/disk_cache/simple/simple_entry_impl.cc
@@ -262,6 +262,8 @@ int SimpleEntryImpl::CreateEntry(Entry** out_entry,
 }
 
 int SimpleEntryImpl::DoomEntry(const CompletionCallback& callback) {
+  if (doomed_)
+    return net::OK;
   net_log_.AddEvent(net::NetLog::TYPE_SIMPLE_CACHE_ENTRY_DOOM_CALL);
   net_log_.AddEvent(net::NetLog::TYPE_SIMPLE_CACHE_ENTRY_DOOM_BEGIN);
 
@@ -558,9 +560,9 @@ void SimpleEntryImpl::RemoveSelfFromBackend() {
 }
 
 void SimpleEntryImpl::MarkAsDoomed() {
+  doomed_ = true;
   if (!backend_.get())
     return;
-  doomed_ = true;
   backend_->index()->Remove(entry_hash_);
   RemoveSelfFromBackend();
 }
diff --git a/net/disk_cache/simple/simple_entry_impl.h b/net/disk_cache/simple/simple_entry_impl.h
index c6ce73a..d7d9b8f 100644
--- a/net/disk_cache/simple/simple_entry_impl.h
+++ b/net/disk_cache/simple/simple_entry_impl.h
@@ -14,6 +14,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "net/base/cache_type.h"
+#include "net/base/net_export.h"
 #include "net/base/net_log.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/disk_cache/simple/simple_entry_format.h"
@@ -37,7 +38,8 @@ struct SimpleEntryCreationResults;
 // 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 NET_EXPORT_PRIVATE SimpleEntryImpl : public Entry,
+    public base::RefCounted<SimpleEntryImpl>,
     public base::SupportsWeakPtr<SimpleEntryImpl> {
   friend class base::RefCounted<SimpleEntryImpl>;
  public: