Skip to content

Commit

Permalink
Bug 1646056 - Use const references as keys instead of raw pointers fo…
Browse files Browse the repository at this point in the history
…r PreloadHashKey. r=mayhemer

Feels a bit more natural for the callers this way. This should have no
behavior change.

Differential Revision: https://phabricator.services.mozilla.com/D79831
  • Loading branch information
emilio committed Jun 18, 2020
1 parent 939c7c4 commit 6bb08bd
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 68 deletions.
2 changes: 1 addition & 1 deletion dom/base/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11415,7 +11415,7 @@ void Document::MaybePreLoadImage(nsIURI* aUri,
PreloadHashKey key = PreloadHashKey::CreateAsImage(
aUri, NodePrincipal(),
dom::Element::StringToCORSMode(aCrossOriginAttr));
if (!mPreloadService.PreloadExists(&key)) {
if (!mPreloadService.PreloadExists(key)) {
PreLoadImage(aUri, aCrossOriginAttr, aReferrerPolicy, aIsImgSet,
aLinkPreload);
}
Expand Down
2 changes: 1 addition & 1 deletion dom/fetch/FetchDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ already_AddRefed<PreloaderBase> FetchDriver::FindPreload(nsIURI* aURI) {
// OK, this request can be satisfied by a preloaded response, try to find one.

auto preloadKey = PreloadHashKey::CreateAsFetch(aURI, cors);
return mDocument->Preloads().LookupPreload(&preloadKey);
return mDocument->Preloads().LookupPreload(preloadKey);
}

void FetchDriver::UpdateReferrerInfoFromNewChannel(nsIChannel* aChannel) {
Expand Down
2 changes: 1 addition & 1 deletion dom/script/ScriptLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,7 @@ nsresult ScriptLoader::StartLoad(ScriptLoadRequest* aRequest) {

auto key = PreloadHashKey::CreateAsScript(
aRequest->mURI, aRequest->CORSMode(), aRequest->mKind);
aRequest->NotifyOpen(&key, channel, mDocument,
aRequest->NotifyOpen(key, channel, mDocument,
aRequest->IsLinkPreloadScript());

rv = channel->AsyncOpen(loader);
Expand Down
2 changes: 1 addition & 1 deletion dom/xhr/XMLHttpRequestMainThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2744,7 +2744,7 @@ already_AddRefed<PreloaderBase> XMLHttpRequestMainThread::FindPreload() {
nsCOMPtr<nsIReferrerInfo> referrerInfo =
ReferrerInfo::CreateForFetch(mPrincipal, doc);
auto key = PreloadHashKey::CreateAsFetch(mRequestURL, cors);
RefPtr<PreloaderBase> preload = doc->Preloads().LookupPreload(&key);
RefPtr<PreloaderBase> preload = doc->Preloads().LookupPreload(key);
if (!preload) {
return nullptr;
}
Expand Down
8 changes: 4 additions & 4 deletions image/imgLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1689,7 +1689,7 @@ bool imgLoader::ValidateRequestWithNewChannel(
proxy->PrioritizeAsPreload();
auto preloadKey = PreloadHashKey::CreateAsImage(
aURI, aTriggeringPrincipal, ConvertToCORSMode(aCORSMode));
proxy->NotifyOpen(&preloadKey, aLoadingDocument, true);
proxy->NotifyOpen(preloadKey, aLoadingDocument, true);
}

// Attach the proxy without notifying
Expand Down Expand Up @@ -1756,7 +1756,7 @@ bool imgLoader::ValidateRequestWithNewChannel(
req->PrioritizeAsPreload();
auto preloadKey = PreloadHashKey::CreateAsImage(
aURI, aTriggeringPrincipal, ConvertToCORSMode(aCORSMode));
req->NotifyOpen(&preloadKey, aLoadingDocument, true);
req->NotifyOpen(preloadKey, aLoadingDocument, true);
}

// Add the proxy without notifying
Expand Down Expand Up @@ -2220,7 +2220,7 @@ nsresult imgLoader::LoadImage(
auto key = PreloadHashKey::CreateAsImage(aURI, aTriggeringPrincipal,
ConvertToCORSMode(corsmode));
if (RefPtr<PreloaderBase> preload =
aLoadingDocument->Preloads().LookupPreload(&key)) {
aLoadingDocument->Preloads().LookupPreload(key)) {
RefPtr<imgRequestProxy> proxy = do_QueryObject(preload);
MOZ_ASSERT(proxy);

Expand Down Expand Up @@ -2449,7 +2449,7 @@ nsresult imgLoader::LoadImage(
proxy->PrioritizeAsPreload();
auto preloadKey = PreloadHashKey::CreateAsImage(
aURI, aTriggeringPrincipal, ConvertToCORSMode(corsmode));
proxy->NotifyOpen(&preloadKey, aLoadingDocument, true);
proxy->NotifyOpen(preloadKey, aLoadingDocument, true);
}

// Note that it's OK to add here even if the request is done. If it is,
Expand Down
2 changes: 1 addition & 1 deletion layout/style/FontFaceSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ nsresult FontFaceSet::StartLoad(gfxUserFontEntry* aUserFontEntry,
auto preloadKey =
PreloadHashKey::CreateAsFont(aFontFaceSrc->mURI->get(), CORS_ANONYMOUS);
RefPtr<PreloaderBase> preload =
mDocument->Preloads().LookupPreload(&preloadKey);
mDocument->Preloads().LookupPreload(preloadKey);

if (preload) {
fontLoader = new nsFontFaceLoader(aUserFontEntry, aFontFaceSrc->mURI->get(),
Expand Down
2 changes: 1 addition & 1 deletion layout/style/Loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ nsresult Loader::LoadSheet(SheetLoadData& aLoadData, SheetState aSheetState) {
}

auto preloadKey = PreloadHashKey::CreateAsStyle(aLoadData);
streamLoader->NotifyOpen(&preloadKey, channel, mDocument,
streamLoader->NotifyOpen(preloadKey, channel, mDocument,
aLoadData.mIsPreload == IsPreload::FromLink);

rv = channel->AsyncOpen(streamLoader);
Expand Down
2 changes: 1 addition & 1 deletion uriloader/preload/FetchPreloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ FetchPreloader::FetchPreloader()
FetchPreloader::FetchPreloader(nsContentPolicyType aContentPolicyType)
: mContentPolicyType(aContentPolicyType) {}

nsresult FetchPreloader::OpenChannel(PreloadHashKey* aKey, nsIURI* aURI,
nsresult FetchPreloader::OpenChannel(const PreloadHashKey& aKey, nsIURI* aURI,
const CORSMode aCORSMode,
const dom::ReferrerPolicy& aReferrerPolicy,
dom::Document* aDocument) {
Expand Down
2 changes: 1 addition & 1 deletion uriloader/preload/FetchPreloader.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class FetchPreloader : public PreloaderBase, public nsIStreamListener {
NS_DECL_NSISTREAMLISTENER

FetchPreloader();
nsresult OpenChannel(PreloadHashKey* aKey, nsIURI* aURI,
nsresult OpenChannel(const PreloadHashKey& aKey, nsIURI* aURI,
const CORSMode aCORSMode,
const dom::ReferrerPolicy& aReferrerPolicy,
dom::Document* aDocument);
Expand Down
8 changes: 4 additions & 4 deletions uriloader/preload/PreloadHashKey.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class PreloadHashKey : public nsURIHashKey {
public:
enum class ResourceType : uint8_t { NONE, SCRIPT, STYLE, IMAGE, FONT, FETCH };

typedef PreloadHashKey* KeyType;
typedef const PreloadHashKey* KeyTypePointer;
using KeyType = const PreloadHashKey&;
using KeyTypePointer = const PreloadHashKey*;

PreloadHashKey() = default;
PreloadHashKey(const nsIURI* aKey, ResourceType aAs);
Expand Down Expand Up @@ -67,9 +67,9 @@ class PreloadHashKey : public nsURIHashKey {
// Construct key for "font"
static PreloadHashKey CreateAsFont(nsIURI* aURI, CORSMode aCORSMode);

KeyType GetKey() const { return const_cast<PreloadHashKey*>(this); }
KeyType GetKey() const { return *this; }
KeyTypePointer GetKeyPointer() const { return this; }
static KeyTypePointer KeyToPointer(KeyType aKey) { return aKey; }
static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }

bool KeyEquals(KeyTypePointer aOther) const;
static PLDHashNumber HashKey(KeyTypePointer aKey);
Expand Down
46 changes: 21 additions & 25 deletions uriloader/preload/PreloadService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

namespace mozilla {

bool PreloadService::RegisterPreload(PreloadHashKey* aKey,
bool PreloadService::RegisterPreload(const PreloadHashKey& aKey,
PreloaderBase* aPreload) {
if (PreloadExists(aKey)) {
return false;
Expand All @@ -26,20 +26,20 @@ bool PreloadService::RegisterPreload(PreloadHashKey* aKey,
return true;
}

void PreloadService::DeregisterPreload(PreloadHashKey* aKey) {
void PreloadService::DeregisterPreload(const PreloadHashKey& aKey) {
mPreloads.Remove(aKey);
}

void PreloadService::ClearAllPreloads() { mPreloads.Clear(); }

bool PreloadService::PreloadExists(PreloadHashKey* aKey) {
bool PreloadService::PreloadExists(const PreloadHashKey& aKey) {
bool found;
mPreloads.GetWeak(aKey, &found);
return found;
}

already_AddRefed<PreloaderBase> PreloadService::LookupPreload(
PreloadHashKey* aKey) const {
const PreloadHashKey& aKey) const {
return mPreloads.Get(aKey);
}

Expand Down Expand Up @@ -155,28 +155,24 @@ already_AddRefed<PreloaderBase> PreloadService::PreloadOrCoalesce(
return nullptr;
}

RefPtr<PreloaderBase> preload = LookupPreload(&preloadKey);
if (!preload) {
if (aAs.LowerCaseEqualsASCII("script")) {
PreloadScript(uri, aType, aCharset, aCORS, aReferrerPolicy, aIntegrity,
true /* isInHead - TODO */);
} else if (aAs.LowerCaseEqualsASCII("style")) {
PreloadStyle(uri, aCharset, aCORS, aReferrerPolicy, aIntegrity);
} else if (aAs.LowerCaseEqualsASCII("image")) {
PreloadImage(uri, aCORS, aReferrerPolicy, isImgSet);
} else if (aAs.LowerCaseEqualsASCII("font")) {
PreloadFont(uri, aCORS, aReferrerPolicy);
} else if (aAs.LowerCaseEqualsASCII("fetch")) {
PreloadFetch(uri, aCORS, aReferrerPolicy);
}
if (RefPtr<PreloaderBase> preload = LookupPreload(preloadKey)) {
return preload.forget();
}

preload = LookupPreload(&preloadKey);
if (!preload) {
return nullptr;
}
if (aAs.LowerCaseEqualsASCII("script")) {
PreloadScript(uri, aType, aCharset, aCORS, aReferrerPolicy, aIntegrity,
true /* isInHead - TODO */);
} else if (aAs.LowerCaseEqualsASCII("style")) {
PreloadStyle(uri, aCharset, aCORS, aReferrerPolicy, aIntegrity);
} else if (aAs.LowerCaseEqualsASCII("image")) {
PreloadImage(uri, aCORS, aReferrerPolicy, isImgSet);
} else if (aAs.LowerCaseEqualsASCII("font")) {
PreloadFont(uri, aCORS, aReferrerPolicy);
} else if (aAs.LowerCaseEqualsASCII("fetch")) {
PreloadFetch(uri, aCORS, aReferrerPolicy);
}

return preload.forget();
return LookupPreload(preloadKey);
}

void PreloadService::PreloadScript(nsIURI* aURI, const nsAString& aType,
Expand Down Expand Up @@ -217,7 +213,7 @@ void PreloadService::PreloadFont(nsIURI* aURI, const nsAString& aCrossOrigin,

RefPtr<FontPreloader> preloader = new FontPreloader();
dom::ReferrerPolicy referrerPolicy = PreloadReferrerPolicy(aReferrerPolicy);
preloader->OpenChannel(&key, aURI, cors, referrerPolicy, mDocument);
preloader->OpenChannel(key, aURI, cors, referrerPolicy, mDocument);
}

void PreloadService::PreloadFetch(nsIURI* aURI, const nsAString& aCrossOrigin,
Expand All @@ -230,7 +226,7 @@ void PreloadService::PreloadFetch(nsIURI* aURI, const nsAString& aCrossOrigin,

RefPtr<FetchPreloader> preloader = new FetchPreloader();
dom::ReferrerPolicy referrerPolicy = PreloadReferrerPolicy(aReferrerPolicy);
preloader->OpenChannel(&key, aURI, cors, referrerPolicy, mDocument);
preloader->OpenChannel(key, aURI, cors, referrerPolicy, mDocument);
}

// static
Expand Down
9 changes: 5 additions & 4 deletions uriloader/preload/PreloadService.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,22 @@ class PreloadService {
//
// Returns false and does nothing if a preload is already registered under
// this key, true otherwise.
bool RegisterPreload(PreloadHashKey* aKey, PreloaderBase* aPreload);
bool RegisterPreload(const PreloadHashKey& aKey, PreloaderBase* aPreload);

// Called when the load is about to be cancelled. Exact behavior is to be
// determined yet.
void DeregisterPreload(PreloadHashKey* aKey);
void DeregisterPreload(const PreloadHashKey& aKey);

// Called when the scope is to go away.
void ClearAllPreloads();

// True when there is a preload registered under the key.
bool PreloadExists(PreloadHashKey* aKey);
bool PreloadExists(const PreloadHashKey& aKey);

// Returns an existing preload under the key or null, when there is none
// registered under the key.
already_AddRefed<PreloaderBase> LookupPreload(PreloadHashKey* aKey) const;
already_AddRefed<PreloaderBase> LookupPreload(
const PreloadHashKey& aKey) const;

void SetSpeculationBase(nsIURI* aURI) { mSpeculationBaseURI = aURI; }
already_AddRefed<nsIURI> GetPreloadURI(const nsAString& aURL);
Expand Down
10 changes: 5 additions & 5 deletions uriloader/preload/PreloaderBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ void PreloaderBase::AddLoadBackgroundFlag(nsIChannel* aChannel) {
aChannel->SetLoadFlags(loadFlags | nsIRequest::LOAD_BACKGROUND);
}

void PreloaderBase::NotifyOpen(PreloadHashKey* aKey, dom::Document* aDocument,
bool aIsPreload) {
void PreloaderBase::NotifyOpen(const PreloadHashKey& aKey,
dom::Document* aDocument, bool aIsPreload) {
if (aDocument && !aDocument->Preloads().RegisterPreload(aKey, this)) {
// This means there is already a preload registered under this key in this
// document. We only allow replacement when this is a regular load.
Expand All @@ -102,11 +102,11 @@ void PreloaderBase::NotifyOpen(PreloadHashKey* aKey, dom::Document* aDocument,
aDocument->Preloads().RegisterPreload(aKey, this);
}

mKey = *aKey;
mKey = aKey;
mIsUsed = !aIsPreload;
}

void PreloaderBase::NotifyOpen(PreloadHashKey* aKey, nsIChannel* aChannel,
void PreloaderBase::NotifyOpen(const PreloadHashKey& aKey, nsIChannel* aChannel,
dom::Document* aDocument, bool aIsPreload) {
NotifyOpen(aKey, aDocument, aIsPreload);
mChannel = aChannel;
Expand Down Expand Up @@ -152,7 +152,7 @@ void PreloaderBase::NotifyUsage(LoadBackground aLoadBackground) {

void PreloaderBase::RemoveSelf(dom::Document* aDocument) {
if (aDocument) {
aDocument->Preloads().DeregisterPreload(&mKey);
aDocument->Preloads().DeregisterPreload(mKey);
}
}

Expand Down
4 changes: 2 additions & 2 deletions uriloader/preload/PreloaderBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ class PreloaderBase : public SupportsWeakPtr<PreloaderBase>,
// Called by resource loaders to register this preload in the document's
// preload service to provide coalescing, and access to the preload when it
// should be used for an actual load.
void NotifyOpen(PreloadHashKey* aKey, dom::Document* aDocument,
void NotifyOpen(const PreloadHashKey& aKey, dom::Document* aDocument,
bool aIsPreload);
void NotifyOpen(PreloadHashKey* aKey, nsIChannel* aChannel,
void NotifyOpen(const PreloadHashKey& aKey, nsIChannel* aChannel,
dom::Document* aDocument, bool aIsPreload);

// Called when the load is about to be started all over again and thus this
Expand Down
Loading

0 comments on commit 6bb08bd

Please sign in to comment.