Skip to content

Commit

Permalink
Bug 1443379 - Stop pretending cacheKey can be anything other than uin…
Browse files Browse the repository at this point in the history
…t32_t, r=smaug
  • Loading branch information
mystor committed Apr 10, 2018
1 parent adc5b07 commit b286bc1
Show file tree
Hide file tree
Showing 25 changed files with 74 additions and 216 deletions.
2 changes: 1 addition & 1 deletion browser/components/shell/nsMacShellService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ nsMacShellService::SetDesktopBackground(nsIDOMElement* aElement,
loadContext = do_QueryInterface(docShell);
}

return wbp->SaveURI(imageURI, nullptr,
return wbp->SaveURI(imageURI, 0,
docURI, content->OwnerDoc()->GetReferrerPolicy(),
nullptr, nullptr,
mBackgroundFile, loadContext);
Expand Down
4 changes: 0 additions & 4 deletions devtools/server/actors/source.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,6 @@ let SourceActor = ActorClassWithSpec(sourceSpec, {
if (loadFromCache &&
webNav.currentDocumentChannel instanceof Ci.nsICacheInfoChannel) {
cacheKey = webNav.currentDocumentChannel.cacheKey;
assert(
cacheKey,
"Could not fetch the cacheKey from the related document."
);
}
}

Expand Down
4 changes: 2 additions & 2 deletions devtools/shared/DevToolsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ function mainThreadFetch(urlIn, aOptions = { loadFromCache: true,
window: null,
charset: null,
principal: null,
cacheKey: null }) {
cacheKey: 0 }) {
// Create a channel.
let url = urlIn.split(" -> ").pop();
let channel;
Expand All @@ -532,7 +532,7 @@ function mainThreadFetch(urlIn, aOptions = { loadFromCache: true,
// When loading from cache, the cacheKey allows us to target a specific
// SHEntry and offer ways to restore POST requests from cache.
if (aOptions.loadFromCache &&
aOptions.cacheKey && channel instanceof Ci.nsICacheInfoChannel) {
aOptions.cacheKey != 0 && channel instanceof Ci.nsICacheInfoChannel) {
channel.cacheKey = aOptions.cacheKey;
}

Expand Down
2 changes: 1 addition & 1 deletion devtools/shared/gcli/commands/screenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ var saveToFile = Task.async(function* (context, reply) {
let listener = new DownloadListener(window, tr);
persist.progressListener = listener;
persist.savePrivacyAwareURI(sourceURI,
null,
0,
document.documentURIObject,
Ci.nsIHttpChannel.REFERRER_POLICY_UNSET,
null,
Expand Down
24 changes: 12 additions & 12 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10006,7 +10006,7 @@ nsDocShell::InternalLoad(nsIURI* aURI,
mLoadType, true, true, true);

nsCOMPtr<nsIInputStream> postData;
nsCOMPtr<nsISupports> cacheKey;
uint32_t cacheKey = 0;

bool scrollRestorationIsManual = false;
if (mOSHE) {
Expand All @@ -10021,7 +10021,7 @@ nsDocShell::InternalLoad(nsIURI* aURI,
// wouldn't want here.
if (aLoadType & LOAD_CMD_NORMAL) {
mOSHE->GetPostData(getter_AddRefs(postData));
mOSHE->GetCacheKey(getter_AddRefs(cacheKey));
mOSHE->GetCacheKey(&cacheKey);

// Link our new SHEntry to the old SHEntry's back/forward
// cache data, since the two SHEntries correspond to the
Expand Down Expand Up @@ -10057,7 +10057,7 @@ nsDocShell::InternalLoad(nsIURI* aURI,

// Make sure we won't just repost without hitting the
// cache first
if (cacheKey) {
if (cacheKey != 0) {
mOSHE->SetCacheKey(cacheKey);
}
}
Expand Down Expand Up @@ -10890,12 +10890,12 @@ nsDocShell::DoURILoad(nsIURI* aURI,

nsCOMPtr<nsICacheInfoChannel> cacheChannel(do_QueryInterface(channel));
/* Get the cache Key from SH */
nsCOMPtr<nsISupports> cacheKey;
uint32_t cacheKey = 0;
if (cacheChannel) {
if (mLSHE) {
mLSHE->GetCacheKey(getter_AddRefs(cacheKey));
mLSHE->GetCacheKey(&cacheKey);
} else if (mOSHE) { // for reload cases
mOSHE->GetCacheKey(getter_AddRefs(cacheKey));
mOSHE->GetCacheKey(&cacheKey);
}
}

Expand All @@ -10922,7 +10922,7 @@ nsDocShell::DoURILoad(nsIURI* aURI,
* data *only* from the cache. If it is a normal reload, the
* cache is free to go to the server for updated postdata.
*/
if (cacheChannel && cacheKey) {
if (cacheChannel && cacheKey != 0) {
if (mLoadType == LOAD_HISTORY ||
mLoadType == LOAD_RELOAD_CHARSET_CHANGE) {
cacheChannel->SetCacheKey(cacheKey);
Expand All @@ -10947,7 +10947,7 @@ nsDocShell::DoURILoad(nsIURI* aURI,
mLoadType == LOAD_RELOAD_CHARSET_CHANGE ||
mLoadType == LOAD_RELOAD_CHARSET_CHANGE_BYPASS_CACHE ||
mLoadType == LOAD_RELOAD_CHARSET_CHANGE_BYPASS_PROXY_AND_CACHE) {
if (cacheChannel && cacheKey) {
if (cacheChannel && cacheKey != 0) {
cacheChannel->SetCacheKey(cacheKey);
}
}
Expand Down Expand Up @@ -11517,10 +11517,10 @@ nsDocShell::OnNewURI(nsIURI* aURI, nsIChannel* aChannel,
" reloads unless we're in a newly created iframe!");

nsCOMPtr<nsICacheInfoChannel> cacheChannel(do_QueryInterface(aChannel));
nsCOMPtr<nsISupports> cacheKey;
uint32_t cacheKey = 0;
// Get the Cache Key and store it in SH.
if (cacheChannel) {
cacheChannel->GetCacheKey(getter_AddRefs(cacheKey));
cacheChannel->GetCacheKey(&cacheKey);
}
// If we already have a loading history entry, store the new cache key
// in it. Otherwise, since we're doing a reload and won't be updating
Expand Down Expand Up @@ -12091,7 +12091,7 @@ nsDocShell::AddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel,
bool loadReplace = false;
nsCOMPtr<nsIURI> referrerURI;
uint32_t referrerPolicy = mozilla::net::RP_Unset;
nsCOMPtr<nsISupports> cacheKey;
uint32_t cacheKey = 0;
nsCOMPtr<nsIPrincipal> triggeringPrincipal = aTriggeringPrincipal;
nsCOMPtr<nsIPrincipal> principalToInherit = aPrincipalToInherit;
bool expired = false;
Expand All @@ -12104,7 +12104,7 @@ nsDocShell::AddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel,
* in SH.
*/
if (cacheChannel) {
cacheChannel->GetCacheKey(getter_AddRefs(cacheKey));
cacheChannel->GetCacheKey(&cacheKey);
}
nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aChannel));

Expand Down
4 changes: 2 additions & 2 deletions docshell/shistory/nsISHEntry.idl
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ interface nsISHEntry : nsISupports
attribute unsigned long ID;

/** attribute to set and get the cache key for the entry */
attribute nsISupports cacheKey;
attribute unsigned long cacheKey;

/** attribute to indicate whether layoutHistoryState should be saved */
attribute boolean saveLayoutStateFlag;
Expand Down Expand Up @@ -203,7 +203,7 @@ interface nsISHEntry : nsISupports
[noscript] void create(in nsIURI URI, in AString title,
in nsIInputStream inputStream,
in nsILayoutHistoryState layoutHistoryState,
in nsISupports cacheKey, in ACString contentType,
in unsigned long cacheKey, in ACString contentType,
in nsIPrincipal triggeringPrincipal,
in nsIPrincipal principalToInherit,
in nsIDRef docshellID,
Expand Down
7 changes: 3 additions & 4 deletions docshell/shistory/nsSHEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,14 @@ nsSHEntry::SetIsSubFrame(bool aFlag)
}

NS_IMETHODIMP
nsSHEntry::GetCacheKey(nsISupports** aResult)
nsSHEntry::GetCacheKey(uint32_t* aResult)
{
*aResult = mShared->mCacheKey;
NS_IF_ADDREF(*aResult);
return NS_OK;
}

NS_IMETHODIMP
nsSHEntry::SetCacheKey(nsISupports* aCacheKey)
nsSHEntry::SetCacheKey(uint32_t aCacheKey)
{
mShared->mCacheKey = aCacheKey;
return NS_OK;
Expand Down Expand Up @@ -440,7 +439,7 @@ NS_IMETHODIMP
nsSHEntry::Create(nsIURI* aURI, const nsAString& aTitle,
nsIInputStream* aInputStream,
nsILayoutHistoryState* aLayoutHistoryState,
nsISupports* aCacheKey, const nsACString& aContentType,
uint32_t aCacheKey, const nsACString& aContentType,
nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aPrincipalToInherit,
const nsID& aDocShellID,
Expand Down
1 change: 1 addition & 0 deletions docshell/shistory/nsSHEntryShared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ nsSHEntryShared::Shutdown()

nsSHEntryShared::nsSHEntryShared()
: mDocShellID({0})
, mCacheKey(0)
, mLastTouched(0)
, mID(gSHEntrySharedID++)
, mViewerBounds(0, 0, 0, 0)
Expand Down
2 changes: 1 addition & 1 deletion docshell/shistory/nsSHEntryShared.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class nsSHEntryShared final
nsCOMPtr<nsIPrincipal> mPrincipalToInherit;
nsCString mContentType;

nsCOMPtr<nsISupports> mCacheKey;
uint32_t mCacheKey;
uint32_t mLastTouched;

// These members aren't copied by nsSHEntryShared::Duplicate() because
Expand Down
2 changes: 1 addition & 1 deletion dom/base/nsContentAreaDragDrop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ nsContentAreaDragDropDataProvider::SaveURIToFile(nsIURI* inSourceURI,
persist->SetPersistFlags(nsIWebBrowserPersist::PERSIST_FLAGS_AUTODETECT_APPLY_CONVERSION);

// referrer policy can be anything since the referrer is nullptr
return persist->SavePrivacyAwareURI(inSourceURI, nullptr, nullptr,
return persist->SavePrivacyAwareURI(inSourceURI, 0, nullptr,
mozilla::net::RP_Unset,
nullptr, nullptr,
inDestFile, isPrivate);
Expand Down
19 changes: 4 additions & 15 deletions dom/webbrowserpersist/WebBrowserPersistLocalDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,23 +168,12 @@ WebBrowserPersistLocalDocument::GetContentDisposition(nsAString& aCD)
NS_IMETHODIMP
WebBrowserPersistLocalDocument::GetCacheKey(uint32_t* aKey)
{
*aKey = 0;
nsCOMPtr<nsISHEntry> history = GetHistory();
if (!history) {
*aKey = 0;
return NS_OK;
}
nsCOMPtr<nsISupports> abstractKey;
nsresult rv = history->GetCacheKey(getter_AddRefs(abstractKey));
if (NS_WARN_IF(NS_FAILED(rv)) || !abstractKey) {
*aKey = 0;
return NS_OK;
if (history) {
history->GetCacheKey(aKey);
}
nsCOMPtr<nsISupportsPRUint32> u32 = do_QueryInterface(abstractKey);
if (NS_WARN_IF(!u32)) {
*aKey = 0;
return NS_OK;
}
return u32->GetData(aKey);
return NS_OK;
}

NS_IMETHODIMP
Expand Down
9 changes: 3 additions & 6 deletions dom/webbrowserpersist/nsIWebBrowserPersist.idl
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,7 @@ interface nsIWebBrowserPersist : nsICancelable
* @param aURI URI to save to file. Some implementations of this interface
* may also support <CODE>nullptr</CODE> to imply the currently
* loaded URI.
* @param aCacheKey An object representing the URI in the cache or
* <CODE>nullptr</CODE>. This can be a necko cache key,
* an nsIWebPageDescriptor, or the currentDescriptor of an
* nsIWebPageDescriptor.
* @param aCacheKey The necko cache key integer.
* @param aReferrer The referrer URI to pass with an HTTP request or
* <CODE>nullptr</CODE>.
* @param aReferrerPolicy The referrer policy for when and what to send via
Expand All @@ -146,7 +143,7 @@ interface nsIWebBrowserPersist : nsICancelable
*
* @throws NS_ERROR_INVALID_ARG One or more arguments was invalid.
*/
void saveURI(in nsIURI aURI, in nsISupports aCacheKey,
void saveURI(in nsIURI aURI, in unsigned long aCacheKey,
in nsIURI aReferrer, in unsigned long aReferrerPolicy,
in nsIInputStream aPostData,
in string aExtraHeaders, in nsISupports aFile,
Expand All @@ -158,7 +155,7 @@ interface nsIWebBrowserPersist : nsICancelable
* of intermediate data, etc.)
* @see saveURI for all other parameter descriptions
*/
void savePrivacyAwareURI(in nsIURI aURI, in nsISupports aCacheKey,
void savePrivacyAwareURI(in nsIURI aURI, in unsigned long aCacheKey,
in nsIURI aReferrer, in unsigned long aReferrerPolicy,
in nsIInputStream aPostData,
in string aExtraHeaders, in nsISupports aFile,
Expand Down
43 changes: 6 additions & 37 deletions dom/webbrowserpersist/nsWebBrowserPersist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ NS_IMETHODIMP nsWebBrowserPersist::SetProgressListener(
}

NS_IMETHODIMP nsWebBrowserPersist::SaveURI(
nsIURI *aURI, nsISupports *aCacheKey,
nsIURI *aURI, uint32_t aCacheKey,
nsIURI *aReferrer, uint32_t aReferrerPolicy,
nsIInputStream *aPostData, const char *aExtraHeaders,
nsISupports *aFile, nsILoadContext* aPrivacyContext)
Expand All @@ -424,7 +424,7 @@ NS_IMETHODIMP nsWebBrowserPersist::SaveURI(
}

NS_IMETHODIMP nsWebBrowserPersist::SavePrivacyAwareURI(
nsIURI *aURI, nsISupports *aCacheKey,
nsIURI *aURI, uint32_t aCacheKey,
nsIURI *aReferrer, uint32_t aReferrerPolicy,
nsIInputStream *aPostData, const char *aExtraHeaders,
nsISupports *aFile, bool aIsPrivate)
Expand Down Expand Up @@ -631,7 +631,7 @@ nsWebBrowserPersist::SerializeNextFile()

// The Referrer Policy doesn't matter here since the referrer is
// nullptr.
rv = SaveURIInternal(uri, nullptr, nullptr,
rv = SaveURIInternal(uri, 0, nullptr,
mozilla::net::RP_Unset, nullptr, nullptr,
fileAsURI, true, mIsPrivate);
// If SaveURIInternal fails, then it will have called EndDownload,
Expand Down Expand Up @@ -1328,7 +1328,7 @@ nsWebBrowserPersist::AppendPathToURI(nsIURI *aURI, const nsAString & aPath, nsCO
}

nsresult nsWebBrowserPersist::SaveURIInternal(
nsIURI *aURI, nsISupports *aCacheKey, nsIURI *aReferrer,
nsIURI *aURI, uint32_t aCacheKey, nsIURI *aReferrer,
uint32_t aReferrerPolicy, nsIInputStream *aPostData,
const char *aExtraHeaders, nsIURI *aFile,
bool aCalcFileExt, bool aIsPrivate)
Expand All @@ -1350,36 +1350,6 @@ nsresult nsWebBrowserPersist::SaveURIInternal(
loadFlags |= nsIRequest::LOAD_FROM_CACHE;
}

// Extract the cache key
nsCOMPtr<nsISupports> cacheKey;
if (aCacheKey)
{
// Test if the cache key is actually a web page descriptor (docshell)
// or session history entry.
nsCOMPtr<nsISHEntry> shEntry = do_QueryInterface(aCacheKey);
if (!shEntry)
{
nsCOMPtr<nsIWebPageDescriptor> webPageDescriptor =
do_QueryInterface(aCacheKey);
if (webPageDescriptor)
{
nsCOMPtr<nsISupports> currentDescriptor;
webPageDescriptor->GetCurrentDescriptor(getter_AddRefs(currentDescriptor));
shEntry = do_QueryInterface(currentDescriptor);
}
}

if (shEntry)
{
shEntry->GetCacheKey(getter_AddRefs(cacheKey));
}
else
{
// Assume a plain cache key
cacheKey = aCacheKey;
}
}

// Open a channel to the URI
nsCOMPtr<nsIChannel> inputChannel;
rv = NS_NewChannel(getter_AddRefs(inputChannel),
Expand Down Expand Up @@ -1452,9 +1422,8 @@ nsresult nsWebBrowserPersist::SaveURIInternal(

// Cache key
nsCOMPtr<nsICacheInfoChannel> cacheChannel(do_QueryInterface(httpChannel));
if (cacheChannel && cacheKey)
{
cacheChannel->SetCacheKey(cacheKey);
if (cacheChannel && aCacheKey != 0) {
cacheChannel->SetCacheKey(aCacheKey);
}

// Headers
Expand Down
2 changes: 1 addition & 1 deletion dom/webbrowserpersist/nsWebBrowserPersist.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class nsWebBrowserPersist final : public nsIInterfaceRequestor,
private:
virtual ~nsWebBrowserPersist();
nsresult SaveURIInternal(
nsIURI *aURI, nsISupports *aCacheKey, nsIURI *aReferrer,
nsIURI *aURI, uint32_t aCacheKey, nsIURI *aReferrer,
uint32_t aReferrerPolicy, nsIInputStream *aPostData,
const char *aExtraHeaders, nsIURI *aFile,
bool aCalcFileExt, bool aIsPrivate);
Expand Down
13 changes: 6 additions & 7 deletions netwerk/base/nsICacheInfoChannel.idl
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,22 @@ interface nsICacheInfoChannel : nsISupports
uint64_t getCacheEntryId();

/**
* Set/get the cache key... uniquely identifies the data in the cache
* for this channel. Holding a reference to this key does NOT prevent
* the cached data from being removed.
* Set/get the cache key. This integer uniquely identifies the data in
* the cache for this channel.
*
* A cache key retrieved from a particular instance of nsICacheInfoChannel
* could be set on another instance of nsICacheInfoChannel provided the
* underlying implementations are compatible and provided the new
* underlying implementations are compatible and provided the new
* channel instance was created with the same URI. The implementation of
* nsICacheInfoChannel would be expected to use the cache entry identified
* by the cache token. Depending on the value of nsIRequest::loadFlags,
* the cache entry may be validated, overwritten, or simply read.
*
* The cache key may be NULL indicating that the URI of the channel is
* sufficient to locate the same cache entry. Setting a NULL cache key
* The cache key may be 0 indicating that the URI of the channel is
* sufficient to locate the same cache entry. Setting a 0 cache key
* is likewise valid.
*/
attribute nsISupports cacheKey;
attribute unsigned long cacheKey;

/**
* Tells the channel to behave as if the LOAD_FROM_CACHE flag has been set,
Expand Down
Loading

0 comments on commit b286bc1

Please sign in to comment.