diff --git a/netwerk/cache2/CacheEntry.cpp b/netwerk/cache2/CacheEntry.cpp index 55f0371151f5..16de01ae9eb0 100644 --- a/netwerk/cache2/CacheEntry.cpp +++ b/netwerk/cache2/CacheEntry.cpp @@ -545,7 +545,6 @@ already_AddRefed CacheEntry::ReopenTruncated(bool aMemoryOnly, mUseDisk && !aMemoryOnly, mSkipSizeCheck, mPinned, - true, // always create true, // truncate existing (this one) getter_AddRefs(handle)); @@ -1072,13 +1071,12 @@ NS_IMETHODIMP CacheEntry::GetIsForcedValid(bool *aIsForcedValid) } nsAutoCString key; - - nsresult rv = HashingKeyWithStorage(key); + nsresult rv = HashingKey(key); if (NS_FAILED(rv)) { return rv; } - *aIsForcedValid = CacheStorageService::Self()->IsForcedValidEntry(key); + *aIsForcedValid = CacheStorageService::Self()->IsForcedValidEntry(mStorageID, key); LOG(("CacheEntry::GetIsForcedValid [this=%p, IsForcedValid=%d]", this, *aIsForcedValid)); return NS_OK; @@ -1089,12 +1087,12 @@ NS_IMETHODIMP CacheEntry::ForceValidFor(uint32_t aSecondsToTheFuture) LOG(("CacheEntry::ForceValidFor [this=%p, aSecondsToTheFuture=%d]", this, aSecondsToTheFuture)); nsAutoCString key; - nsresult rv = HashingKeyWithStorage(key); + nsresult rv = HashingKey(key); if (NS_FAILED(rv)) { return rv; } - CacheStorageService::Self()->ForceEntryValidFor(key, aSecondsToTheFuture); + CacheStorageService::Self()->ForceEntryValidFor(mStorageID, key, aSecondsToTheFuture); return NS_OK; } @@ -1325,6 +1323,8 @@ NS_IMETHODIMP CacheEntry::AsyncDoom(nsICacheEntryDoomCallback *aCallback) if (mIsDoomed || mDoomCallback) return NS_ERROR_IN_PROGRESS; // to aggregate have DOOMING state + RemoveForcedValidity(); + mIsDoomed = true; mDoomCallback = aCallback; } @@ -1623,6 +1623,8 @@ void CacheEntry::DoomAlreadyRemoved() mozilla::MutexAutoLock lock(mLock); + RemoveForcedValidity(); + mIsDoomed = true; // Pretend pinning is know. This entry is now doomed for good, so don't @@ -1666,6 +1668,25 @@ void CacheEntry::DoomFile() OnFileDoomed(rv); } +void CacheEntry::RemoveForcedValidity() +{ + mLock.AssertCurrentThreadOwns(); + + nsresult rv; + + if (mIsDoomed) { + return; + } + + nsAutoCString entryKey; + rv = HashingKey(entryKey); + if (NS_WARN_IF(NS_FAILED(rv))) { + return; + } + + CacheStorageService::Self()->RemoveEntryForceValid(mStorageID, entryKey); +} + void CacheEntry::BackgroundOp(uint32_t aOperations, bool aForceAsync) { mLock.AssertCurrentThreadOwns(); diff --git a/netwerk/cache2/CacheEntry.h b/netwerk/cache2/CacheEntry.h index 32aa5115e071..1aeed08117cc 100644 --- a/netwerk/cache2/CacheEntry.h +++ b/netwerk/cache2/CacheEntry.h @@ -266,6 +266,9 @@ private: // Called only from DoomAlreadyRemoved() void DoomFile(); + // When this entry is doomed the first time, this method removes + // any force-valid timing info for this entry. + void RemoveForcedValidity(); already_AddRefed ReopenTruncated(bool aMemoryOnly, nsICacheEntryOpenCallback* aCallback); diff --git a/netwerk/cache2/CacheStorage.cpp b/netwerk/cache2/CacheStorage.cpp index 5630071cce98..d3fd4ca23694 100644 --- a/netwerk/cache2/CacheStorage.cpp +++ b/netwerk/cache2/CacheStorage.cpp @@ -104,7 +104,6 @@ NS_IMETHODIMP CacheStorage::AsyncOpenURI(nsIURI *aURI, RefPtr entry; rv = CacheStorageService::Self()->AddStorageEntry( this, noRefURI, aIdExtension, - true, // create always truncate, // replace any existing one? getter_AddRefs(entry)); NS_ENSURE_SUCCESS(rv, rv); @@ -131,7 +130,6 @@ NS_IMETHODIMP CacheStorage::OpenTruncate(nsIURI *aURI, const nsACString & aIdExt RefPtr handle; rv = CacheStorageService::Self()->AddStorageEntry( this, noRefURI, aIdExtension, - true, // create always true, // replace any existing one getter_AddRefs(handle)); NS_ENSURE_SUCCESS(rv, rv); diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp index 22cce5cff8e7..34e7ee410374 100644 --- a/netwerk/cache2/CacheStorageService.cpp +++ b/netwerk/cache2/CacheStorageService.cpp @@ -769,6 +769,11 @@ NS_IMETHODIMP CacheStorageService::Clear() { mozilla::MutexAutoLock lock(mLock); + { + mozilla::MutexAutoLock forcedValidEntriesLock(mForcedValidEntriesLock); + mForcedValidEntries.Clear(); + } + NS_ENSURE_TRUE(!mShutdown, NS_ERROR_NOT_INITIALIZED); nsTArray keys; @@ -1021,7 +1026,7 @@ CacheStorageService::RemoveEntry(CacheEntry* aEntry, bool aOnlyUnreferenced) return false; } - if (!aEntry->IsUsingDisk() && IsForcedValidEntry(entryKey)) { + if (!aEntry->IsUsingDisk() && IsForcedValidEntry(aEntry->GetStorageID(), entryKey)) { LOG((" forced valid, not removing")); return false; } @@ -1094,13 +1099,19 @@ CacheStorageService::RecordMemoryOnlyEntry(CacheEntry* aEntry, // Checks if a cache entry is forced valid (will be loaded directly from cache // without further validation) - see nsICacheEntry.idl for further details -bool CacheStorageService::IsForcedValidEntry(nsACString &aCacheEntryKey) +bool CacheStorageService::IsForcedValidEntry(nsACString const &aContextKey, + nsACString const &aEntryKey) +{ + return IsForcedValidEntry(aContextKey + aEntryKey); +} + +bool CacheStorageService::IsForcedValidEntry(nsACString const &aContextEntryKey) { mozilla::MutexAutoLock lock(mForcedValidEntriesLock); TimeStamp validUntil; - if (!mForcedValidEntries.Get(aCacheEntryKey, &validUntil)) { + if (!mForcedValidEntries.Get(aContextEntryKey, &validUntil)) { return false; } @@ -1114,13 +1125,14 @@ bool CacheStorageService::IsForcedValidEntry(nsACString &aCacheEntryKey) } // Entry timeout has been reached - mForcedValidEntries.Remove(aCacheEntryKey); + mForcedValidEntries.Remove(aContextEntryKey); return false; } // Allows a cache entry to be loaded directly from cache without further // validation - see nsICacheEntry.idl for further details -void CacheStorageService::ForceEntryValidFor(nsACString &aCacheEntryKey, +void CacheStorageService::ForceEntryValidFor(nsACString const &aContextKey, + nsACString const &aEntryKey, uint32_t aSecondsToTheFuture) { mozilla::MutexAutoLock lock(mForcedValidEntriesLock); @@ -1131,7 +1143,15 @@ void CacheStorageService::ForceEntryValidFor(nsACString &aCacheEntryKey, // This will be the timeout TimeStamp validUntil = now + TimeDuration::FromSeconds(aSecondsToTheFuture); - mForcedValidEntries.Put(aCacheEntryKey, validUntil); + mForcedValidEntries.Put(aContextKey + aEntryKey, validUntil); +} + +void CacheStorageService::RemoveEntryForceValid(nsACString const &aContextKey, + nsACString const &aEntryKey) +{ + mozilla::MutexAutoLock lock(mForcedValidEntriesLock); + + mForcedValidEntries.Remove(aContextKey + aEntryKey); } // Cleans out the old entries in mForcedValidEntries @@ -1378,7 +1398,6 @@ nsresult CacheStorageService::AddStorageEntry(CacheStorage const* aStorage, nsIURI* aURI, const nsACString & aIdExtension, - bool aCreateIfNotExist, bool aReplace, CacheEntryHandle** aResult) { @@ -1393,7 +1412,7 @@ CacheStorageService::AddStorageEntry(CacheStorage const* aStorage, aStorage->WriteToDisk(), aStorage->SkipSizeCheck(), aStorage->Pinning(), - aCreateIfNotExist, aReplace, + aReplace, aResult); } @@ -1404,7 +1423,6 @@ CacheStorageService::AddStorageEntry(nsCSubstring const& aContextKey, bool aWriteToDisk, bool aSkipSizeCheck, bool aPin, - bool aCreateIfNotExist, bool aReplace, CacheEntryHandle** aResult) { @@ -1440,7 +1458,7 @@ CacheStorageService::AddStorageEntry(nsCSubstring const& aContextKey, if (entryExists && !aReplace) { // check whether we want to turn this entry to a memory-only. if (MOZ_UNLIKELY(!aWriteToDisk) && MOZ_LIKELY(entry->IsUsingDisk())) { - LOG((" entry is persistnet but we want mem-only, replacing it")); + LOG((" entry is persistent but we want mem-only, replacing it")); aReplace = true; } } @@ -1457,10 +1475,20 @@ CacheStorageService::AddStorageEntry(nsCSubstring const& aContextKey, entry = nullptr; entryExists = false; + + // Would only lead to deleting force-valid timestamp again. We don't need the + // replace information anymore after this point anyway. + aReplace = false; } - // Ensure entry for the particular URL, if not read/only - if (!entryExists && (aCreateIfNotExist || aReplace)) { + // Ensure entry for the particular URL + if (!entryExists) { + // When replacing with a new entry, always remove the current force-valid timestamp, + // this is the only place to do it. + if (aReplace) { + RemoveEntryForceValid(aContextKey, entryKey); + } + // Entry is not in the hashtable or has just been truncated... entry = new CacheEntry(aContextKey, aURI, aIdExtension, aWriteToDisk, aSkipSizeCheck, aPin); entries->Put(entryKey, entry); @@ -1639,6 +1667,10 @@ CacheStorageService::DoomStorageEntry(CacheStorage const* aStorage, } } } + + if (!entry) { + RemoveEntryForceValid(contextKey, entryKey); + } } if (entry) { @@ -1764,6 +1796,21 @@ CacheStorageService::DoomStorageEntries(nsCSubstring const& aContextKey, } } + { + mozilla::MutexAutoLock lock(mForcedValidEntriesLock); + + for (auto iter = mForcedValidEntries.Iter(); !iter.Done(); iter.Next()) { + bool matches; + DebugOnly rv = CacheFileUtils::KeyMatchesLoadContextInfo( + iter.Key(), aContext, &matches); + MOZ_ASSERT(NS_SUCCEEDED(rv)); + + if (matches) { + iter.Remove(); + } + } + } + // An artificial callback. This is a candidate for removal tho. In the new // cache any 'doom' or 'evict' function ensures that the entry or entries // being doomed is/are not accessible after the function returns. So there is @@ -1824,24 +1871,28 @@ CacheStorageService::CacheFileDoomed(nsILoadContextInfo* aLoadContextInfo, mozilla::MutexAutoLock lock(mLock); - if (mShutdown) + if (mShutdown) { return; + } CacheEntryTable* entries; - if (!sGlobalEntryTables->Get(contextKey, &entries)) - return; - RefPtr entry; - if (!entries->Get(entryKey, getter_AddRefs(entry))) - return; - if (!entry->IsFileDoomed()) - return; + if (sGlobalEntryTables->Get(contextKey, &entries) && + entries->Get(entryKey, getter_AddRefs(entry))) { + if (entry->IsFileDoomed()) { + // Need to remove under the lock to avoid possible race leading + // to duplication of the entry per its key. + RemoveExactEntry(entries, entryKey, entry, false); + entry->DoomAlreadyRemoved(); + } - // Need to remove under the lock to avoid possible race leading - // to duplication of the entry per its key. - RemoveExactEntry(entries, entryKey, entry, false); - entry->DoomAlreadyRemoved(); + // Entry found, but it's not the entry that has been found doomed + // by the lower eviction layer. Just leave everything unchanged. + return; + } + + RemoveEntryForceValid(contextKey, entryKey); } bool diff --git a/netwerk/cache2/CacheStorageService.h b/netwerk/cache2/CacheStorageService.h index 7da8297067e9..921d081b6a94 100644 --- a/netwerk/cache2/CacheStorageService.h +++ b/netwerk/cache2/CacheStorageService.h @@ -158,23 +158,35 @@ private: * directly from cache) for the given number of seconds * See nsICacheEntry.idl for more details */ - void ForceEntryValidFor(nsACString &aCacheEntryKey, + void ForceEntryValidFor(nsACString const &aContextKey, + nsACString const &aEntryKey, uint32_t aSecondsToTheFuture); + /** + * Remove the validity info + */ + void RemoveEntryForceValid(nsACString const &aContextKey, + nsACString const &aEntryKey); + + /** + * Retrieves the status of the cache entry to see if it has been forced valid + * (so it will loaded directly from cache without further validation) + */ + bool IsForcedValidEntry(nsACString const &aContextKey, + nsACString const &aEntryKey); + private: friend class CacheIndex; /** - * Retrieves the status of the cache entry to see if it has been forced valid - * (so it will loaded directly from cache without further validation) * CacheIndex uses this to prevent a cache entry from being prememptively * thrown away when forced valid * See nsICacheEntry.idl for more details */ - bool IsForcedValidEntry(nsACString &aCacheEntryKey); + bool IsForcedValidEntry(nsACString const &aEntryKeyWithContext); private: - // These are helpers for telemetry monitorying of the memory pools. + // These are helpers for telemetry monitoring of the memory pools. void TelemetryPrune(TimeStamp &now); void TelemetryRecordEntryCreation(CacheEntry const* entry); void TelemetryRecordEntryRemoval(CacheEntry const* entry); @@ -190,7 +202,6 @@ private: nsresult AddStorageEntry(CacheStorage const* aStorage, nsIURI* aURI, const nsACString & aIdExtension, - bool aCreateIfNotExist, bool aReplace, CacheEntryHandle** aResult); @@ -286,7 +297,6 @@ private: bool aWriteToDisk, bool aSkipSizeCheck, bool aPin, - bool aCreateIfNotExist, bool aReplace, CacheEntryHandle** aResult);