From ac22fc9f3902973ab0dffd1fde9846b13e272af2 Mon Sep 17 00:00:00 2001 From: Anny Gakhokidze Date: Fri, 29 Mar 2019 15:48:59 -0400 Subject: [PATCH] Bug 1539482 - Reduce the number of IPC calls on nsISHEntry in nsDocShell::LoadHistoryEntry, r=peterv, r=nika for adding sync IPC messages In nsDocShell::LoadHistoryEntry method, when it gets called from parent process, there are 13 sync IPC calls on nsISHEntry that retrieve information from the session history entry and create a doc shell load state object using the retrieved information. By adding a new method 'CreateLoadInfo'on nsISHEntry, inside of which the doc shell load state object will be created (with appropriate data filled out) and returned, we eliminate 12 sync IPC call, resulting in just 1 IPC call to nsISHEntry::CreateLoadInfo. Differential Revision: https://phabricator.services.mozilla.com/D26042 --HG-- extra : rebase_source : a4e1fa52932fd5caabb59bd133e9fbee7f4d0e4a extra : amend_source : f4d9f01afac0337808ba347eb997ce83e6ae1488 extra : source : 6ad53b35c7b4be933a3db1e1d45fa3da8a57abad extra : histedit_source : c08d0cebcc11a3a4f64d01566cb62d9a334a12ec --- docshell/base/nsDocShell.cpp | 79 ++++++--------------------- docshell/base/nsDocShellLoadState.cpp | 16 +++++- docshell/base/nsDocShellLoadState.h | 6 ++ docshell/shistory/PSHEntry.ipdl | 4 +- docshell/shistory/SHEntryChild.cpp | 13 +++++ docshell/shistory/SHEntryParent.cpp | 6 ++ docshell/shistory/SHEntryParent.h | 1 + docshell/shistory/nsISHEntry.idl | 9 +++ docshell/shistory/nsSHEntry.cpp | 73 +++++++++++++++++++++++++ docshell/shistory/nsSHEntry.h | 1 + dom/ipc/DOMTypes.ipdlh | 6 +- ipc/ipdl/sync-messages.ini | 2 + 12 files changed, 151 insertions(+), 65 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 8f0435cc3413..c4dfcf04fdcf 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -11593,28 +11593,21 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) { } NS_ENSURE_TRUE(aEntry, NS_ERROR_FAILURE); - - nsCOMPtr uri = aEntry->GetURI(); - nsCOMPtr originalURI = aEntry->GetOriginalURI(); - nsCOMPtr resultPrincipalURI = aEntry->GetResultPrincipalURI(); - bool loadReplace = aEntry->GetLoadReplace(); - nsCOMPtr postData = aEntry->GetPostData(); - nsAutoCString contentType; - aEntry->GetContentType(contentType); - nsCOMPtr triggeringPrincipal = aEntry->GetTriggeringPrincipal(); - nsCOMPtr principalToInherit = aEntry->GetPrincipalToInherit(); - nsCOMPtr storagePrincipalToInherit = - aEntry->GetStoragePrincipalToInherit(); - nsCOMPtr csp = aEntry->GetCsp(); - nsCOMPtr referrerInfo = aEntry->GetReferrerInfo(); + nsresult rv; + RefPtr loadState; + rv = aEntry->CreateLoadInfo(getter_AddRefs(loadState)); + NS_ENSURE_SUCCESS(rv, rv); + // We are setting load type afterwards so we don't have to + // send it in an IPC message + loadState->SetLoadType(aLoadType); // Calling CreateAboutBlankContentViewer can set mOSHE to null, and if // that's the only thing holding a ref to aEntry that will cause aEntry to // die while we're loading it. So hold a strong ref to aEntry here, just // in case. nsCOMPtr kungFuDeathGrip(aEntry); - nsresult rv; - if (SchemeIsJavascript(uri)) { + + if (SchemeIsJavascript(loadState->URI())) { // We're loading a URL that will execute script from inside asyncOpen. // Replace the current document with about:blank now to prevent // anything from the current document from leaking into any JavaScript @@ -11622,9 +11615,9 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) { // Don't cache the presentation if we're going to just reload the // current entry. Caching would lead to trying to save the different // content viewers in the same nsISHEntry object. - rv = CreateAboutBlankContentViewer(principalToInherit, - storagePrincipalToInherit, nullptr, - nullptr, aEntry != mOSHE); + rv = CreateAboutBlankContentViewer(loadState->PrincipalToInherit(), + loadState->StoragePrincipalToInherit(), + nullptr, nullptr, aEntry != mOSHE); if (NS_FAILED(rv)) { // The creation of the intermittent about:blank content @@ -11633,11 +11626,12 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) { return NS_OK; } - if (!triggeringPrincipal) { + if (!loadState->TriggeringPrincipal()) { // Ensure that we have a triggeringPrincipal. Otherwise javascript: // URIs will pick it up from the about:blank page we just loaded, // and we don't really want even that in this case. - triggeringPrincipal = NullPrincipal::CreateWithInheritedAttributes(this); + nsCOMPtr principal = NullPrincipal::CreateWithInheritedAttributes(this); + loadState->SetTriggeringPrincipal(principal); } } @@ -11645,7 +11639,7 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) { * reload or shift-reload, take user's permission before we * repost the data to the server. */ - if ((aLoadType & LOAD_CMD_RELOAD) && postData) { + if ((aLoadType & LOAD_CMD_RELOAD) && loadState->PostDataStream()) { bool repost; rv = ConfirmRepost(&repost); if (NS_FAILED(rv)) { @@ -11658,50 +11652,13 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) { } } - // Do not inherit principal from document (security-critical!); - uint32_t flags = INTERNAL_LOAD_FLAGS_NONE; - - nsAutoString srcdoc; - nsCOMPtr baseURI; - if (aEntry->GetIsSrcdocEntry()) { - aEntry->GetSrcdocData(srcdoc); - baseURI = aEntry->GetBaseURI(); - flags |= INTERNAL_LOAD_FLAGS_IS_SRCDOC; - } else { - srcdoc = VoidString(); - } - // If there is no valid triggeringPrincipal, we deny the load - MOZ_ASSERT(triggeringPrincipal, + MOZ_ASSERT(loadState->TriggeringPrincipal(), "need a valid triggeringPrincipal to load from history"); - if (!triggeringPrincipal) { + if (!loadState->TriggeringPrincipal()) { return NS_ERROR_FAILURE; } - // Passing nullptr as aSourceDocShell gives the same behaviour as before - // aSourceDocShell was introduced. According to spec we should be passing - // the source browsing context that was used when the history entry was - // first created. bug 947716 has been created to address this issue. - Maybe> emplacedResultPrincipalURI; - emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI)); - - RefPtr loadState = new nsDocShellLoadState(uri); - loadState->SetReferrerInfo(referrerInfo); - loadState->SetOriginalURI(originalURI); - loadState->SetMaybeResultPrincipalURI(emplacedResultPrincipalURI); - loadState->SetLoadReplace(loadReplace); - loadState->SetTriggeringPrincipal(triggeringPrincipal); - loadState->SetPrincipalToInherit(principalToInherit); - loadState->SetLoadFlags(flags); - loadState->SetTypeHint(contentType); - loadState->SetPostDataStream(postData); - loadState->SetLoadType(aLoadType); - loadState->SetSHEntry(aEntry); - loadState->SetFirstParty(true); - loadState->SetSrcdocData(srcdoc); - loadState->SetBaseURI(baseURI); - loadState->SetCsp(csp); - rv = InternalLoad(loadState, nullptr, // No nsIDocShell nullptr); // No nsIRequest diff --git a/docshell/base/nsDocShellLoadState.cpp b/docshell/base/nsDocShellLoadState.cpp index ba52d2ebf81b..c9f68d41160f 100644 --- a/docshell/base/nsDocShellLoadState.cpp +++ b/docshell/base/nsDocShellLoadState.cpp @@ -52,7 +52,6 @@ nsDocShellLoadState::nsDocShellLoadState( mOriginalFrameSrc = aLoadState.OriginalFrameSrc(); mIsFormSubmission = aLoadState.IsFormSubmission(); mLoadType = aLoadState.LoadType(); - mSrcdocData.SetIsVoid(true); mTarget = aLoadState.Target(); mLoadFlags = aLoadState.LoadFlags(); mFirstParty = aLoadState.FirstParty(); @@ -66,11 +65,14 @@ nsDocShellLoadState::nsDocShellLoadState( mBaseURI = aLoadState.BaseURI(); mTriggeringPrincipal = aLoadState.TriggeringPrincipal(); mPrincipalToInherit = aLoadState.PrincipalToInherit(); + mStoragePrincipalToInherit = aLoadState.StoragePrincipalToInherit(); mCsp = aLoadState.Csp(); mOriginalURIString = aLoadState.OriginalURIString(); mCancelContentJSEpoch = aLoadState.CancelContentJSEpoch(); mPostDataStream = aLoadState.PostDataStream(); mHeadersStream = aLoadState.HeadersStream(); + mSrcdocData = aLoadState.SrcdocData(); + mResultPrincipalURI = aLoadState.ResultPrincipalURI(); } nsDocShellLoadState::~nsDocShellLoadState() {} @@ -323,6 +325,15 @@ void nsDocShellLoadState::SetPrincipalToInherit( mPrincipalToInherit = aPrincipalToInherit; } +nsIPrincipal* nsDocShellLoadState::StoragePrincipalToInherit() const { + return mStoragePrincipalToInherit; +} + +void nsDocShellLoadState::SetStoragePrincipalToInherit( + nsIPrincipal* aStoragePrincipalToInherit) { + mStoragePrincipalToInherit = aStoragePrincipalToInherit; +} + void nsDocShellLoadState::SetCsp(nsIContentSecurityPolicy* aCsp) { mCsp = aCsp; } @@ -629,12 +640,15 @@ DocShellLoadStateInit nsDocShellLoadState::Serialize() { loadState.BaseURI() = mBaseURI; loadState.TriggeringPrincipal() = mTriggeringPrincipal; loadState.PrincipalToInherit() = mPrincipalToInherit; + loadState.StoragePrincipalToInherit() = mStoragePrincipalToInherit; loadState.Csp() = mCsp; loadState.OriginalURIString() = mOriginalURIString; loadState.CancelContentJSEpoch() = mCancelContentJSEpoch; loadState.ReferrerInfo() = mReferrerInfo; loadState.PostDataStream() = mPostDataStream; loadState.HeadersStream() = mHeadersStream; + loadState.SrcdocData() = mSrcdocData; + loadState.ResultPrincipalURI() = mResultPrincipalURI; return loadState; } diff --git a/docshell/base/nsDocShellLoadState.h b/docshell/base/nsDocShellLoadState.h index b5a60b3108aa..159c62980c7a 100644 --- a/docshell/base/nsDocShellLoadState.h +++ b/docshell/base/nsDocShellLoadState.h @@ -76,6 +76,10 @@ class nsDocShellLoadState final { void SetPrincipalToInherit(nsIPrincipal* aPrincipalToInherit); + nsIPrincipal* StoragePrincipalToInherit() const; + + void SetStoragePrincipalToInherit(nsIPrincipal* aStoragePrincipalToInherit); + bool LoadReplace() const; void SetLoadReplace(bool aLoadReplace); @@ -293,6 +297,8 @@ class nsDocShellLoadState final { nsCOMPtr mPrincipalToInherit; + nsCOMPtr mStoragePrincipalToInherit; + // If this attribute is true, then a top-level navigation // to a data URI will be allowed. bool mForceAllowDataURI; diff --git a/docshell/shistory/PSHEntry.ipdl b/docshell/shistory/PSHEntry.ipdl index 22fefccaa97e..e1b99b063b78 100644 --- a/docshell/shistory/PSHEntry.ipdl +++ b/docshell/shistory/PSHEntry.ipdl @@ -8,8 +8,7 @@ include protocol PSHistory; include DOMTypes; include NewPSHEntry; -using refcounted class nsIInputStream from "mozilla/ipc/IPCStreamUtils.h"; - +using refcounted class nsDocShellLoadState from "mozilla/dom/DocShellMessageUtils.h"; using struct nsID from "nsID.h"; using nsIntRect from "nsRect.h"; @@ -104,6 +103,7 @@ parent: sync GetChildAt(int32_t index) returns (MaybeNewPSHEntry childEntry); sync ReplaceChild(PSHEntry newChildEntry) returns (nsresult result); sync ClearEntry(uint64_t aNewSharedID); + sync CreateLoadInfo() returns (nsDocShellLoadState loadState); sync __delete__(); }; diff --git a/docshell/shistory/SHEntryChild.cpp b/docshell/shistory/SHEntryChild.cpp index e14e84433277..e4e69d065ba5 100644 --- a/docshell/shistory/SHEntryChild.cpp +++ b/docshell/shistory/SHEntryChild.cpp @@ -950,6 +950,19 @@ SHEntryChild::SetPersist(bool aPersist) { return SendSetPersist(aPersist) ? NS_OK : NS_ERROR_FAILURE; } +NS_IMETHODIMP +SHEntryChild::CreateLoadInfo(nsDocShellLoadState** aLoadState) { + *aLoadState = nullptr; + RefPtr loadState; + if (!SendCreateLoadInfo(&loadState)) { + return NS_ERROR_FAILURE; + } + // Avoid dealing with serializing the PSHEntry by setting it here + loadState->SetSHEntry(this); + loadState.forget(aLoadState); + return NS_OK; +} + void SHEntryChild::EvictContentViewer() { nsCOMPtr viewer = GetContentViewer(); if (viewer) { diff --git a/docshell/shistory/SHEntryParent.cpp b/docshell/shistory/SHEntryParent.cpp index 85c111d0ea82..8d48eeedc0bd 100644 --- a/docshell/shistory/SHEntryParent.cpp +++ b/docshell/shistory/SHEntryParent.cpp @@ -538,5 +538,11 @@ bool SHEntryParent::RecvClearEntry(const uint64_t& aNewSharedID) { return true; } +bool SHEntryParent::RecvCreateLoadInfo( + RefPtr* aLoadState) { + mEntry->CreateLoadInfo(getter_AddRefs(*aLoadState)); + return true; +} + } // namespace dom } // namespace mozilla diff --git a/docshell/shistory/SHEntryParent.h b/docshell/shistory/SHEntryParent.h index 658cf709f9d6..ee91026c1269 100644 --- a/docshell/shistory/SHEntryParent.h +++ b/docshell/shistory/SHEntryParent.h @@ -183,6 +183,7 @@ class SHEntryParent final : public PSHEntryParent { void GetOrCreate(nsISHEntry* aSHEntry, MaybeNewPSHEntry* aResult) { GetOrCreate(Manager(), aSHEntry, *aResult); } + bool RecvCreateLoadInfo(RefPtr* aLoadState); RefPtr mEntry; }; diff --git a/docshell/shistory/nsISHEntry.idl b/docshell/shistory/nsISHEntry.idl index e43027df58d2..44e061601f99 100644 --- a/docshell/shistory/nsISHEntry.idl +++ b/docshell/shistory/nsISHEntry.idl @@ -35,9 +35,12 @@ class SHEntrySharedParentState; } } +class nsSHEntryShared; +class nsDocShellLoadState; %} [ref] native nsIntRect(nsIntRect); [ptr] native nsDocShellEditorDataPtr(nsDocShellEditorData); +[ptr] native nsDocShellLoadStatePtr(nsDocShellLoadState); [builtinclass, scriptable, uuid(0dad26b8-a259-42c7-93f1-2fa7fc076e45)] interface nsISHEntry : nsISupports @@ -407,5 +410,11 @@ interface nsISHEntry : nsISupports * Remove all children of this entry and call abandonBFCacheEntry. */ [notxpcom] void ClearEntry(); + + /** + * Create nsDocShellLoadState and fill it with information. + * Don't set nsSHEntry here to avoid serializing it. + */ + [noscript] nsDocShellLoadStatePtr CreateLoadInfo(); }; diff --git a/docshell/shistory/nsSHEntry.cpp b/docshell/shistory/nsSHEntry.cpp index ead26f4f2019..f195c5cb6a3d 100644 --- a/docshell/shistory/nsSHEntry.cpp +++ b/docshell/shistory/nsSHEntry.cpp @@ -925,6 +925,79 @@ nsSHEntry::SetPersist(bool aPersist) { return NS_OK; } +NS_IMETHODIMP +nsSHEntry::CreateLoadInfo(nsDocShellLoadState** aLoadState) { + nsCOMPtr uri = GetURI(); + RefPtr loadState(new nsDocShellLoadState(uri)); + + nsCOMPtr originalURI = GetOriginalURI(); + loadState->SetOriginalURI(originalURI); + + mozilla::Maybe> emplacedResultPrincipalURI; + nsCOMPtr resultPrincipalURI = GetResultPrincipalURI(); + emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI)); + loadState->SetMaybeResultPrincipalURI(emplacedResultPrincipalURI); + + loadState->SetLoadReplace(GetLoadReplace()); + nsCOMPtr postData = GetPostData(); + loadState->SetPostDataStream(postData); + + nsAutoCString contentType; + GetContentType(contentType); + loadState->SetTypeHint(contentType); + + nsCOMPtr triggeringPrincipal = GetTriggeringPrincipal(); + loadState->SetTriggeringPrincipal(triggeringPrincipal); + nsCOMPtr principalToInherit = GetPrincipalToInherit(); + loadState->SetPrincipalToInherit(principalToInherit); + nsCOMPtr storagePrincipalToInherit = + GetStoragePrincipalToInherit(); + loadState->SetStoragePrincipalToInherit(storagePrincipalToInherit); + nsCOMPtr csp = GetCsp(); + loadState->SetCsp(csp); + nsCOMPtr referrerInfo = GetReferrerInfo(); + loadState->SetReferrerInfo(referrerInfo); + + // Do not inherit principal from document (security-critical!); + uint32_t flags = nsDocShell::InternalLoad::INTERNAL_LOAD_FLAGS_NONE; + + // Passing nullptr as aSourceDocShell gives the same behaviour as before + // aSourceDocShell was introduced. According to spec we should be passing + // the source browsing context that was used when the history entry was + // first created. bug 947716 has been created to address this issue. + nsAutoString srcdoc; + nsCOMPtr baseURI; + if (GetIsSrcdocEntry()) { + GetSrcdocData(srcdoc); + baseURI = GetBaseURI(); + flags |= nsDocShell::InternalLoad::INTERNAL_LOAD_FLAGS_IS_SRCDOC; + } else { + srcdoc = VoidString(); + } + loadState->SetSrcdocData(srcdoc); + loadState->SetBaseURI(baseURI); + loadState->SetLoadFlags(flags); + + loadState->SetFirstParty(true); + + loadState.forget(aLoadState); + return NS_OK; +} + +NS_IMETHODIMP +nsLegacySHEntry::CreateLoadInfo(nsDocShellLoadState** aLoadState) { + RefPtr loadState; + nsSHEntry::CreateLoadInfo(getter_AddRefs(loadState)); + // If CreateLoadInfo is getting called from a parent process, + // then the call will never go through SHEntryChild::CreateLoadInfo + // and then the nsSHEntry will never be set on load state. + // This is why we have to override this method here + // and set the nsSHEntry. + loadState->SetSHEntry(this); + loadState.forget(aLoadState); + return NS_OK; +} + void nsSHEntry::EvictContentViewer() { nsCOMPtr viewer = GetContentViewer(); if (viewer) { diff --git a/docshell/shistory/nsSHEntry.h b/docshell/shistory/nsSHEntry.h index 7dcfddcb4803..8d63a21987ca 100644 --- a/docshell/shistory/nsSHEntry.h +++ b/docshell/shistory/nsSHEntry.h @@ -118,6 +118,7 @@ class nsLegacySHEntry final : public nsSHEntry { NS_IMETHOD_(bool) HasDetachedEditor() override; NS_IMETHOD_(bool) HasBFCacheEntry(nsIBFCacheEntry* aEntry) override; NS_IMETHOD AbandonBFCacheEntry() override; + NS_IMETHOD CreateLoadInfo(nsDocShellLoadState** aLoadState) override; private: nsSHEntryShared* GetState(); diff --git a/dom/ipc/DOMTypes.ipdlh b/dom/ipc/DOMTypes.ipdlh index 738dbb955c72..f8da4ad161f6 100644 --- a/dom/ipc/DOMTypes.ipdlh +++ b/dom/ipc/DOMTypes.ipdlh @@ -11,6 +11,7 @@ include "mozilla/layers/LayersMessageUtils.h"; include IPCBlob; include IPCStream; include ProtocolTypes; +include protocol PSHEntry; using struct mozilla::void_t from "ipc/IPCMessageUtils.h"; @@ -34,6 +35,7 @@ using refcounted class nsIContentSecurityPolicy from "mozilla/dom/CSPMessageUtil using refcounted class nsIInputStream from "mozilla/ipc/IPCStreamUtils.h"; using refcounted class nsIReferrerInfo from "mozilla/dom/ReferrerInfoUtils.h"; using class mozilla::TimeStamp from "mozilla/TimeStamp.h"; +using refcounted class nsISHEntry from "nsISHEntry.h"; namespace mozilla { namespace dom { @@ -222,6 +224,7 @@ struct DocShellLoadStateInit bool InheritPrincipal; bool PrincipalIsExplicit; nsIPrincipal PrincipalToInherit; + nsIPrincipal StoragePrincipalToInherit; bool ForceAllowDataURI; bool OriginalFrameSrc; bool IsFormSubmission; @@ -247,11 +250,12 @@ struct DocShellLoadStateInit nsIInputStream PostDataStream; nsIInputStream HeadersStream; + nsString SrcdocData; // useless without sourcedocshell + // Fields missing due to lack of need or serialization // nsCOMPtr mSHEntry; // nsCOMPtr mSourceDocShell; // bool mIsSrcDocLoad; // useless without sourcedocshell - // nsString mSrcdocData; // useless without sourcedocshell // nsIChannel pendingRedirectedChannel; // sent through other mechanism }; diff --git a/ipc/ipdl/sync-messages.ini b/ipc/ipdl/sync-messages.ini index 79ebfa2fe934..3e17383a85e6 100644 --- a/ipc/ipdl/sync-messages.ini +++ b/ipc/ipdl/sync-messages.ini @@ -1004,6 +1004,8 @@ description = Standing up Fission description = Standing up Fission [PSHEntry::__delete__] description = Standing up Fission +[PSHEntry::CreateLoadInfo] +description = Standing up Fission # The rest [PHeapSnapshotTempFileHelper::OpenHeapSnapshotTempFile]