Bug 1000338 - nsICacheEntry.lastModified not properly implemented, r=michal
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 16 Sep 2014 15:51:50 +0200
changeset 230162 6d54ee2b5a829d1c472ef5abc8a86f4ede4a3609
parent 230161 64be5e76ca62c295922295fb6a1a2a44abace8fc
child 230163 5780024ea016d3e8b44dbde2c4f1d88ba2b5fb20
push id611
push userraliiev@mozilla.com
push dateMon, 05 Jan 2015 23:23:16 +0000
treeherdermozilla-release@345cd3b9c445 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs1000338
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1000338 - nsICacheEntry.lastModified not properly implemented, r=michal
netwerk/cache2/CacheEntry.cpp
netwerk/cache2/CacheFile.cpp
netwerk/cache2/CacheFile.h
netwerk/cache2/CacheFileMetadata.cpp
netwerk/cache2/CacheFileMetadata.h
netwerk/test/unit/test_cache2-28-last-access-attrs.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cache2/CacheEntry.cpp
+++ b/netwerk/cache2/CacheEntry.cpp
@@ -754,16 +754,21 @@ void CacheEntry::InvokeAvailableCallback
     nsRefPtr<AvailableCallbackRunnable> event =
       new AvailableCallbackRunnable(this, aCallback);
 
     rv = aCallback.mTargetThread->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL);
     LOG(("  redispatched, (rv = 0x%08x)", rv));
     return;
   }
 
+  if (NS_SUCCEEDED(mFileStatus)) {
+    // Let the last-fetched and fetch-count properties be updated.
+    mFile->OnFetched();
+  }
+
   if (mIsDoomed || aCallback.mNotWanted) {
     LOG(("  doomed or not wanted, notifying OCEA with NS_ERROR_CACHE_KEY_NOT_FOUND"));
     aCallback.mCallback->OnCacheEntryAvailable(
       nullptr, false, nullptr, NS_ERROR_CACHE_KEY_NOT_FOUND);
     return;
   }
 
   if (state == READY) {
--- a/netwerk/cache2/CacheFile.cpp
+++ b/netwerk/cache2/CacheFile.cpp
@@ -906,37 +906,16 @@ CacheFile::GetExpirationTime(uint32_t *_
   CacheFileAutoLock lock(this);
   MOZ_ASSERT(mMetadata);
   NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
 
   return mMetadata->GetExpirationTime(_retval);
 }
 
 nsresult
-CacheFile::SetLastModified(uint32_t aLastModified)
-{
-  CacheFileAutoLock lock(this);
-  MOZ_ASSERT(mMetadata);
-  NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
-
-  PostWriteTimer();
-  return mMetadata->SetLastModified(aLastModified);
-}
-
-nsresult
-CacheFile::GetLastModified(uint32_t *_retval)
-{
-  CacheFileAutoLock lock(this);
-  MOZ_ASSERT(mMetadata);
-  NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
-
-  return mMetadata->GetLastModified(_retval);
-}
-
-nsresult
 CacheFile::SetFrecency(uint32_t aFrecency)
 {
   CacheFileAutoLock lock(this);
   MOZ_ASSERT(mMetadata);
   NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
 
   PostWriteTimer();
 
@@ -952,16 +931,26 @@ CacheFile::GetFrecency(uint32_t *_retval
   CacheFileAutoLock lock(this);
   MOZ_ASSERT(mMetadata);
   NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
 
   return mMetadata->GetFrecency(_retval);
 }
 
 nsresult
+CacheFile::GetLastModified(uint32_t *_retval)
+{
+  CacheFileAutoLock lock(this);
+  MOZ_ASSERT(mMetadata);
+  NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
+
+  return mMetadata->GetLastModified(_retval);
+}
+
+nsresult
 CacheFile::GetLastFetched(uint32_t *_retval)
 {
   CacheFileAutoLock lock(this);
   MOZ_ASSERT(mMetadata);
   NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
 
   return mMetadata->GetLastFetched(_retval);
 }
@@ -971,16 +960,28 @@ CacheFile::GetFetchCount(uint32_t *_retv
 {
   CacheFileAutoLock lock(this);
   MOZ_ASSERT(mMetadata);
   NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
 
   return mMetadata->GetFetchCount(_retval);
 }
 
+nsresult
+CacheFile::OnFetched()
+{
+  CacheFileAutoLock lock(this);
+  MOZ_ASSERT(mMetadata);
+  NS_ENSURE_TRUE(mMetadata, NS_ERROR_UNEXPECTED);
+
+  PostWriteTimer();
+
+  return mMetadata->OnFetched();
+}
+
 void
 CacheFile::Lock()
 {
   mLock.Lock();
 }
 
 void
 CacheFile::Unlock()
--- a/netwerk/cache2/CacheFile.h
+++ b/netwerk/cache2/CacheFile.h
@@ -85,22 +85,24 @@ public:
 
   // metadata forwarders
   nsresult GetElement(const char *aKey, char **_retval);
   nsresult SetElement(const char *aKey, const char *aValue);
   nsresult VisitMetaData(nsICacheEntryMetaDataVisitor *aVisitor);
   nsresult ElementsSize(uint32_t *_retval);
   nsresult SetExpirationTime(uint32_t aExpirationTime);
   nsresult GetExpirationTime(uint32_t *_retval);
-  nsresult SetLastModified(uint32_t aLastModified);
-  nsresult GetLastModified(uint32_t *_retval);
   nsresult SetFrecency(uint32_t aFrecency);
   nsresult GetFrecency(uint32_t *_retval);
+  nsresult GetLastModified(uint32_t *_retval);
   nsresult GetLastFetched(uint32_t *_retval);
   nsresult GetFetchCount(uint32_t *_retval);
+  // Called by upper layers to indicated the entry has been fetched,
+  // i.e. delivered to the consumer.
+  nsresult OnFetched();
 
   bool DataSize(int64_t* aSize);
   void Key(nsACString& aKey) { aKey = mKey; }
   bool IsDoomed();
   bool IsWriteInProgress();
 
   // Memory reporting
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
--- a/netwerk/cache2/CacheFileMetadata.cpp
+++ b/netwerk/cache2/CacheFileMetadata.cpp
@@ -22,16 +22,18 @@
 namespace mozilla {
 namespace net {
 
 #define kMinMetadataRead 1024  // TODO find optimal value from telemetry
 #define kAlignSize       4096
 
 #define kCacheEntryVersion 1
 
+#define NOW_SECONDS() (uint32_t(PR_Now() / PR_USEC_PER_SEC))
+
 NS_IMPL_ISUPPORTS(CacheFileMetadata, CacheFileIOListener)
 
 CacheFileMetadata::CacheFileMetadata(CacheFileHandle *aHandle, const nsACString &aKey)
   : CacheMemoryConsumer(NORMAL)
   , mHandle(aHandle)
   , mHashArray(nullptr)
   , mHashArraySize(0)
   , mHashCount(0)
@@ -77,17 +79,16 @@ CacheFileMetadata::CacheFileMetadata(boo
 {
   LOG(("CacheFileMetadata::CacheFileMetadata() [this=%p, key=%s]",
        this, PromiseFlatCString(aKey).get()));
 
   MOZ_COUNT_CTOR(CacheFileMetadata);
   memset(&mMetaHdr, 0, sizeof(CacheFileMetadataHeader));
   mMetaHdr.mVersion = kCacheEntryVersion;
   mMetaHdr.mExpirationTime = nsICacheEntry::NO_EXPIRATION_TIME;
-  mMetaHdr.mFetchCount = 1;
   mKey = aKey;
   mMetaHdr.mKeySize = mKey.Length();
 
   DebugOnly<nsresult> rv;
   rv = ParseKey(aKey);
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
@@ -499,79 +500,87 @@ CacheFileMetadata::SetHash(uint32_t aInd
 }
 
 nsresult
 CacheFileMetadata::SetExpirationTime(uint32_t aExpirationTime)
 {
   LOG(("CacheFileMetadata::SetExpirationTime() [this=%p, expirationTime=%d]",
        this, aExpirationTime));
 
-  MarkDirty();
+  MarkDirty(false);
   mMetaHdr.mExpirationTime = aExpirationTime;
   return NS_OK;
 }
 
 nsresult
 CacheFileMetadata::GetExpirationTime(uint32_t *_retval)
 {
   *_retval = mMetaHdr.mExpirationTime;
   return NS_OK;
 }
 
 nsresult
-CacheFileMetadata::SetLastModified(uint32_t aLastModified)
-{
-  LOG(("CacheFileMetadata::SetLastModified() [this=%p, lastModified=%d]",
-       this, aLastModified));
-
-  MarkDirty();
-  mMetaHdr.mLastModified = aLastModified;
-  return NS_OK;
-}
-
-nsresult
-CacheFileMetadata::GetLastModified(uint32_t *_retval)
-{
-  *_retval = mMetaHdr.mLastModified;
-  return NS_OK;
-}
-
-nsresult
 CacheFileMetadata::SetFrecency(uint32_t aFrecency)
 {
   LOG(("CacheFileMetadata::SetFrecency() [this=%p, frecency=%f]",
        this, (double)aFrecency));
 
-  MarkDirty();
+  MarkDirty(false);
   mMetaHdr.mFrecency = aFrecency;
   return NS_OK;
 }
 
 nsresult
 CacheFileMetadata::GetFrecency(uint32_t *_retval)
 {
   *_retval = mMetaHdr.mFrecency;
   return NS_OK;
 }
 
 nsresult
+CacheFileMetadata::GetLastModified(uint32_t *_retval)
+{
+  *_retval = mMetaHdr.mLastModified;
+  return NS_OK;
+}
+
+nsresult
 CacheFileMetadata::GetLastFetched(uint32_t *_retval)
 {
   *_retval = mMetaHdr.mLastFetched;
   return NS_OK;
 }
 
 nsresult
 CacheFileMetadata::GetFetchCount(uint32_t *_retval)
 {
   *_retval = mMetaHdr.mFetchCount;
   return NS_OK;
 }
 
 nsresult
+CacheFileMetadata::OnFetched()
+{
+  MarkDirty(false);
+
+  mMetaHdr.mLastFetched = NOW_SECONDS();
+  ++mMetaHdr.mFetchCount;
+  return NS_OK;
+}
+
+void
+CacheFileMetadata::MarkDirty(bool aUpdateLastModified)
+{
+  mIsDirty = true;
+  if (aUpdateLastModified) {
+    mMetaHdr.mLastModified = NOW_SECONDS();
+  }
+}
+
+nsresult
 CacheFileMetadata::OnFileOpened(CacheFileHandle *aHandle, nsresult aResult)
 {
   MOZ_CRASH("CacheFileMetadata::OnFileOpened should not be called!");
   return NS_ERROR_UNEXPECTED;
 }
 
 nsresult
 CacheFileMetadata::OnDataWritten(CacheFileHandle *aHandle, const char *aBuf,
@@ -716,17 +725,17 @@ CacheFileMetadata::InitEmptyMetadata()
 {
   if (mBuf) {
     free(mBuf);
     mBuf = nullptr;
     mBufSize = 0;
   }
   mOffset = 0;
   mMetaHdr.mVersion = kCacheEntryVersion;
-  mMetaHdr.mFetchCount = 1;
+  mMetaHdr.mFetchCount = 0;
   mMetaHdr.mExpirationTime = nsICacheEntry::NO_EXPIRATION_TIME;
   mMetaHdr.mKeySize = mKey.Length();
 
   DoMemoryReport(MemoryUsage());
 }
 
 nsresult
 CacheFileMetadata::ParseMetadata(uint32_t aMetaOffset, uint32_t aBufOffset,
@@ -827,17 +836,16 @@ CacheFileMetadata::ParseMetadata(uint32_
   mHashCount = hashCount;
   if (mHashArraySize) {
     mHashArray = static_cast<CacheHash::Hash16_t *>(
                    moz_xmalloc(mHashArraySize));
     memcpy(mHashArray, mBuf + hashesOffset, mHashArraySize);
   }
 
 
-  mMetaHdr.mFetchCount++;
   MarkDirty();
 
   mElementsSize = metaposOffset - elementsOffset;
   memmove(mBuf, mBuf + elementsOffset, mElementsSize);
   mOffset = aMetaOffset;
 
   // TODO: shrink memory if buffer is too big
 
--- a/netwerk/cache2/CacheFileMetadata.h
+++ b/netwerk/cache2/CacheFileMetadata.h
@@ -133,26 +133,28 @@ public:
   nsresult     SetElement(const char *aKey, const char *aValue);
   nsresult     Visit(nsICacheEntryMetaDataVisitor *aVisitor);
 
   CacheHash::Hash16_t GetHash(uint32_t aIndex);
   nsresult            SetHash(uint32_t aIndex, CacheHash::Hash16_t aHash);
 
   nsresult SetExpirationTime(uint32_t aExpirationTime);
   nsresult GetExpirationTime(uint32_t *_retval);
-  nsresult SetLastModified(uint32_t aLastModified);
-  nsresult GetLastModified(uint32_t *_retval);
   nsresult SetFrecency(uint32_t aFrecency);
   nsresult GetFrecency(uint32_t *_retval);
+  nsresult GetLastModified(uint32_t *_retval);
   nsresult GetLastFetched(uint32_t *_retval);
   nsresult GetFetchCount(uint32_t *_retval);
+  // Called by upper layers to indicate the entry this metadata belongs
+  // with has been fetched, i.e. delivered to the consumer.
+  nsresult OnFetched();
 
   int64_t  Offset() { return mOffset; }
   uint32_t ElementsSize() { return mElementsSize; }
-  void     MarkDirty() { mIsDirty = true; }
+  void     MarkDirty(bool aUpdateLastModified = true);
   bool     IsDirty() { return mIsDirty; }
   uint32_t MemoryUsage() { return sizeof(CacheFileMetadata) + mHashArraySize + mBufSize; }
 
   NS_IMETHOD OnFileOpened(CacheFileHandle *aHandle, nsresult aResult);
   NS_IMETHOD OnDataWritten(CacheFileHandle *aHandle, const char *aBuf,
                            nsresult aResult);
   NS_IMETHOD OnDataRead(CacheFileHandle *aHandle, char *aBuf, nsresult aResult);
   NS_IMETHOD OnFileDoomed(CacheFileHandle *aHandle, nsresult aResult);
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_cache2-28-last-access-attrs.js
@@ -0,0 +1,39 @@
+function run_test()
+{
+  do_get_profile();
+  function NowSeconds() {
+    return parseInt((new Date()).getTime() / 1000);
+  }
+  function do_check_time(t, min, max) {
+    do_check_true(t >= min);
+    do_check_true(t <= max);
+  }
+
+  var timeStart = NowSeconds();
+
+  asyncOpenCacheEntry("http://t/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+    new OpenCallback(NEW, "m", "d", function(entry) {
+
+      var firstOpen = NowSeconds();
+      do_check_eq(entry.fetchCount, 1);
+      do_check_time(entry.lastFetched, timeStart, firstOpen);
+      do_check_time(entry.lastModified, timeStart, firstOpen);
+
+      do_timeout(2000, () => {
+        asyncOpenCacheEntry("http://t/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+          new OpenCallback(NORMAL, "m", "d", function(entry) {
+
+            var secondOpen = NowSeconds();
+            do_check_eq(entry.fetchCount, 2);
+            do_check_time(entry.lastFetched, firstOpen, secondOpen);
+            do_check_time(entry.lastModified, timeStart, firstOpen);
+
+            finish_cache2_test();
+          })
+        );
+      })
+    })
+  );
+
+  do_test_pending();
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -54,16 +54,19 @@ support-files =
 [test_cache2-22-anon-visit.js]
 [test_cache2-23-read-over-chunk.js]
 [test_cache2-24-exists.js]
 [test_cache2-25-chunk-memory-limit.js]
 [test_cache2-26-no-outputstream-open.js]
 # GC, that this patch is dependent on, doesn't work well on Android.
 skip-if = os == "android"
 [test_cache2-27-force-valid-for.js]
+[test_cache2-28-last-access-attrs.js]
+# This test will be fixed in bug 1067931
+skip-if = true
 [test_304_responses.js]
 [test_cacheForOfflineUse_no-store.js]
 [test_307_redirect.js]
 [test_NetUtil.js]
 [test_URIs.js]
 [test_aboutblank.js]
 [test_assoc.js]
 [test_auth_jar.js]