Bug 1931418 - Reduce the amount of recursion checks. r=dom-core,sefeng

Modify the frame recursion check to only happen when a load is triggered
from a src or data attribute (depending on if it's an iframe, embed or
an object element). This prevents recursion checks from blocking
ordinary navigations and navigations with a target.

Differential Revision: https://phabricator.services.mozilla.com/D229503

docshell/base/nsDocShellLoadState.cpp

@@ -66,6 +66,7 @@ nsDocShellLoadState::nsDocShellLoadState(
   mForceAllowDataURI = aLoadState.ForceAllowDataURI();
   mIsExemptFromHTTPSFirstMode = aLoadState.IsExemptFromHTTPSFirstMode();
   mOriginalFrameSrc = aLoadState.OriginalFrameSrc();
+  mShouldCheckForRecursion = aLoadState.ShouldCheckForRecursion();
   mIsFormSubmission = aLoadState.IsFormSubmission();
   mLoadType = aLoadState.LoadType();
   mTarget = aLoadState.Target();
@@ -171,6 +172,7 @@ nsDocShellLoadState::nsDocShellLoadState(const nsDocShellLoadState& aOther)
       mIsExemptFromHTTPSFirstMode(aOther.mIsExemptFromHTTPSFirstMode),
       mHttpsFirstDowngradeData(aOther.GetHttpsFirstDowngradeData()),
       mOriginalFrameSrc(aOther.mOriginalFrameSrc),
+      mShouldCheckForRecursion(aOther.mShouldCheckForRecursion),
       mIsFormSubmission(aOther.mIsFormSubmission),
       mLoadType(aOther.mLoadType),
       mSHEntry(aOther.mSHEntry),
@@ -227,6 +229,7 @@ nsDocShellLoadState::nsDocShellLoadState(nsIURI* aURI, uint64_t aLoadIdentifier)
       mForceAllowDataURI(false),
       mIsExemptFromHTTPSFirstMode(false),
       mOriginalFrameSrc(false),
+      mShouldCheckForRecursion(false),
       mIsFormSubmission(false),
       mLoadType(LOAD_NORMAL),
       mSrcdocData(VoidString()),
@@ -671,6 +674,15 @@ void nsDocShellLoadState::SetOriginalFrameSrc(bool aOriginalFrameSrc) {
   mOriginalFrameSrc = aOriginalFrameSrc;
 }
 
+bool nsDocShellLoadState::ShouldCheckForRecursion() const {
+  return mShouldCheckForRecursion;
+}
+
+void nsDocShellLoadState::SetShouldCheckForRecursion(
+    bool aShouldCheckForRecursion) {
+  mShouldCheckForRecursion = aShouldCheckForRecursion;
+}
+
 bool nsDocShellLoadState::IsFormSubmission() const { return mIsFormSubmission; }
 
 void nsDocShellLoadState::SetIsFormSubmission(bool aIsFormSubmission) {
@@ -1311,6 +1323,7 @@ DocShellLoadStateInit nsDocShellLoadState::Serialize(
   loadState.ForceAllowDataURI() = mForceAllowDataURI;
   loadState.IsExemptFromHTTPSFirstMode() = mIsExemptFromHTTPSFirstMode;
   loadState.OriginalFrameSrc() = mOriginalFrameSrc;
+  loadState.ShouldCheckForRecursion() = mShouldCheckForRecursion;
   loadState.IsFormSubmission() = mIsFormSubmission;
   loadState.LoadType() = mLoadType;
   loadState.Target() = mTarget;

docshell/base/nsDocShellLoadState.h

@@ -160,6 +160,10 @@ class nsDocShellLoadState final {
 
   void SetOriginalFrameSrc(bool aOriginalFrameSrc);
 
+  bool ShouldCheckForRecursion() const;
+
+  void SetShouldCheckForRecursion(bool aShouldCheckForRecursion);
+
   bool IsFormSubmission() const;
 
   void SetIsFormSubmission(bool aIsFormSubmission);
@@ -514,6 +518,12 @@ class nsDocShellLoadState final {
   // element loading its original src (or srcdoc) attribute.
   bool mOriginalFrameSrc;
 
+  // If this attribute is true, this load corresponds to a frame, object, or
+  // embed element that needs a recursion check when loading it's src (or data).
+  // Unlike mOriginalFrameSrc, this attribute will always be set regardless
+  // whether we've loaded the src already.
+  bool mShouldCheckForRecursion;
+
   // If this attribute is true, then the load was initiated by a
   // form submission.
   bool mIsFormSubmission;

dom/base/nsFrameLoader.cpp

@@ -195,6 +195,7 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext,
       mHideCalled(false),
       mNetworkCreated(aNetworkCreated),
       mLoadingOriginalSrc(false),
+      mShouldCheckForRecursion(false),
       mRemoteBrowserShown(false),
       mRemoteBrowserSized(false),
       mIsRemoteFrame(aIsRemoteFrame),
@@ -498,7 +499,8 @@ already_AddRefed<nsFrameLoader> nsFrameLoader::Recreate(
   return fl.forget();
 }
 
-void nsFrameLoader::LoadFrame(bool aOriginalSrc) {
+void nsFrameLoader::LoadFrame(bool aOriginalSrc,
+                              bool aShouldCheckForRecursion) {
   if (NS_WARN_IF(!mOwnerContent)) {
     return;
   }
@@ -554,7 +556,7 @@ void nsFrameLoader::LoadFrame(bool aOriginalSrc) {
   }
 
   if (NS_SUCCEEDED(rv)) {
-    rv = LoadURI(uri, principal, csp, aOriginalSrc);
+    rv = LoadURI(uri, principal, csp, aOriginalSrc, aShouldCheckForRecursion);
   }
 
   if (NS_FAILED(rv)) {
@@ -586,7 +588,8 @@ void nsFrameLoader::FireErrorEvent() {
 nsresult nsFrameLoader::LoadURI(nsIURI* aURI,
                                 nsIPrincipal* aTriggeringPrincipal,
                                 nsIContentSecurityPolicy* aCsp,
-                                bool aOriginalSrc) {
+                                bool aOriginalSrc,
+                                bool aShouldCheckForRecursion) {
   if (!aURI) return NS_ERROR_INVALID_POINTER;
   NS_ENSURE_STATE(!mDestroyCalled && mOwnerContent);
   MOZ_ASSERT(
@@ -594,6 +597,7 @@ nsresult nsFrameLoader::LoadURI(nsIURI* aURI,
       "Must have an explicit triggeringPrincipal to nsFrameLoader::LoadURI.");
 
   mLoadingOriginalSrc = aOriginalSrc;
+  mShouldCheckForRecursion = aShouldCheckForRecursion;
 
   nsCOMPtr<Document> doc = mOwnerContent->OwnerDoc();
 
@@ -626,6 +630,7 @@ void nsFrameLoader::ResumeLoad(uint64_t aPendingSwitchID) {
   }
 
   mLoadingOriginalSrc = false;
+  mShouldCheckForRecursion = false;
   mURIToLoad = nullptr;
   mPendingSwitchID = aPendingSwitchID;
   mTriggeringPrincipal = mOwnerContent->NodePrincipal();
@@ -659,6 +664,7 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() {
   if (!mPendingSwitchID) {
     loadState = new nsDocShellLoadState(mURIToLoad);
     loadState->SetOriginalFrameSrc(mLoadingOriginalSrc);
+    loadState->SetShouldCheckForRecursion(mShouldCheckForRecursion);
 
     // The triggering principal could be null if the frame is loaded other
     // than the src attribute, for example, the frame is sandboxed. In that
@@ -764,6 +770,7 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() {
   NS_ENSURE_SUCCESS(rv, rv);
 
   mLoadingOriginalSrc = false;
+  mShouldCheckForRecursion = false;
 
   // Kick off the load...
   bool tmpState = mNeedsAsyncDestroy;

dom/base/nsFrameLoader.h

@@ -191,7 +191,7 @@ class nsFrameLoader final : public nsStubMutationObserver,
    * Start loading the frame. This method figures out what to load
    * from the owner content in the frame loader.
    */
-  void LoadFrame(bool aOriginalSrc);
+  void LoadFrame(bool aOriginalSrc, bool aShouldCheckForRecursion);
 
   /**
    * Loads the specified URI in this frame. Behaves identically to loadFrame,
@@ -207,7 +207,8 @@ class nsFrameLoader final : public nsStubMutationObserver,
    *        frame load is upgraded from http to https.
    */
   nsresult LoadURI(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal,
-                   nsIContentSecurityPolicy* aCsp, bool aOriginalSrc);
+                   nsIContentSecurityPolicy* aCsp, bool aOriginalSrc,
+                   bool aShouldCheckForRecursion);
 
   /**
    * Resume a redirected load within this frame.
@@ -549,6 +550,9 @@ class nsFrameLoader final : public nsStubMutationObserver,
   // attribute of the frame element.
   bool mLoadingOriginalSrc : 1;
 
+  // True if a pending load corresponds to the src attribute being changed.
+  bool mShouldCheckForRecursion : 1;
+
   bool mRemoteBrowserShown : 1;
   bool mRemoteBrowserSized : 1;
   bool mIsRemoteFrame : 1;

dom/base/nsFrameLoaderOwner.cpp

@@ -257,7 +257,8 @@ void nsFrameLoaderOwner::ChangeRemoteness(
     if (aOptions.mPendingSwitchID.WasPassed()) {
       mFrameLoader->ResumeLoad(aOptions.mPendingSwitchID.Value());
     } else {
-      mFrameLoader->LoadFrame(false);
+      mFrameLoader->LoadFrame(/* aOriginalSrc */ false,
+                              /* aShouldCheckForRecursion */ false);
     }
   };
 

dom/base/nsObjectLoadingContent.cpp

@@ -1464,6 +1464,8 @@ nsresult nsObjectLoadingContent::OpenChannel() {
     auto referrerInfo = MakeRefPtr<ReferrerInfo>(*doc);
     loadState->SetReferrerInfo(referrerInfo);
 
+    loadState->SetShouldCheckForRecursion(true);
+
     chan =
         DocumentChannel::CreateForObject(loadState, loadInfo, loadFlags, shim);
     MOZ_ASSERT(chan);

dom/html/nsGenericHTMLFrameElement.cpp

@@ -174,7 +174,7 @@ void nsGenericHTMLFrameElement::LoadSrc() {
 
   bool origSrc = !mSrcLoadHappened;
   mSrcLoadHappened = true;
-  mFrameLoader->LoadFrame(origSrc);
+  mFrameLoader->LoadFrame(origSrc, /* aShouldCheckForRecursion */ true);
 }
 
 nsresult nsGenericHTMLFrameElement::BindToTree(BindContext& aContext,

dom/ipc/DOMTypes.ipdlh

@@ -210,6 +210,7 @@ struct DocShellLoadStateInit
   bool ForceAllowDataURI;
   bool IsExemptFromHTTPSFirstMode;
   bool OriginalFrameSrc;
+  bool ShouldCheckForRecursion;
   bool IsFormSubmission;
   bool FirstParty;
   bool HasValidUserGestureActivation;

dom/xul/XULFrameElement.cpp

@@ -110,7 +110,8 @@ void XULFrameElement::LoadSrc() {
         *this, u"XULFrameLoaderCreated"_ns, CanBubble::eYes);
   }
 
-  mFrameLoader->LoadFrame(false);
+  mFrameLoader->LoadFrame(/* aOriginalSrc */ false,
+                          /* aShouldCheckForRecursion */ false);
 }
 
 void XULFrameElement::SwapFrameLoaders(HTMLIFrameElement& aOtherLoaderOwner,

netwerk/ipc/DocumentLoadListener.cpp

@@ -521,6 +521,10 @@ WindowGlobalParent* DocumentLoadListener::GetParentWindowContext() const {
 
 bool CheckRecursiveLoad(CanonicalBrowsingContext* aLoadingContext,
                         nsDocShellLoadState* aLoadState, bool aIsDocumentLoad) {
+  if (!aLoadState->ShouldCheckForRecursion()) {
+    return true;
+  }
+
   // Bug 136580: Check for recursive frame loading excluding about:srcdoc URIs.
   // srcdoc URIs require their contents to be specified inline, so it isn't
   // possible for undesirable recursion to occur without the aid of a