Don't doom the wrong simple cache entry.

net/disk_cache/entry_unittest.cc

Index: net/disk_cache/entry_unittest.cc
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 56371437da3dbbd6950d7819d9c0b028f3cfdf65..e41fd650241f85cdcd780f8705b21b02495fef12 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,43 @@ 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.

[pasko, 2013/09/12 10:46:48]

do we want more sequences like that in the following CLs to be even more sure?
what would they be?

[gavinp, 2013/09/12 15:39:48]

I'm not sure.

+  SetSimpleCacheMode();
+  InitCache();
+  disk_cache::Entry* null = NULL;
+
+  const char key[] = "the first key";
+
+  net::TestCompletionCallback cb;
+  disk_cache::Entry* entry1 = NULL;
+  ASSERT_EQ(net::OK,
+            cb.GetResult(cache_->CreateEntry(key, &entry1, cb.callback())));

[pasko, 2013/09/12 10:46:48]

The common style in all disk_cache testing is to write:
ASSERT_EQ(net::OK, CreateEntry(key, &entry1));

please use this pattern also in the test above. We should use  cache_->Method()
only when we do not want to wait for result, like testing optimistic operations.

[gavinp, 2013/09/12 15:39:48]

Done.

+  ScopedEntryPtr entry1_closer(entry1);
+  EXPECT_NE(null, entry1);
+
+  EXPECT_EQ(net::OK,
+            cb.GetResult(cache_->DoomEntry(key, cb.callback())));
+
+  disk_cache::Entry* entry2 = NULL;
+  ASSERT_EQ(net::OK,
+            cb.GetResult(cache_->CreateEntry(key, &entry2, cb.callback())));
+  ScopedEntryPtr entry2_closer(entry2);
+  EXPECT_NE(null, entry2);
+
+  disk_cache::SimpleEntryImpl* simple_entry1 =
+      static_cast<disk_cache::SimpleEntryImpl*>(entry1);

[pasko, 2013/09/12 10:46:48]

why casting? could you just call Doom() on the entry or DoomEntry() on the
backend?

[gavinp, 2013/09/12 15:39:48]

I want to wait for the callback, so I know this doom is complete before I
attempt to OpenEntry below; I'm trying to make sure entry2 isn't being doomed.

I added a comment.

+  EXPECT_EQ(net::OK,
+            cb.GetResult(simple_entry1->DoomEntry(cb.callback())));
+
+  disk_cache::Entry* entry3 = NULL;
+  ASSERT_EQ(net::OK,
+            cb.GetResult(cache_->OpenEntry(key, &entry3, cb.callback())));
+  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();