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

@@ -11593,28 +11593,21 @@ nsresult nsDocShell::LoadHistoryEntry(nsISHEntry* aEntry, uint32_t aLoadType) {
   }
 
   NS_ENSURE_TRUE(aEntry, NS_ERROR_FAILURE);
-
-  nsCOMPtr<nsIURI> uri = aEntry->GetURI();
-  nsCOMPtr<nsIURI> originalURI = aEntry->GetOriginalURI();
-  nsCOMPtr<nsIURI> resultPrincipalURI = aEntry->GetResultPrincipalURI();
-  bool loadReplace = aEntry->GetLoadReplace();
-  nsCOMPtr<nsIInputStream> postData = aEntry->GetPostData();
-  nsAutoCString contentType;
-  aEntry->GetContentType(contentType);
-  nsCOMPtr<nsIPrincipal> triggeringPrincipal = aEntry->GetTriggeringPrincipal();
-  nsCOMPtr<nsIPrincipal> principalToInherit = aEntry->GetPrincipalToInherit();
-  nsCOMPtr<nsIPrincipal> storagePrincipalToInherit =
-      aEntry->GetStoragePrincipalToInherit();
-  nsCOMPtr<nsIContentSecurityPolicy> csp = aEntry->GetCsp();
-  nsCOMPtr<nsIReferrerInfo> referrerInfo = aEntry->GetReferrerInfo();
+  nsresult rv;
+  RefPtr<nsDocShellLoadState> 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<nsISHEntry> 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<nsIPrincipal> 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<nsIURI> 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<nsCOMPtr<nsIURI>> emplacedResultPrincipalURI;
-  emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI));
-
-  RefPtr<nsDocShellLoadState> 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

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;
 }

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<nsIPrincipal> mPrincipalToInherit;
 
+  nsCOMPtr<nsIPrincipal> mStoragePrincipalToInherit;
+
   // If this attribute is true, then a top-level navigation
   // to a data URI will be allowed.
   bool mForceAllowDataURI;

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__();
 };

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<nsDocShellLoadState> 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<nsIContentViewer> viewer = GetContentViewer();
   if (viewer) {

docshell/shistory/SHEntryParent.cpp

@@ -538,5 +538,11 @@ bool SHEntryParent::RecvClearEntry(const uint64_t& aNewSharedID) {
   return true;
 }
 
+bool SHEntryParent::RecvCreateLoadInfo(
+    RefPtr<nsDocShellLoadState>* aLoadState) {
+  mEntry->CreateLoadInfo(getter_AddRefs(*aLoadState));
+  return true;
+}
+
 }  // namespace dom
 }  // namespace mozilla

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<nsDocShellLoadState>* aLoadState);
 
   RefPtr<LegacySHEntry> mEntry;
 };

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();
 };
 

docshell/shistory/nsSHEntry.cpp

@@ -925,6 +925,79 @@ nsSHEntry::SetPersist(bool aPersist) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsSHEntry::CreateLoadInfo(nsDocShellLoadState** aLoadState) {
+  nsCOMPtr<nsIURI> uri = GetURI();
+  RefPtr<nsDocShellLoadState> loadState(new nsDocShellLoadState(uri));
+
+  nsCOMPtr<nsIURI> originalURI = GetOriginalURI();
+  loadState->SetOriginalURI(originalURI);
+
+  mozilla::Maybe<nsCOMPtr<nsIURI>> emplacedResultPrincipalURI;
+  nsCOMPtr<nsIURI> resultPrincipalURI = GetResultPrincipalURI();
+  emplacedResultPrincipalURI.emplace(std::move(resultPrincipalURI));
+  loadState->SetMaybeResultPrincipalURI(emplacedResultPrincipalURI);
+
+  loadState->SetLoadReplace(GetLoadReplace());
+  nsCOMPtr<nsIInputStream> postData = GetPostData();
+  loadState->SetPostDataStream(postData);
+
+  nsAutoCString contentType;
+  GetContentType(contentType);
+  loadState->SetTypeHint(contentType);
+
+  nsCOMPtr<nsIPrincipal> triggeringPrincipal = GetTriggeringPrincipal();
+  loadState->SetTriggeringPrincipal(triggeringPrincipal);
+  nsCOMPtr<nsIPrincipal> principalToInherit = GetPrincipalToInherit();
+  loadState->SetPrincipalToInherit(principalToInherit);
+  nsCOMPtr<nsIPrincipal> storagePrincipalToInherit =
+      GetStoragePrincipalToInherit();
+  loadState->SetStoragePrincipalToInherit(storagePrincipalToInherit);
+  nsCOMPtr<nsIContentSecurityPolicy> csp = GetCsp();
+  loadState->SetCsp(csp);
+  nsCOMPtr<nsIReferrerInfo> 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<nsIURI> 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<nsDocShellLoadState> 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<nsIContentViewer> viewer = GetContentViewer();
   if (viewer) {

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();

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<nsISHEntry> mSHEntry;
   // nsCOMPtr<nsIDocShell> mSourceDocShell;
   // bool mIsSrcDocLoad; // useless without sourcedocshell
-  // nsString mSrcdocData; // useless without sourcedocshell
   // nsIChannel pendingRedirectedChannel; // sent through other mechanism
 };
 

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]