commit c9e18bdf3b9c591c2cac70f7880802b68a61081e
parent 17923efc19a4f53c0eed7532f8b50617d16782fa
Author: Randell Jesup <rjesup@mozilla.com>
Date: Wed, 1 Oct 2025 18:45:59 +0000
Bug 1917974: Hook in Prefetching of cache entries for Compression Dictionaries r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D258165
Diffstat:
9 files changed, 108 insertions(+), 77 deletions(-)
diff --git a/netwerk/cache2/CacheFileIOManager.cpp b/netwerk/cache2/CacheFileIOManager.cpp
@@ -1872,7 +1872,7 @@ nsresult CacheFileIOManager::OpenFileInternal(const SHA1Sum::Hash* aHash,
if (exists) {
// If this file has been found evicted through the context file evictor
// above for any of pinned or non-pinned state, these calls ensure we doom
- // the handle ASAP we know the real pinning state after metadta has been
+ // the handle ASAP we know the real pinning state after metadata has been
// parsed. DoomFileInternal on the |handle| doesn't doom right now, since
// the pinning state is unknown and we pass down a pinning restriction.
if (evictedAsPinned) {
@@ -2512,6 +2512,11 @@ nsresult CacheFileIOManager::DoomFileInternal(
}
if (!aHandle->IsSpecialFile()) {
+ // Remove from Dictionary cache if this is a dictionary
+ if (!mDictionaryCache) {
+ // mDictionaryCache = DictionaryCache::GetInstance();
+ }
+ // mDictionaryCache->RemoveDictionaryFor(aHandle->mKey);
CacheIndex::RemoveEntry(aHandle->Hash());
}
@@ -2525,7 +2530,7 @@ nsresult CacheFileIOManager::DoomFileInternal(
CacheFileUtils::ParseKey(aHandle->Key(), &idExtension, &url);
MOZ_ASSERT(info);
if (info) {
- storageService->CacheFileDoomed(info, idExtension, url);
+ storageService->CacheFileDoomed(aHandle->mKey, info, idExtension, url);
}
}
}
@@ -3515,7 +3520,8 @@ nsresult CacheFileIOManager::EvictByContextInternal(
}
// Filter by origin.
- if (!origin.IsEmpty()) {
+ if (!origin.IsEmpty()) { // XXX also look for dict:<origin>, or let that
+ // be handled by Doom? Probably Doom
RefPtr<MozURL> url;
rv = MozURL::Init(getter_AddRefs(url), uriSpec);
if (NS_FAILED(rv)) {
diff --git a/netwerk/cache2/CacheFileIOManager.h b/netwerk/cache2/CacheFileIOManager.h
@@ -20,6 +20,7 @@
#include "nsString.h"
#include "nsTHashtable.h"
#include "prio.h"
+#include "Dictionary.h"
// #define DEBUG_HANDLES 1
#if !defined(MOZ_WIDGET_ANDROID)
@@ -481,6 +482,9 @@ class CacheFileIOManager final : public nsITimerCallback, public nsINamed {
static StaticRefPtr<CacheFileIOManager> gInstance;
+ // Pointer to DictionaryCache singleton
+ RefPtr<DictionaryCache> mDictionaryCache;
+
TimeStamp mStartTime;
// Set true on the IO thread, CLOSE level as part of the internal shutdown
// procedure.
diff --git a/netwerk/cache2/CacheStorageService.cpp b/netwerk/cache2/CacheStorageService.cpp
@@ -2057,7 +2057,8 @@ nsresult CacheStorageService::WalkStorageEntries(
return event->Walk();
}
-void CacheStorageService::CacheFileDoomed(nsILoadContextInfo* aLoadContextInfo,
+void CacheStorageService::CacheFileDoomed(const nsACString& aKey,
+ nsILoadContextInfo* aLoadContextInfo,
const nsACString& aIdExtension,
const nsACString& aURISpec) {
nsAutoCString contextKey;
diff --git a/netwerk/cache2/CacheStorageService.h b/netwerk/cache2/CacheStorageService.h
@@ -285,9 +285,11 @@ class CacheStorageService final : public nsICacheStorageService,
/**
* CacheFileIOManager uses this method to notify CacheStorageService that
* an active entry was removed. This method is called even if the entry
- * removal was originated by CacheStorageService.
+ * removal was originated by CacheStorageService. This also removes the entry
+ * from the DictionaryCache.
*/
- void CacheFileDoomed(nsILoadContextInfo* aLoadContextInfo,
+ void CacheFileDoomed(const nsACString& aKey,
+ nsILoadContextInfo* aLoadContextInfo,
const nsACString& aIdExtension,
const nsACString& aURISpec);
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -59,6 +59,8 @@ using namespace mozilla;
namespace mozilla {
namespace net {
+// Access to all these classes is from MainThread unless otherwise specified
+
LazyLogModule gDictionaryLog("CompressionDictionaries");
#define DICTIONARY_LOG(args) \
@@ -146,8 +148,9 @@ void DictionaryCacheEntry::UseCompleted() {
}
// returns aShouldSuspend=true if we should suspend to wait for the prefetch
-bool DictionaryCacheEntry::Prefetch(nsILoadContextInfo* aLoadContextInfo,
- const std::function<void()>& aFunc) {
+nsresult DictionaryCacheEntry::Prefetch(nsILoadContextInfo* aLoadContextInfo,
+ bool& aShouldSuspend,
+ const std::function<void()>& aFunc) {
DICTIONARY_LOG(("Prefetch for %s", mURI.get()));
// Start reading the cache entry into memory and call completion
// function when done
@@ -159,28 +162,36 @@ bool DictionaryCacheEntry::Prefetch(nsILoadContextInfo* aLoadContextInfo,
nsCOMPtr<nsICacheStorageService> cacheStorageService(
components::CacheStorage::Service());
if (!cacheStorageService) {
- return false;
+ aShouldSuspend = false;
+ return NS_ERROR_FAILURE;
}
nsCOMPtr<nsICacheStorage> cacheStorage;
nsresult rv = cacheStorageService->DiskCacheStorage(
aLoadContextInfo, getter_AddRefs(cacheStorage));
if (NS_FAILED(rv)) {
- return false;
+ aShouldSuspend = false;
+ return NS_ERROR_FAILURE;
+ }
+ if (NS_FAILED(cacheStorage->AsyncOpenURIString(
+ mURI, ""_ns, nsICacheStorage::OPEN_READONLY, this))) {
+ // For some reason the cache no longer has this entry;
+ aShouldSuspend = false;
+ return NS_ERROR_FAILURE;
}
- // Add before we schedule!
mWaitingPrefetch.AppendElement(aFunc);
- cacheStorage->AsyncOpenURIString(mURI, ""_ns,
- nsICacheStorage::OPEN_READONLY, this);
DICTIONARY_LOG(("Started Prefetch for %s, anonymous=%d", mURI.get(),
aLoadContextInfo->IsAnonymous()));
- return true;
+ aShouldSuspend = true;
+ return NS_OK;
}
DICTIONARY_LOG(("Prefetch for %s - already have data in memory (%u users)",
mURI.get(), mUsers));
- return false;
+ aShouldSuspend = false;
+ return NS_OK;
}
DICTIONARY_LOG(("Prefetch for %s - already waiting", mURI.get()));
- return true;
+ aShouldSuspend = true;
+ return NS_OK;
}
void DictionaryCacheEntry::AccumulateHash(const char* aBuf, int32_t aCount) {
@@ -620,7 +631,6 @@ NS_IMETHODIMP
DictionaryOriginReader::OnDataAvailable(nsIRequest* request,
nsIInputStream* aInputStream,
uint64_t aOffset, uint32_t aCount) {
- uint32_t n;
DICTIONARY_LOG(
("DictionaryOriginReader %p OnDataAvailable %u", this, aCount));
return NS_OK;
diff --git a/netwerk/cache2/Dictionary.h b/netwerk/cache2/Dictionary.h
@@ -16,6 +16,7 @@
#include "nsIObserver.h"
#include "nsIStreamListener.h"
#include "mozilla/RefPtr.h"
+#include "mozilla/Vector.h"
#include "nsString.h"
#include "nsTArray.h"
#include "mozilla/TimeStamp.h"
@@ -65,8 +66,8 @@ class DictionaryCacheEntry final : public nsICacheEntryOpenCallback,
// Start reading the cache entry into memory and call completion
// function when done
- bool Prefetch(nsILoadContextInfo* aLoadContextInfo,
- const std::function<void()>& aFunc);
+ nsresult Prefetch(nsILoadContextInfo* aLoadContextInfo, bool& aShouldSuspend,
+ const std::function<void()>& aFunc);
const nsACString& GetHash() const { return mHash; }
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -547,38 +547,6 @@ nsresult nsHttpChannel::Init(nsIURI* uri, uint32_t caps, nsProxyInfo* proxyInfo,
proxyURI, channelId, aLoadInfo);
LOG1(("nsHttpChannel::Init [this=%p]\n", this));
- if (NS_FAILED(rv)) return rv;
- // 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
- rv = gHttpHandler->AddAcceptAndDictionaryHeaders(
- uri, &mRequestHead, IsHTTPS(),
- [self = RefPtr(this)](DictionaryCacheEntry* aDict) {
- self->mDictDecompress = aDict;
- if (self->mDictDecompress) {
- LOG_DICTIONARIES(
- ("Added dictionary header for %p, DirectoryCacheEntry %p",
- self.get(), aDict));
- // mDictDecompress is set if we added Available-Dictionary
- self->mDictDecompress->InUse();
- self->mUsingDictionary = true;
- self->mShouldSuspendForDictionary = self->mDictDecompress->Prefetch(
- GetLoadContextInfo(self), [self]() {
- // this is called when the prefetch is complete to
- // un-Suspend the channel
- MOZ_ASSERT(self->mDictDecompress->DictionaryReady());
- if (self->mSuspendedForDictionary) {
- LOG(
- ("nsHttpChannel::Init [this=%p] Resuming channel "
- "suspended for "
- "Dictionary",
- self.get()));
- self->mSuspendedForDictionary = false;
- self->Resume();
- }
- });
- }
- });
return rv;
}
@@ -1988,6 +1956,41 @@ nsresult nsHttpChannel::SetupChannelForTransaction() {
MOZ_ASSERT(NS_SUCCEEDED(rv));
}
}
+ } else {
+ // 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
+ rv = gHttpHandler->AddAcceptAndDictionaryHeaders(
+ mURI, &mRequestHead, IsHTTPS(),
+ [self = RefPtr(this)](DictionaryCacheEntry* aDict) {
+ self->mDictDecompress = aDict;
+ if (self->mDictDecompress) {
+ LOG_DICTIONARIES(
+ ("Added dictionary header for %p, DirectoryCacheEntry %p",
+ self.get(), aDict));
+ // mDictDecompress is set if we added Available-Dictionary
+ self->mDictDecompress->InUse();
+ self->mUsingDictionary = true;
+ return NS_SUCCEEDED(self->mDictDecompress->Prefetch(
+ GetLoadContextInfo(self), self->mShouldSuspendForDictionary,
+ [self]() {
+ // this is called when the prefetch is complete to
+ // un-Suspend the channel
+ MOZ_ASSERT(self->mDictDecompress->DictionaryReady());
+ if (self->mSuspendedForDictionary) {
+ LOG(
+ ("nsHttpChannel::SetupChannelForTransaction [this=%p] "
+ "Resuming channel "
+ "suspended for Dictionary",
+ self.get()));
+ self->mSuspendedForDictionary = false;
+ self->Resume();
+ }
+ }));
+ }
+ return true;
+ });
+ if (NS_FAILED(rv)) return rv;
}
// See bug #466080. Transfer LOAD_ANONYMOUS flag to socket-layer.
diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -656,15 +656,12 @@ nsresult nsHttpHandler::InitConnectionMgr() {
// set by Fetch from the old request
nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
nsIURI* aURI, nsHttpRequestHead* aRequest, bool aSecure,
- const std::function<void(DictionaryCacheEntry*)>& aCallback) {
+ const std::function<bool(DictionaryCacheEntry*)>& aCallback) {
LOG(("Adding Dictionary headers"));
nsresult rv = NS_OK;
// Add the "Accept-Encoding" header and possibly Dictionary headers
if (aSecure) {
// The dictionary info may require us to check the cache.
- // XXX This would require that AddAcceptAndDictionaryHeaders be effectively
- // async, perhaps by passing a lambda to call AddAcceptAndDictionaryHeaders
- // and then unblock the request
if (StaticPrefs::network_http_dictionaries_enable()) {
mDictionaryCache->GetDictionaryFor(
aURI, [self = RefPtr(this), aRequest,
@@ -684,26 +681,6 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
nsAutoCStringN<64> encodedHash =
":"_ns + aDict->GetHash() + ":"_ns;
- LOG_DICTIONARIES(
- ("Setting Available-Dictionary: %s", encodedHash.get()));
- rv = aRequest->SetHeader(
- nsHttp::Available_Dictionary, encodedHash, false,
- nsHttpHeaderArray::eVarietyRequestOverride);
- if (NS_FAILED(rv)) {
- (aCallback)(nullptr);
- return rv;
- }
- if (!aDict->GetId().IsEmpty()) {
- LOG_DICTIONARIES(("Setting Dictionary-Id: %s",
- PromiseFlatCString(aDict->GetId()).get()));
- rv = aRequest->SetHeader(
- nsHttp::Dictionary_Id, aDict->GetId(), false,
- nsHttpHeaderArray::eVarietyRequestOverride);
- if (NS_FAILED(rv)) {
- (aCallback)(nullptr);
- return rv;
- }
- }
// Need to retain access to the dictionary until the request
// completes. Note that this includes if the dictionary we offered
// gets replaced by another request while we're waiting for a
@@ -711,7 +688,34 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
// dictionary into memory before overwriting it and store in dict
// temporarily.
aRequest->SetDictionary(aDict);
- (aCallback)(aDict);
+
+ // We want to make sure that the cache entry doesn't disappear out
+ // from under us if we set the header, so do the callback to
+ // Prefetch() the entry before adding the headers (so we don't
+ // have to remove the headers if Prefetch() fails). It might fail
+ // if something is asynchronously Dooming the entry but the
+ // DictionaryCache hasn't been updated to remove the entry yet (or
+ // any other thing that were to desynchronize the DictionaryCache
+ // with the actual cache.
+ if ((aCallback)(aDict)) {
+ LOG_DICTIONARIES(
+ ("Setting Available-Dictionary: %s", encodedHash.get()));
+ rv = aRequest->SetHeader(
+ nsHttp::Available_Dictionary, encodedHash, false,
+ nsHttpHeaderArray::eVarietyRequestOverride);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+ if (!aDict->GetId().IsEmpty()) {
+ nsCString id(nsPrintfCString("\"%s\"", aDict->GetId().get()));
+ rv = aRequest->SetHeader(
+ nsHttp::Dictionary_Id, aDict->GetId(), false,
+ nsHttpHeaderArray::eVarietyRequestOverride);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+ }
+ }
return NS_OK;
}
rv = aRequest->SetHeader(
diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h
@@ -119,7 +119,7 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
[[nodiscard]] nsresult AddAcceptAndDictionaryHeaders(
nsIURI* aURI, nsHttpRequestHead* aRequest, bool aSecure,
- const std::function<void(DictionaryCacheEntry*)>& aCallback);
+ const std::function<bool(DictionaryCacheEntry*)>& aCallback);
[[nodiscard]] nsresult AddStandardRequestHeaders(
nsHttpRequestHead*, nsIURI* aURI, ExtContentPolicyType aContentPolicyType,
bool aShouldResistFingerprinting);
