Disk cache: For an AppCache, now we only update the

LRU list when an entry is created. This means that
we don't update the list anymore when an entry is
accessed, even if we are writing to the entry.

The general idea is that now we should be able to
open an AppCache and read from it without modifying
the contents of the cache, so that if the browser
crashes, we won't find "dirty" entries to discard.

By minimizing writes to the LRU list, we reduce the
chances that the list will get corrupt beyond the
point where we cannot trust it anymore if the whole
system crashes (and not just the browser).

BUG=51870
TEST=net_unittests


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

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

11 files changed, 189 insertions, 33 deletions

diff --git a/net/data/heapcheck/net_unittests.gtest.txt b/net/data/heapcheck/net_unittests.gtest.txt
index afc1db3..fb1a17b 100644
--- a/net/data/heapcheck/net_unittests.gtest.txt
+++ b/net/data/heapcheck/net_unittests.gtest.txt
@@ -9,15 +9,18 @@ URLRequestTestHTTP.*
 CertDatabaseNSSTest.ImportFromPKCS12*
 # Exclude tests that simulate a crash: they leak a lot of stuff.
 # Note that *InvalidEntry* is too general.
+DiskCacheBackendTest.AppCacheInvalidEntry
+DiskCacheBackendTest.AppCacheInvalidEntryRead
+DiskCacheBackendTest.AppCacheInvalidEntryWithLoad
 DiskCacheBackendTest.InvalidEntry
-DiskCacheBackendTest.NewEvictionInvalidEntry
+DiskCacheBackendTest.InvalidEntryEnumeration
 DiskCacheBackendTest.InvalidEntryRead
-DiskCacheBackendTest.NewEvictionInvalidEntryRead
 DiskCacheBackendTest.InvalidEntryWithLoad
-DiskCacheBackendTest.NewEvictionInvalidEntryWithLoad
 DiskCacheBackendTest.TrimInvalidEntry
-DiskCacheBackendTest.NewEvictionTrimInvalidEntry
 DiskCacheBackendTest.TrimInvalidEntry2
-DiskCacheBackendTest.NewEvictionTrimInvalidEntry2
-DiskCacheBackendTest.InvalidEntryEnumeration
+DiskCacheBackendTest.NewEvictionInvalidEntry
 DiskCacheBackendTest.NewEvictionInvalidEntryEnumeration
+DiskCacheBackendTest.NewEvictionInvalidEntryRead
+DiskCacheBackendTest.NewEvictionInvalidEntryWithLoad
+DiskCacheBackendTest.NewEvictionTrimInvalidEntry
+DiskCacheBackendTest.NewEvictionTrimInvalidEntry2
diff --git a/net/data/valgrind/net_unittests.gtest-memcheck.txt b/net/data/valgrind/net_unittests.gtest-memcheck.txt
index 458ddef..e7f42dd 100644
--- a/net/data/valgrind/net_unittests.gtest-memcheck.txt
+++ b/net/data/valgrind/net_unittests.gtest-memcheck.txt
@@ -2,6 +2,9 @@
 # Similar list in ../purify/net_unittests.exe.gtest.txt
 # TODO(dkegel): either merge the two files or keep them in sync,
 # see http://code.google.com/p/chromium/issues/detail?id=8951
+DiskCacheBackendTest.AppCacheInvalidEntry
+DiskCacheBackendTest.AppCacheInvalidEntryRead
+DiskCacheBackendTest.AppCacheInvalidEntryWithLoad
 DiskCacheBackendTest.InvalidEntry
 DiskCacheBackendTest.InvalidEntryRead
 DiskCacheBackendTest.InvalidEntryWithLoad
diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index 2b6d3af..f518834 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -511,6 +511,12 @@ int BackendImpl::SyncInit() {
   if (!block_files_.Init(create_files))
     return net::ERR_FAILED;
 
+  // We want to minimize the changes to cache for an AppCache.
+  if (cache_type() == net::APP_CACHE) {
+    DCHECK(!new_eviction_);
+    read_only_ = true;
+  }
+
   // stats_ and rankings_ may end up calling back to us so we better be enabled.
   disabled_ = false;
   if (!stats_.Init(this, &data_->header.stats))
@@ -597,6 +603,7 @@ int BackendImpl::SyncDoomAllEntries() {
 
 int BackendImpl::SyncDoomEntriesBetween(const base::Time initial_time,
                                         const base::Time end_time) {
+  DCHECK_NE(net::APP_CACHE, cache_type_);
   if (end_time.is_null())
     return SyncDoomEntriesSince(initial_time);
 
@@ -634,6 +641,7 @@ int BackendImpl::SyncDoomEntriesBetween(const base::Time initial_time,
 // We use OpenNextEntryImpl to retrieve elements from the cache, until we get
 // entries that are too old.
 int BackendImpl::SyncDoomEntriesSince(const base::Time initial_time) {
+  DCHECK_NE(net::APP_CACHE, cache_type_);
   if (disabled_)
     return net::ERR_FAILED;
 
@@ -744,7 +752,8 @@ EntryImpl* BackendImpl::CreateEntryImpl(const std::string& key) {
     return NULL;
   }
 
-  scoped_refptr<EntryImpl> cache_entry(new EntryImpl(this, entry_address));
+  scoped_refptr<EntryImpl> cache_entry(
+      new EntryImpl(this, entry_address, false));
   IncreaseNumRefs();
 
   if (!cache_entry->CreateEntry(node_address, key, hash)) {
@@ -1342,7 +1351,8 @@ int BackendImpl::NewEntry(Addr address, EntryImpl** entry, bool* dirty) {
     return 0;
   }
 
-  scoped_refptr<EntryImpl> cache_entry(new EntryImpl(this, address));
+  scoped_refptr<EntryImpl> cache_entry(
+      new EntryImpl(this, address, read_only_));
   IncreaseNumRefs();
   *entry = NULL;
 
@@ -1660,7 +1670,7 @@ void BackendImpl::AddStorageSize(int32 bytes) {
   data_->header.num_bytes += bytes;
   DCHECK_GE(data_->header.num_bytes, 0);
 
-  if (data_->header.num_bytes > max_size_)
+  if (data_->header.num_bytes > max_size_ && !read_only_)
     eviction_.TrimCache(false);
 }
 
diff --git a/net/disk_cache/backend_unittest.cc b/net/disk_cache/backend_unittest.cc
index 727b29d..57c3187 100644
--- a/net/disk_cache/backend_unittest.cc
+++ b/net/disk_cache/backend_unittest.cc
@@ -113,6 +113,11 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyBasics) {
   BackendBasics();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheBasics) {
+  SetCacheType(net::APP_CACHE);
+  BackendBasics();
+}
+
 void DiskCacheBackendTest::BackendKeying() {
   InitCache();
   const char* kName1 = "the first key";
@@ -168,6 +173,11 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyKeying) {
   BackendKeying();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheKeying) {
+  SetCacheType(net::APP_CACHE);
+  BackendKeying();
+}
+
 TEST_F(DiskCacheTest, CreateBackend) {
   TestCompletionCallback cb;
 
@@ -466,6 +476,14 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyLoad) {
   BackendLoad();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheLoad) {
+  SetCacheType(net::APP_CACHE);
+  // Work with a tiny index table (16 entries)
+  SetMask(0xf);
+  SetMaxSize(0x100000);
+  BackendLoad();
+}
+
 // Before looking for invalid entries, let's check a valid entry.
 void DiskCacheBackendTest::BackendValidEntry() {
   SetDirectMode();
@@ -540,6 +558,12 @@ TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntry) {
   BackendInvalidEntry();
 }
 
+// We'll be leaking memory from this test.
+TEST_F(DiskCacheBackendTest, AppCacheInvalidEntry) {
+  SetCacheType(net::APP_CACHE);
+  BackendInvalidEntry();
+}
+
 // Almost the same test, but this time crash the cache after reading an entry.
 // We'll be leaking memory from this test.
 void DiskCacheBackendTest::BackendInvalidEntryRead() {
@@ -562,8 +586,15 @@ void DiskCacheBackendTest::BackendInvalidEntryRead() {
 
   SimulateCrash();
 
-  EXPECT_NE(net::OK, OpenEntry(key, &entry));
-  EXPECT_EQ(0, cache_->GetEntryCount());
+  if (type_ == net::APP_CACHE) {
+    // Reading an entry and crashing should not make it dirty.
+    ASSERT_EQ(net::OK, OpenEntry(key, &entry));
+    EXPECT_EQ(1, cache_->GetEntryCount());
+    entry->Close();
+  } else {
+    EXPECT_NE(net::OK, OpenEntry(key, &entry));
+    EXPECT_EQ(0, cache_->GetEntryCount());
+  }
 }
 
 // We'll be leaking memory from this test.
@@ -578,6 +609,12 @@ TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntryRead) {
 }
 
 // We'll be leaking memory from this test.
+TEST_F(DiskCacheBackendTest, AppCacheInvalidEntryRead) {
+  SetCacheType(net::APP_CACHE);
+  BackendInvalidEntryRead();
+}
+
+// We'll be leaking memory from this test.
 void DiskCacheBackendTest::BackendInvalidEntryWithLoad() {
   // Work with a tiny index table (16 entries)
   SetMask(0xf);
@@ -638,6 +675,12 @@ TEST_F(DiskCacheBackendTest, NewEvictionInvalidEntryWithLoad) {
 }
 
 // We'll be leaking memory from this test.
+TEST_F(DiskCacheBackendTest, AppCacheInvalidEntryWithLoad) {
+  SetCacheType(net::APP_CACHE);
+  BackendInvalidEntryWithLoad();
+}
+
+// We'll be leaking memory from this test.
 void DiskCacheBackendTest::BackendTrimInvalidEntry() {
   // Use the implementation directly... we need to simulate a crash.
   SetDirectMode();
@@ -807,6 +850,11 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyEnumerations) {
   BackendEnumerations();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheEnumerations) {
+  SetCacheType(net::APP_CACHE);
+  BackendEnumerations();
+}
+
 // Verifies enumerations while entries are open.
 void DiskCacheBackendTest::BackendEnumerations2() {
   InitCache();
@@ -823,7 +871,7 @@ void DiskCacheBackendTest::BackendEnumerations2() {
   ASSERT_EQ(net::OK, OpenEntry(second, &entry1));
   void* iter = NULL;
   ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
-  ASSERT_EQ(entry2->GetKey(), second);
+  EXPECT_EQ(entry2->GetKey(), second);
 
   // Two entries and the iterator pointing at "first".
   entry1->Close();
@@ -831,7 +879,22 @@ void DiskCacheBackendTest::BackendEnumerations2() {
 
   // The iterator should still be valid, so we should not crash.
   ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
-  ASSERT_EQ(entry2->GetKey(), first);
+  EXPECT_EQ(entry2->GetKey(), first);
+  entry2->Close();
+  cache_->EndEnumeration(&iter);
+
+  // Modify the oldest entry and get the newest element.
+  ASSERT_EQ(net::OK, OpenEntry(first, &entry1));
+  EXPECT_EQ(0, WriteData(entry1, 0, 200, NULL, 0, false));
+  ASSERT_EQ(net::OK, OpenNextEntry(&iter, &entry2));
+  if (type_ == net::APP_CACHE) {
+    // The list is not updated.
+    EXPECT_EQ(entry2->GetKey(), second);
+  } else {
+    EXPECT_EQ(entry2->GetKey(), first);
+  }
+
+  entry1->Close();
   entry2->Close();
   cache_->EndEnumeration(&iter);
 }
@@ -850,6 +913,10 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyEnumerations2) {
   BackendEnumerations2();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheEnumerations2) {
+  SetCacheType(net::APP_CACHE);
+  BackendEnumerations2();
+}
 
 // Verify handling of invalid entries while doing enumerations.
 // We'll be leaking memory from this test.
@@ -1622,6 +1689,11 @@ TEST_F(DiskCacheBackendTest, MemoryOnlyDoomAll) {
   BackendDoomAll();
 }
 
+TEST_F(DiskCacheBackendTest, AppCacheOnlyDoomAll) {
+  SetCacheType(net::APP_CACHE);
+  BackendDoomAll();
+}
+
 // If the index size changes when we doom the cache, we should not crash.
 void DiskCacheBackendTest::BackendDoomAll2() {
   EXPECT_EQ(2, cache_->GetEntryCount());
diff --git a/net/disk_cache/block_files.cc b/net/disk_cache/block_files.cc
index 48e5be3..9f6c489 100644
--- a/net/disk_cache/block_files.cc
+++ b/net/disk_cache/block_files.cc
@@ -503,7 +503,7 @@ bool BlockFiles::FixBlockFileHeader(MappedFile* file) {
   return true;
 }
 
-// We are interested in the total number of block used by this file type, and
+// We are interested in the total number of blocks used by this file type, and
 // the max number of blocks that we can store (reported as the percentage of
 // used blocks). In order to find out the number of used blocks, we have to
 // substract the empty blocks from the total blocks for each file in the chain.
diff --git a/net/disk_cache/disk_cache_test_base.cc b/net/disk_cache/disk_cache_test_base.cc
index 9e7d98f6..3edfffd 100644
--- a/net/disk_cache/disk_cache_test_base.cc
+++ b/net/disk_cache/disk_cache_test_base.cc
@@ -74,7 +74,7 @@ void DiskCacheTestWithCache::InitDiskCache() {
 
   TestCompletionCallback cb;
   int rv = disk_cache::BackendImpl::CreateBackend(
-               path, force_creation_, size_, net::DISK_CACHE,
+               path, force_creation_, size_, type_,
                disk_cache::kNoRandom, thread, &cache_, &cb);
   ASSERT_EQ(net::OK, cb.GetResult(rv));
 }
@@ -97,6 +97,7 @@ void DiskCacheTestWithCache::InitDiskCacheImpl(const FilePath& path) {
   if (new_eviction_)
     cache_impl_->SetNewEviction();
 
+  cache_impl_->SetType(type_);
   cache_impl_->SetFlags(disk_cache::kNoRandom);
   TestCompletionCallback cb;
   int rv = cache_impl_->Init(&cb);
diff --git a/net/disk_cache/disk_cache_test_base.h b/net/disk_cache/disk_cache_test_base.h
index a647d97..80c50a1 100644
--- a/net/disk_cache/disk_cache_test_base.h
+++ b/net/disk_cache/disk_cache_test_base.h
@@ -8,6 +8,7 @@
 
 #include "base/basictypes.h"
 #include "base/thread.h"
+#include "net/base/cache_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -41,9 +42,10 @@ class DiskCacheTestWithCache : public DiskCacheTest {
  protected:
   DiskCacheTestWithCache()
       : cache_(NULL), cache_impl_(NULL), mem_cache_(NULL), mask_(0), size_(0),
-        memory_only_(false), implementation_(false), force_creation_(false),
-        new_eviction_(false), first_cleanup_(true), integrity_(true),
-        use_current_thread_(false), cache_thread_("CacheThread") {}
+        type_(net::DISK_CACHE), memory_only_(false), implementation_(false),
+        force_creation_(false), new_eviction_(false), first_cleanup_(true),
+        integrity_(true), use_current_thread_(false),
+        cache_thread_("CacheThread") {}
 
   void InitCache();
   virtual void TearDown();
@@ -86,6 +88,10 @@ class DiskCacheTestWithCache : public DiskCacheTest {
     use_current_thread_ = true;
   }
 
+  void SetCacheType(net::CacheType type) {
+    type_ = type;
+  }
+
   // Utility methods to access the cache and wait for each operation to finish.
   int OpenEntry(const std::string& key, disk_cache::Entry** entry);
   int CreateEntry(const std::string& key, disk_cache::Entry** entry);
@@ -97,7 +103,7 @@ class DiskCacheTestWithCache : public DiskCacheTest {
   int OpenNextEntry(void** iter, disk_cache::Entry** next_entry);
   void FlushQueueForTest();
   int ReadData(disk_cache::Entry* entry, int index, int offset,
-                net::IOBuffer* buf, int len);
+               net::IOBuffer* buf, int len);
   int WriteData(disk_cache::Entry* entry, int index, int offset,
                 net::IOBuffer* buf, int len, bool truncate);
   int ReadSparseData(disk_cache::Entry* entry, int64 offset, net::IOBuffer* buf,
@@ -113,6 +119,7 @@ class DiskCacheTestWithCache : public DiskCacheTest {
 
   uint32 mask_;
   int size_;
+  net::CacheType type_;
   bool memory_only_;
   bool implementation_;
   bool force_creation_;
diff --git a/net/disk_cache/entry_impl.cc b/net/disk_cache/entry_impl.cc
index 8f514b0..f152f10 100644
--- a/net/disk_cache/entry_impl.cc
+++ b/net/disk_cache/entry_impl.cc
@@ -286,8 +286,8 @@ bool EntryImpl::UserBuffer::GrowBuffer(int required, int limit) {
 
 // ------------------------------------------------------------------------
 
-EntryImpl::EntryImpl(BackendImpl* backend, Addr address)
-    : entry_(NULL, Addr(0)), node_(NULL, Addr(0)) {
+EntryImpl::EntryImpl(BackendImpl* backend, Addr address, bool read_only)
+    : entry_(NULL, Addr(0)), node_(NULL, Addr(0)), read_only_(read_only) {
   entry_.LazyInit(backend->File(address), address);
   doomed_ = false;
   backend_ = backend;
@@ -394,7 +394,7 @@ int EntryImpl::ReadData(int index, int offset, net::IOBuffer* buf, int buf_len,
   if (!callback)
     return ReadDataImpl(index, offset, buf, buf_len, callback);
 
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   if (index < 0 || index >= kNumStreams)
     return net::ERR_INVALID_ARGUMENT;
 
@@ -415,7 +415,7 @@ int EntryImpl::WriteData(int index, int offset, net::IOBuffer* buf, int buf_len,
   if (!callback)
     return WriteDataImpl(index, offset, buf, buf_len, callback, truncate);
 
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   if (index < 0 || index >= kNumStreams)
     return net::ERR_INVALID_ARGUMENT;
 
@@ -487,7 +487,7 @@ void EntryImpl::DoomImpl() {
 
 int EntryImpl::ReadDataImpl(int index, int offset, net::IOBuffer* buf,
                             int buf_len, CompletionCallback* callback) {
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   if (index < 0 || index >= kNumStreams)
     return net::ERR_INVALID_ARGUMENT;
 
@@ -553,7 +553,7 @@ int EntryImpl::ReadDataImpl(int index, int offset, net::IOBuffer* buf,
 int EntryImpl::WriteDataImpl(int index, int offset, net::IOBuffer* buf,
                              int buf_len, CompletionCallback* callback,
                              bool truncate) {
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   if (index < 0 || index >= kNumStreams)
     return net::ERR_INVALID_ARGUMENT;
 
@@ -639,7 +639,7 @@ int EntryImpl::WriteDataImpl(int index, int offset, net::IOBuffer* buf,
 
 int EntryImpl::ReadSparseDataImpl(int64 offset, net::IOBuffer* buf, int buf_len,
                                   CompletionCallback* callback) {
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   int result = InitSparseData();
   if (net::OK != result)
     return result;
@@ -653,7 +653,7 @@ int EntryImpl::ReadSparseDataImpl(int64 offset, net::IOBuffer* buf, int buf_len,
 
 int EntryImpl::WriteSparseDataImpl(int64 offset, net::IOBuffer* buf,
                                    int buf_len, CompletionCallback* callback) {
-  DCHECK(node_.Data()->dirty);
+  DCHECK(node_.Data()->dirty || read_only_);
   int result = InitSparseData();
   if (net::OK != result)
     return result;
@@ -820,6 +820,9 @@ bool EntryImpl::LoadNodeAddress() {
 bool EntryImpl::Update() {
   DCHECK(node_.HasData());
 
+  if (read_only_)
+    return true;
+
   RankingsNode* rankings = node_.Data();
   if (!rankings->dirty) {
     rankings->dirty = backend_->GetCurrentEntryId();
@@ -956,7 +959,7 @@ void EntryImpl::DeleteData(Addr address, int index) {
 void EntryImpl::UpdateRank(bool modified) {
   if (!doomed_) {
     // Everything is handled by the backend.
-    backend_->UpdateRank(this, true);
+    backend_->UpdateRank(this, modified);
     return;
   }
 
diff --git a/net/disk_cache/entry_impl.h b/net/disk_cache/entry_impl.h
index 979e066..b3481f0 100644
--- a/net/disk_cache/entry_impl.h
+++ b/net/disk_cache/entry_impl.h
@@ -30,7 +30,7 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
     kAsyncIO
   };
 
-  EntryImpl(BackendImpl* backend, Addr address);
+  EntryImpl(BackendImpl* backend, Addr address, bool read_only);
 
   // Entry interface.
   virtual void Doom();
@@ -213,6 +213,7 @@ class EntryImpl : public Entry, public base::RefCounted<EntryImpl> {
   mutable std::string key_;           // Copy of the key.
   int unreported_size_[kNumStreams];  // Bytes not reported yet to the backend.
   bool doomed_;               // True if this entry was removed from the cache.
+  bool read_only_;            // True if not yet writing.
   scoped_ptr<SparseControl> sparse_;  // Support for sparse entries.
 
   DISALLOW_COPY_AND_ASSIGN(EntryImpl);
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 1ff033c..8799ed8 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -29,6 +29,7 @@ class DiskCacheEntryTest : public DiskCacheTestWithCache {
   void ExternalAsyncIO();
   void StreamAccess();
   void GetKey();
+  void GetTimes();
   void GrowData();
   void TruncateData();
   void ZeroLengthIO();
@@ -490,6 +491,59 @@ TEST_F(DiskCacheEntryTest, MemoryOnlyGetKey) {
   GetKey();
 }
 
+void DiskCacheEntryTest::GetTimes() {
+  std::string key("the first key");
+  disk_cache::Entry* entry;
+
+  Time t1 = Time::Now();
+  ASSERT_EQ(net::OK, CreateEntry(key, &entry));
+  EXPECT_TRUE(entry->GetLastModified() >= t1);
+  EXPECT_TRUE(entry->GetLastModified() == entry->GetLastUsed());
+
+  PlatformThread::Sleep(20);
+  Time t2 = Time::Now();
+  EXPECT_TRUE(t2 > t1);
+  EXPECT_EQ(0, WriteData(entry, 0, 200, NULL, 0, false));
+  if (type_ == net::APP_CACHE) {
+    EXPECT_TRUE(entry->GetLastModified() < t2);
+  } else {
+    EXPECT_TRUE(entry->GetLastModified() >= t2);
+  }
+  EXPECT_TRUE(entry->GetLastModified() == entry->GetLastUsed());
+
+  PlatformThread::Sleep(20);
+  Time t3 = Time::Now();
+  EXPECT_TRUE(t3 > t2);
+  const int kSize = 200;
+  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(kSize);
+  EXPECT_EQ(kSize, ReadData(entry, 0, 0, buffer, kSize));
+  if (type_ == net::APP_CACHE) {
+    EXPECT_TRUE(entry->GetLastUsed() < t2);
+    EXPECT_TRUE(entry->GetLastModified() < t2);
+  } else {
+    EXPECT_TRUE(entry->GetLastUsed() >= t3);
+    EXPECT_TRUE(entry->GetLastModified() < t3);
+  }
+  entry->Close();
+}
+
+TEST_F(DiskCacheEntryTest, GetTimes) {
+  InitCache();
+  GetTimes();
+}
+
+TEST_F(DiskCacheEntryTest, MemoryOnlyGetTimes) {
+  SetMemoryOnlyMode();
+  InitCache();
+  GetTimes();
+}
+
+TEST_F(DiskCacheEntryTest, AppCacheGetTimes) {
+  SetCacheType(net::APP_CACHE);
+  InitCache();
+  GetTimes();
+}
+
 void DiskCacheEntryTest::GrowData() {
   std::string key1("the first key");
   disk_cache::Entry* entry;
diff --git a/net/disk_cache/mem_entry_impl.cc b/net/disk_cache/mem_entry_impl.cc
index 645619e..8d8bb4c 100644
--- a/net/disk_cache/mem_entry_impl.cc
+++ b/net/disk_cache/mem_entry_impl.cc
@@ -326,8 +326,9 @@ int MemEntryImpl::ReadyForSparseIO(
 
 bool MemEntryImpl::CreateEntry(const std::string& key) {
   key_ = key;
-  last_modified_ = Time::Now();
-  last_used_ = Time::Now();
+  Time current = Time::Now();
+  last_modified_ = current;
+  last_used_ = current;
   Open();
   backend_->ModifyStorageSize(0, static_cast<int32>(key.size()));
   return true;
@@ -431,8 +432,9 @@ bool MemEntryImpl::InitChildEntry(MemEntryImpl* parent, int child_id) {
   DCHECK(!child_id_);
   parent_ = parent;
   child_id_ = child_id;
-  last_modified_ = Time::Now();
-  last_used_ = Time::Now();
+  Time current = Time::Now();
+  last_modified_ = current;
+  last_used_ = current;
   // Insert this to the backend's ranking list.
   backend_->InsertIntoRankingList(this);
   return true;