commit dd77492425eb3746d216d214b97cc0be6dc08844
parent 0c8682ebee779910a79cfd1f2dcbd60526e9bd8e
Author: Randell Jesup <rjesup@mozilla.com>
Date: Wed, 1 Oct 2025 18:46:02 +0000
Bug 1917974: Clean up call to GetDictionaryFor() r=valentin,necko-reviewers
Differential Revision: https://phabricator.services.mozilla.com/D263067
Diffstat:
1 file changed, 50 insertions(+), 44 deletions(-)
diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -33,6 +33,7 @@
#include "mozilla/Printf.h"
#include "mozilla/RandomNum.h"
#include "mozilla/SHA1.h"
+#include "mozilla/ScopeExit.h"
#include "mozilla/Sprintf.h"
#include "mozilla/StaticPrefs_network.h"
#include "mozilla/StaticPrefs_privacy.h"
@@ -659,72 +660,77 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
bool aSecure, bool& aAsync,
const std::function<bool(bool, DictionaryCacheEntry*)>& aCallback) {
LOG(("Adding Dictionary headers"));
+ auto guard = MakeScopeExit([&]() { (aCallback)(false, nullptr); });
+
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.
if (StaticPrefs::network_http_dictionaries_enable()) {
+ // Note: this is async; the lambda can happen later
+ // aCallback will now be owned by GetDictionaryFor
+ guard.release();
mDictionaryCache->GetDictionaryFor(
aURI, aType, aAsync,
[self = RefPtr(this), aRequest, aCallback](
bool aNeedsResume, DictionaryCacheEntry* aDict) {
nsresult rv;
- if (aDict) {
- nsAutoCStringN<64> encodedHash =
- ":"_ns + aDict->GetHash() + ":"_ns;
-
- // 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
- // response; in that case we need to read in a copy of the
- // dictionary into memory before overwriting it and store in dict
- // temporarily.
- aRequest->SetDictionary(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)(aNeedsResume, aDict)) {
- LOG_DICTIONARIES(
- ("Setting Available-Dictionary: %s", encodedHash.get()));
+ if (!aDict) {
+ rv = aRequest->SetHeader(
+ nsHttp::Accept_Encoding, self->mHttpsAcceptEncodings, false,
+ nsHttpHeaderArray::eVarietyRequestOverride);
+ (aCallback)(false, nullptr);
+ return rv;
+ }
+
+ nsAutoCStringN<64> encodedHash = ":"_ns + aDict->GetHash() + ":"_ns;
+
+ // 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
+ // response; in that case we need to read in a copy of the
+ // dictionary into memory before overwriting it and store in dict
+ // temporarily.
+ aRequest->SetDictionary(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)(aNeedsResume, 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()) {
+ nsPrintfCString id("\"%s\"", aDict->GetId().get());
+ LOG_DICTIONARIES(("Setting Dictionary-Id: %s", id.get()));
rv = aRequest->SetHeader(
- nsHttp::Available_Dictionary, encodedHash, false,
+ nsHttp::Dictionary_Id, id, 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 aRequest->SetHeader(
- nsHttp::Accept_Encoding, self->mDictionaryAcceptEncodings,
- false, nsHttpHeaderArray::eVarietyRequestOverride);
}
- return NS_OK;
+ return aRequest->SetHeader(
+ nsHttp::Accept_Encoding, self->mDictionaryAcceptEncodings,
+ false, nsHttpHeaderArray::eVarietyRequestOverride);
}
- rv = aRequest->SetHeader(
- nsHttp::Accept_Encoding, self->mHttpsAcceptEncodings, false,
- nsHttpHeaderArray::eVarietyRequestOverride);
- (aCallback)(false, nullptr);
- return rv;
+ return NS_OK;
});
} else {
rv = aRequest->SetHeader(nsHttp::Accept_Encoding, mHttpsAcceptEncodings,
false,
nsHttpHeaderArray::eVarietyRequestOverride);
aAsync = false;
- (aCallback)(false, nullptr);
}
} else {
// We need to not override a previous setting of 'identity' (for range
@@ -737,8 +743,8 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
nsHttpHeaderArray::eVarietyRequestOverride);
}
aAsync = false;
- (aCallback)(false, nullptr);
}
+ // guard may call aCallback here
return rv;
}
