commit e46c06071ea5a0e5570c71446f9b01973576dda2
parent 05874864bfe4325ec857ade6b539f152c2fa5bd1
Author: agoloman <agoloman@mozilla.com>
Date: Sat, 25 Oct 2025 08:58:54 +0300
Revert "Bug 1996305: Ensure Suspend() is called before starting async dictionary origin reads r=necko-reviewers,valentin" for causing wpt failures @nsCOMPtr.h.
This reverts commit 9b1999e2513718c32e08a977e982f47892775e1b.
Revert "Bug 1996298: PurgeByFrecency was over-purging due to holding references r=jstutte"
This reverts commit 37189355ecc35635266b246e9ba67e6a2544a9fc.
Diffstat:
6 files changed, 35 insertions(+), 69 deletions(-)
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -746,12 +746,7 @@ static bool RemoveExactEntry(CacheEntryTable* aEntries, nsACString const& aKey,
// Remove from DictionaryCache immediately, to ensure the removal is
// synchronous
-
- if (aEntry->GetEnhanceID().EqualsLiteral("dict:")) {
- DictionaryCache::RemoveOriginFor(aEntry->GetURI());
- } else {
- DictionaryCache::RemoveDictionaryFor(aEntry->GetURI());
- }
+ DictionaryCache::RemoveDictionaryFor(aEntry->GetURI());
LOG(("RemoveExactEntry [entry=%p removed]", aEntry));
aEntries->Remove(aKey);
@@ -1515,8 +1510,8 @@ Result<size_t, nsresult> CacheStorageService::MemoryPool::PurgeByFrecency(
} else {
if (entry->GetEnhanceID().EqualsLiteral("dict:")) {
LOG(
- ("*** Ignored Entry is a dictionary origin, metadata size %d, "
- "referenced %d, Frecency %f",
+ ("*** Entry is a dictionary origin, metadata size %d, referenced "
+ "%d, Frecency %f",
entry->GetMetadataMemoryConsumption(), entry->IsReferenced(),
entry->GetFrecency()));
}
@@ -1535,9 +1530,7 @@ Result<size_t, nsresult> CacheStorageService::MemoryPool::PurgeByFrecency(
break;
}
- // Ensure it's deleted immediately if purged so we can record the
- // mMemorySize savings
- RefPtr<CacheEntry> entry = std::move(checkPurge.mEntry);
+ RefPtr<CacheEntry> entry = checkPurge.mEntry;
if (entry->Purge(CacheEntry::PURGE_WHOLE)) {
numPurged++;
@@ -1551,10 +1544,7 @@ Result<size_t, nsresult> CacheStorageService::MemoryPool::PurgeByFrecency(
}
}
- LOG(
- ("MemoryPool::PurgeByFrecency done, purged %zu - mMemorySize %u, "
- "memoryLimit %u",
- numPurged, (uint32_t)mMemorySize, memoryLimit));
+ LOG(("MemoryPool::PurgeByFrecency done"));
return numPurged;
}
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -644,7 +644,7 @@ DictionaryCacheEntry::OnCacheEntryAvailable(nsICacheEntry* entry, bool isNew,
// Read the metadata for an Origin and parse it, creating DictionaryCacheEntrys
// as needed. If aType is TYPE_OTHER, there is no Match() to do
void DictionaryOriginReader::Start(
- bool aCreate, DictionaryOrigin* aOrigin, nsACString& aKey, nsIURI* aURI,
+ DictionaryOrigin* aOrigin, nsACString& aKey, nsIURI* aURI,
ExtContentPolicyType aType, DictionaryCache* aCache,
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback) {
mOrigin = aOrigin;
@@ -668,7 +668,7 @@ void DictionaryOriginReader::Start(
PromiseFlatCString(aKey).get(), this));
DictionaryCache::sCacheStorage->AsyncOpenURIString(
aKey, META_DICTIONARY_PREFIX,
- aCreate
+ aOrigin
? nsICacheStorage::OPEN_NORMALLY |
nsICacheStorage::CHECK_MULTITHREADED
: nsICacheStorage::OPEN_READONLY | nsICacheStorage::OPEN_SECRETLY |
@@ -858,13 +858,12 @@ already_AddRefed<DictionaryCacheEntry> DictionaryCache::AddEntry(
// This creates a cycle until the dictionary is removed from the cache
aDictEntry->SetOrigin(origin);
- DICTIONARY_LOG(("Creating cache entry for origin %s", prepath.get()));
// Open (and parse metadata) or create
RefPtr<DictionaryOriginReader> reader = new DictionaryOriginReader();
// the type is irrelevant; we won't be calling Match()
reader->Start(
- true, origin, prepath, aURI, ExtContentPolicy::TYPE_OTHER, this,
+ origin, prepath, aURI, ExtContentPolicy::TYPE_OTHER, this,
[entry = RefPtr(aDictEntry)](
bool, DictionaryCacheEntry* aDict) { // XXX avoid so many lambdas
// which cause allocations
@@ -932,34 +931,6 @@ void DictionaryCache::RemoveDictionary(const nsACString& aKey) {
}
}
-// static
-void DictionaryCache::RemoveOriginFor(const nsACString& aKey) {
- RefPtr<DictionaryCache> cache = GetInstance();
- DICTIONARY_LOG(
- ("Removing dictionary for %80s", PromiseFlatCString(aKey).get()));
- NS_DispatchToMainThread(NewRunnableMethod<const nsCString>(
- "DictionaryCache::RemoveDictionaryFor", cache,
- &DictionaryCache::RemoveDictionary, aKey));
-}
-
-// Remove a dictionary if it exists for the key given
-void DictionaryCache::RemoveOrigin(const nsACString& aKey) {
- DICTIONARY_LOG(("Removing origin for %80s", PromiseFlatCString(aKey).get()));
-
- nsCOMPtr<nsIURI> uri;
- if (NS_FAILED(NS_NewURI(getter_AddRefs(uri), aKey))) {
- return;
- }
- nsAutoCString prepath;
- if (NS_SUCCEEDED(GetDictPath(uri, prepath))) {
- if (auto origin = mDictionaryCache.Lookup(prepath)) {
- origin.Data()->AssertEmpty();
- }
-
- mDictionaryCache.Remove(prepath);
- }
-}
-
// Remove a dictionary if it exists for the key given. Mainthread only.
// Note: due to cookie samesite rules, we need to clean for all ports
// static
@@ -1031,9 +1002,10 @@ void DictionaryCache::RemoveAllDictionaries() {
// Once we have a DictionaryOrigin (in-memory or parsed), scan it for matches.
// If it's not in the cache, return nullptr via callback.
void DictionaryCache::GetDictionaryFor(
- nsIURI* aURI, ExtContentPolicyType aType, nsHttpChannel* aChan,
- void (*aSuspend)(nsHttpChannel*),
+ nsIURI* aURI, ExtContentPolicyType aType, bool& aAsync,
+ nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback) {
+ aAsync = false;
// Note: IETF 2.2.3 Multiple Matching Directories
// We need to return match-dest matches first
// If no match-dest, then the longest match
@@ -1063,10 +1035,9 @@ void DictionaryCache::GetDictionaryFor(
RefPtr<DictionaryOriginReader> reader = new DictionaryOriginReader();
// Must do this before calling start, which can run the callbacks and call
// Resume
- DICTIONARY_LOG(("Suspending to get Dictionary headers"));
+ aAsync = true;
aSuspend(aChan);
- reader->Start(false, existing.Data(), prepath, aURI, aType, this,
- aCallback);
+ reader->Start(existing.Data(), prepath, aURI, aType, this, aCallback);
}
return;
}
@@ -1102,9 +1073,8 @@ void DictionaryCache::GetDictionaryFor(
RefPtr<DictionaryOriginReader> reader = new DictionaryOriginReader();
// After Start(), if we drop this ref reader will kill itself on
// completion; it holds a self-ref
- DICTIONARY_LOG(("Suspending to get Dictionary headers"));
- aSuspend(aChan);
- reader->Start(false, origin, prepath, aURI, aType, this, aCallback);
+ reader->Start(origin, prepath, aURI, aType, this, aCallback);
+ aAsync = true;
return;
}
// No dictionaries for origin
diff --git a/netwerk/cache2/Dictionary.h b/netwerk/cache2/Dictionary.h
@@ -237,7 +237,7 @@ class DictionaryOriginReader final : public nsICacheEntryOpenCallback,
DictionaryOriginReader() {}
void Start(
- bool aCreate, DictionaryOrigin* aOrigin, nsACString& aKey, nsIURI* aURI,
+ DictionaryOrigin* aOrigin, nsACString& aKey, nsIURI* aURI,
ExtContentPolicyType aType, DictionaryCache* aCache,
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback);
void FinishMatch();
@@ -279,9 +279,6 @@ class DictionaryOrigin : public nsICacheEntryMetaDataVisitor {
void FinishAddEntry(DictionaryCacheEntry* aEntry);
void DumpEntries();
void Clear();
- void AssertEmpty() const {
- MOZ_DIAGNOSTIC_ASSERT(mEntries.IsEmpty() && mPendingEntries.IsEmpty());
- }
private:
virtual ~DictionaryOrigin() {}
@@ -333,13 +330,9 @@ class DictionaryCache final {
nsIURI* aURI, bool aNewEntry, DictionaryCacheEntry* aDictEntry);
static void RemoveDictionaryFor(const nsACString& aKey);
- // remove the entire origin (should be empty!)
- static void RemoveOriginFor(const nsACString& aKey);
// Remove a dictionary if it exists for the key given
void RemoveDictionary(const nsACString& aKey);
- // Remove an origin for the key given
- void RemoveOrigin(const nsACString& aKey);
nsresult RemoveEntry(nsIURI* aURI, const nsACString& aKey);
@@ -351,8 +344,8 @@ class DictionaryCache final {
// return an entry
void GetDictionaryFor(
- nsIURI* aURI, ExtContentPolicyType aType, nsHttpChannel* aChan,
- void (*aSuspend)(nsHttpChannel*),
+ nsIURI* aURI, ExtContentPolicyType aType, bool& aAsync,
+ nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback);
size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -633,14 +633,16 @@ nsresult nsHttpChannel::PrepareToConnect() {
// This may be async; the dictionary headers may need to fetch an origin
// dictionary cache entry from disk before adding the headers. We can
// continue with channel creation, and just block on this being done later
+ bool async = false;
AUTO_PROFILER_FLOW_MARKER("nsHttpHandler::AddAcceptAndDictionaryHeaders",
NETWORK, Flow::FromPointer(this));
// AddAcceptAndDictionaryHeaders must call this->Suspend before kicking
// off the async operation that can result in calling the lambda (which
// will Resume), to avoid a race condition.
+ bool aAsync;
nsresult rv = gHttpHandler->AddAcceptAndDictionaryHeaders(
mURI, mLoadInfo->GetExternalContentPolicyType(), &mRequestHead, IsHTTPS(),
- this, nsHttpChannel::StaticSuspend,
+ aAsync, this, nsHttpChannel::StaticSuspend,
[self = RefPtr(this)](bool aNeedsResume, DictionaryCacheEntry* aDict) {
self->mDictDecompress = aDict;
if (aNeedsResume) {
@@ -687,6 +689,11 @@ nsresult nsHttpChannel::PrepareToConnect() {
return true;
});
if (NS_FAILED(rv)) return rv;
+ if (async) {
+ // we'll continue later if GetDictionaryFor is still reading
+ LOG_DICTIONARIES(("Suspending to get Dictionary headers"));
+ Suspend();
+ }
// notify "http-on-modify-request-before-cookies" observers
gHttpHandler->OnModifyRequestBeforeCookies(this);
diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -658,7 +658,8 @@ nsresult nsHttpHandler::InitConnectionMgr() {
// dictionary load.
nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
nsIURI* aURI, ExtContentPolicyType aType, nsHttpRequestHead* aRequest,
- bool aSecure, nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
+ bool aSecure, bool& aAsync, nsHttpChannel* aChan,
+ void (*aSuspend)(nsHttpChannel*),
const std::function<bool(bool, DictionaryCacheEntry*)>& aCallback) {
LOG(("Adding Dictionary headers"));
auto guard = MakeScopeExit([&]() { (aCallback)(false, nullptr); });
@@ -672,7 +673,7 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
// aCallback will now be owned by GetDictionaryFor
guard.release();
mDictionaryCache->GetDictionaryFor(
- aURI, aType, aChan, aSuspend,
+ aURI, aType, aAsync, aChan, aSuspend,
[self = RefPtr(this), aRequest, aCallback](
bool aNeedsResume, DictionaryCacheEntry* aDict) {
if (!aDict) {
@@ -724,7 +725,11 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
}
return NS_OK;
});
+ } else {
+ aAsync = false;
}
+ } else {
+ aAsync = false;
}
// guard may call aCallback here
return rv;
diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h
@@ -119,7 +119,8 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
[[nodiscard]] nsresult AddAcceptAndDictionaryHeaders(
nsIURI* aURI, ExtContentPolicyType aType, nsHttpRequestHead* aRequest,
- bool aSecure, nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
+ bool aSecure, bool& aAsync, nsHttpChannel* aChan,
+ void (*aSuspend)(nsHttpChannel*),
const std::function<bool(bool, DictionaryCacheEntry*)>& aCallback);
[[nodiscard]] nsresult AddStandardRequestHeaders(
nsHttpRequestHead*, nsIURI* aURI, bool aIsHTTPS,
