From f834c0c5b46694b6c0ecc71a606467347d3d0020 Mon Sep 17 00:00:00 2001 From: Honza Bambas Date: Thu, 11 Sep 2014 11:44:31 +0200 Subject: [PATCH] Bug 1000338 - nsICacheEntry.lastModified not properly implemented, r=michal --- netwerk/cache2/CacheEntry.cpp | 5 ++ netwerk/cache2/CacheFile.cpp | 43 +++++++-------- netwerk/cache2/CacheFile.h | 6 ++- netwerk/cache2/CacheFileMetadata.cpp | 54 +++++++++++-------- netwerk/cache2/CacheFileMetadata.h | 8 +-- .../unit/test_cache2-28-last-access-attrs.js | 39 ++++++++++++++ netwerk/test/unit/xpcshell.ini | 1 + 7 files changed, 107 insertions(+), 49 deletions(-) create mode 100644 netwerk/test/unit/test_cache2-28-last-access-attrs.js diff --git a/netwerk/cache2/CacheEntry.cpp b/netwerk/cache2/CacheEntry.cpp index f63a0298316..9aacd15b1fa 100644 --- a/netwerk/cache2/CacheEntry.cpp +++ b/netwerk/cache2/CacheEntry.cpp @@ -759,6 +759,11 @@ void CacheEntry::InvokeAvailableCallback(Callback const & aCallback) 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( diff --git a/netwerk/cache2/CacheFile.cpp b/netwerk/cache2/CacheFile.cpp index 4b99363e43e..436ea270846 100644 --- a/netwerk/cache2/CacheFile.cpp +++ b/netwerk/cache2/CacheFile.cpp @@ -910,27 +910,6 @@ CacheFile::GetExpirationTime(uint32_t *_retval) 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) { @@ -956,6 +935,16 @@ CacheFile::GetFrecency(uint32_t *_retval) 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) { @@ -976,6 +965,18 @@ CacheFile::GetFetchCount(uint32_t *_retval) 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() { diff --git a/netwerk/cache2/CacheFile.h b/netwerk/cache2/CacheFile.h index 5c4e0f4de11..982f6e6edef 100644 --- a/netwerk/cache2/CacheFile.h +++ b/netwerk/cache2/CacheFile.h @@ -90,12 +90,14 @@ public: 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; } diff --git a/netwerk/cache2/CacheFileMetadata.cpp b/netwerk/cache2/CacheFileMetadata.cpp index f290ea03b9b..913657263c3 100644 --- a/netwerk/cache2/CacheFileMetadata.cpp +++ b/netwerk/cache2/CacheFileMetadata.cpp @@ -27,6 +27,8 @@ namespace net { #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) @@ -82,7 +84,6 @@ CacheFileMetadata::CacheFileMetadata(bool aMemoryOnly, const nsACString &aKey) memset(&mMetaHdr, 0, sizeof(CacheFileMetadataHeader)); mMetaHdr.mVersion = kCacheEntryVersion; mMetaHdr.mExpirationTime = nsICacheEntry::NO_EXPIRATION_TIME; - mMetaHdr.mFetchCount = 1; mKey = aKey; mMetaHdr.mKeySize = mKey.Length(); @@ -504,7 +505,7 @@ CacheFileMetadata::SetExpirationTime(uint32_t aExpirationTime) LOG(("CacheFileMetadata::SetExpirationTime() [this=%p, expirationTime=%d]", this, aExpirationTime)); - MarkDirty(); + MarkDirty(false); mMetaHdr.mExpirationTime = aExpirationTime; return NS_OK; } @@ -516,31 +517,13 @@ CacheFileMetadata::GetExpirationTime(uint32_t *_retval) 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; } @@ -552,6 +535,13 @@ CacheFileMetadata::GetFrecency(uint32_t *_retval) return NS_OK; } +nsresult +CacheFileMetadata::GetLastModified(uint32_t *_retval) +{ + *_retval = mMetaHdr.mLastModified; + return NS_OK; +} + nsresult CacheFileMetadata::GetLastFetched(uint32_t *_retval) { @@ -566,6 +556,25 @@ CacheFileMetadata::GetFetchCount(uint32_t *_retval) 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) { @@ -721,7 +730,7 @@ CacheFileMetadata::InitEmptyMetadata() } mOffset = 0; mMetaHdr.mVersion = kCacheEntryVersion; - mMetaHdr.mFetchCount = 1; + mMetaHdr.mFetchCount = 0; mMetaHdr.mExpirationTime = nsICacheEntry::NO_EXPIRATION_TIME; mMetaHdr.mKeySize = mKey.Length(); @@ -832,7 +841,6 @@ CacheFileMetadata::ParseMetadata(uint32_t aMetaOffset, uint32_t aBufOffset, } - mMetaHdr.mFetchCount++; MarkDirty(); mElementsSize = metaposOffset - elementsOffset; diff --git a/netwerk/cache2/CacheFileMetadata.h b/netwerk/cache2/CacheFileMetadata.h index 0b1fae9e75f..70f1f632ca1 100644 --- a/netwerk/cache2/CacheFileMetadata.h +++ b/netwerk/cache2/CacheFileMetadata.h @@ -138,16 +138,18 @@ public: 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; } diff --git a/netwerk/test/unit/test_cache2-28-last-access-attrs.js b/netwerk/test/unit/test_cache2-28-last-access-attrs.js new file mode 100644 index 00000000000..b8d93dc44b9 --- /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(); +} diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 6f01b28f216..00cfec60af4 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -59,6 +59,7 @@ support-files = # 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] [test_304_responses.js] [test_cacheForOfflineUse_no-store.js] [test_307_redirect.js]