Bug 1559841. Make the 'load' event wait for OOP-iframes to load. r=kmag

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

--HG--
extra : moz-landing-system : lando

docshell/base/nsDocShell.cpp

@@ -2130,6 +2130,16 @@ nsDocShell::HistoryPurged(int32_t aNumEntries) {
   return NS_OK;
 }
 
+void nsDocShell::TriggerParentCheckDocShellIsEmpty() {
+  if (RefPtr<nsDocShell> parent = GetInProcessParentDocshell()) {
+    parent->DocLoaderIsEmpty(true);
+  } else if (BrowserChild* browserChild = BrowserChild::GetFrom(this)) {
+    // OOP parent
+    mozilla::Unused << browserChild->SendMaybeFireEmbedderLoadEvents(
+        /*aIsTrusted*/ true, /*aFireLoadAtEmbeddingElement*/ false);
+  }
+}
+
 nsresult nsDocShell::HistoryEntryRemoved(int32_t aIndex) {
   // These indices are used for fastback cache eviction, to determine
   // which session history entries are candidates for content viewer
@@ -3949,6 +3959,16 @@ NS_IMETHODIMP
 nsDocShell::DisplayLoadError(nsresult aError, nsIURI* aURI,
                              const char16_t* aURL, nsIChannel* aFailedChannel,
                              bool* aDisplayedErrorPage) {
+  // If we have a cross-process parent document, we must notify it that we no
+  // longer block its load event.  This is necessary for OOP sub-documents
+  // because error documents do not result in a call to
+  // SendMaybeFireEmbedderLoadEvents via any of the normal call paths.
+  // (Obviously, we must do this before any of the returns below.)
+  if (BrowserChild* browserChild = BrowserChild::GetFrom(this)) {
+    mozilla::Unused << browserChild->SendMaybeFireEmbedderLoadEvents(
+        /*aIsTrusted*/ true, /*aFireLoadAtEmbeddingElement*/ false);
+  }
+
   *aDisplayedErrorPage = false;
   // Get prompt and string bundle services
   nsCOMPtr<nsIPrompt> prompter;

docshell/base/nsDocShell.h

@@ -331,12 +331,7 @@ class nsDocShell final : public nsDocLoader,
   // set. As soon as the current DocShell knows itself can be treated as
   // background loading, it triggers the parent docshell to see if the parent
   // document can fire load event earlier.
-  void TriggerParentCheckDocShellIsEmpty() {
-    RefPtr<nsDocShell> parent = GetInProcessParentDocshell();
-    if (parent) {
-      parent->DocLoaderIsEmpty(true);
-    }
-  }
+  void TriggerParentCheckDocShellIsEmpty();
 
   nsresult HistoryEntryRemoved(int32_t aIndex);
 

dom/base/nsGlobalWindowInner.cpp

@@ -1951,7 +1951,8 @@ void nsGlobalWindowInner::FireFrameLoadEvent(bool aIsTrusted) {
       return;
     }
 
-    mozilla::Unused << browserChild->SendFireFrameLoadEvent(aIsTrusted);
+    mozilla::Unused << browserChild->SendMaybeFireEmbedderLoadEvents(
+        aIsTrusted, /*aFireLoadAtEmbeddingElement*/ true);
   }
 }
 

dom/base/test/mochitest.ini

@@ -862,6 +862,7 @@ tags = audiochannel
 [test_window_constructor.html]
 [test_window_content.html]
 [test_window_cross_origin_props.html]
+skip-if = fission && debug # bug 1580618
 fail-if = fission
 [test_window_define_nonconfigurable.html]
 [test_window_define_symbol.html]

dom/ipc/BrowserBridgeChild.cpp

@@ -145,19 +145,22 @@ BrowserBridgeChild::RecvSetEmbeddedDocAccessibleCOMProxy(
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult BrowserBridgeChild::RecvFireFrameLoadEvent(
-    bool aIsTrusted) {
+mozilla::ipc::IPCResult BrowserBridgeChild::RecvMaybeFireEmbedderLoadEvents(
+    bool aIsTrusted, bool aFireLoadAtEmbeddingElement) {
   RefPtr<Element> owner = mFrameLoader->GetOwnerContent();
   if (!owner) {
     return IPC_OK();
   }
 
-  // Fire the `load` event on our embedder element.
-  nsEventStatus status = nsEventStatus_eIgnore;
-  WidgetEvent event(aIsTrusted, eLoad);
-  event.mFlags.mBubbles = false;
-  event.mFlags.mCancelable = false;
-  EventDispatcher::Dispatch(owner, nullptr, &event, nullptr, &status);
+  if (aFireLoadAtEmbeddingElement) {
+    nsEventStatus status = nsEventStatus_eIgnore;
+    WidgetEvent event(aIsTrusted, eLoad);
+    event.mFlags.mBubbles = false;
+    event.mFlags.mCancelable = false;
+    EventDispatcher::Dispatch(owner, nullptr, &event, nullptr, &status);
+  }
+
+  UnblockOwnerDocsLoadEvent();
 
   return IPC_OK();
 }
@@ -214,6 +217,20 @@ mozilla::ipc::IPCResult BrowserBridgeChild::RecvSubFrameCrashed(
 
 void BrowserBridgeChild::ActorDestroy(ActorDestroyReason aWhy) {
   mIPCOpen = false;
+
+  // Ensure we unblock our document's 'load' event (in case the OOP-iframe has
+  // been removed before it finished loading, or its subprocess crashed):
+  UnblockOwnerDocsLoadEvent();
+}
+
+void BrowserBridgeChild::UnblockOwnerDocsLoadEvent() {
+  if (!mHadInitialLoad) {
+    mHadInitialLoad = true;
+    if (auto* docShell =
+            nsDocShell::Cast(mBrowsingContext->GetParent()->GetDocShell())) {
+      docShell->OOPChildLoadDone(this);
+    }
+  }
 }
 
 }  // namespace dom

dom/ipc/BrowserBridgeChild.h

@@ -83,7 +83,8 @@ class BrowserBridgeChild : public PBrowserBridgeChild {
   mozilla::ipc::IPCResult RecvSetEmbeddedDocAccessibleCOMProxy(
       const IDispatchHolder& aCOMProxy);
 
-  mozilla::ipc::IPCResult RecvFireFrameLoadEvent(bool aIsTrusted);
+  mozilla::ipc::IPCResult RecvMaybeFireEmbedderLoadEvents(
+      bool aIsTrusted, bool aFireLoadAtEmbeddingElement);
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
   mozilla::ipc::IPCResult RecvScrollRectIntoView(
@@ -98,9 +99,12 @@ class BrowserBridgeChild : public PBrowserBridgeChild {
  private:
   ~BrowserBridgeChild();
 
+  void UnblockOwnerDocsLoadEvent();
+
   TabId mId;
   LayersId mLayersId;
   bool mIPCOpen;
+  bool mHadInitialLoad = false;
   RefPtr<nsFrameLoader> mFrameLoader;
   RefPtr<BrowsingContext> mBrowsingContext;
 #if defined(ACCESSIBILITY) && defined(XP_WIN)

dom/ipc/BrowserParent.cpp

@@ -3874,14 +3874,16 @@ mozilla::ipc::IPCResult BrowserParent::RecvGetSystemFont(nsCString* aFontName) {
   return IPC_OK();
 }
 
-mozilla::ipc::IPCResult BrowserParent::RecvFireFrameLoadEvent(bool aIsTrusted) {
+mozilla::ipc::IPCResult BrowserParent::RecvMaybeFireEmbedderLoadEvents(
+    bool aIsTrusted, bool aFireLoadAtEmbeddingElement) {
   BrowserBridgeParent* bridge = GetBrowserBridgeParent();
   if (!bridge) {
     NS_WARNING("Received `load` event on unbridged BrowserParent!");
     return IPC_OK();
   }
 
-  Unused << bridge->SendFireFrameLoadEvent(aIsTrusted);
+  Unused << bridge->SendMaybeFireEmbedderLoadEvents(
+      aIsTrusted, aFireLoadAtEmbeddingElement);
   return IPC_OK();
 }
 

dom/ipc/BrowserParent.h

@@ -784,7 +784,8 @@ class BrowserParent final : public PBrowserParent,
 
   mozilla::ipc::IPCResult RecvQueryVisitedState(nsTArray<URIParams>&& aURIs);
 
-  mozilla::ipc::IPCResult RecvFireFrameLoadEvent(bool aIsTrusted);
+  mozilla::ipc::IPCResult RecvMaybeFireEmbedderLoadEvents(
+      bool aIsTrusted, bool aFireLoadAtEmbeddingElement);
 
  private:
   void SuppressDisplayport(bool aEnabled);

dom/ipc/PBrowser.ipdl

@@ -705,9 +705,11 @@ parent:
     sync ResetPrefersReducedMotionOverrideForTest();
 
     /**
-     * Fire a `load` event on this PBrowser's embedding frame element.
+     * Called once this PBrowser's OOP subdoc no longer blocks its
+     * embedding element's and embedding doc's 'load' events.
      */
-    async FireFrameLoadEvent(bool aIsTrusted);
+    async MaybeFireEmbedderLoadEvents(bool aIsTrusted,
+                                      bool aFireLoadAtEmbeddingElement);
 
     async ScrollRectIntoView(nsRect aRect, ScrollAxis aVertical,
                              ScrollAxis aHorizontal, ScrollFlags aScrollFlags,

dom/ipc/PBrowserBridge.ipdl

@@ -55,9 +55,11 @@ child:
   async SetEmbeddedDocAccessibleCOMProxy(IDispatchHolder aCOMProxy);
 
   /**
-   * Fire a `load` event on this PBrowserBridge's embedding frame element.
+   * Called once this PBrowserBridge's OOP subdoc no longer blocks its
+   * embedding element's and embedding doc's 'load' events.
    */
-  async FireFrameLoadEvent(bool aIsTrusted);
+  async MaybeFireEmbedderLoadEvents(bool aIsTrusted,
+                                    bool aFireLoadAtEmbeddingElement);
 
   async ScrollRectIntoView(nsRect aRect, ScrollAxis aVertical,
                            ScrollAxis aHorizontal, ScrollFlags aScrollFlags,

dom/tests/mochitest/general/mochitest.ini

@@ -138,6 +138,7 @@ skip-if = toolkit == 'android' # bug 1230232 - Mouse doesn't select in the same
 [test_storagePermissionsLimitForeign.html]
 [test_storagePermissionsReject.html]
 [test_storagePermissionsRejectForeign.html]
+skip-if = fission # intermittent since bug 1559841
 [test_stylesheetPI.html]
 [test_vibrator.html]
 [test_WebKitCSSMatrix.html]

dom/tests/mochitest/storageevent/mochitest.ini

@@ -14,4 +14,5 @@ support-files =
 [test_storageLocalStorageEventCheckPropagation.html]
 [test_storageNotifications.html]
 [test_storageSessionStorageEventCheckNoPropagation.html]
+skip-if = fission # intermittent since bug 1559841
 [test_storageSessionStorageEventCheckPropagation.html]

uriloader/base/nsDocLoader.cpp

@@ -4,6 +4,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nspr.h"
+#include "mozilla/dom/BrowserChild.h"
 #include "mozilla/dom/Document.h"
 #include "mozilla/BasicEvents.h"
 #include "mozilla/Components.h"
@@ -52,6 +53,7 @@ using mozilla::eLoad;
 using mozilla::EventDispatcher;
 using mozilla::LogLevel;
 using mozilla::WidgetEvent;
+using mozilla::dom::BrowserChild;
 using mozilla::dom::Document;
 
 //
@@ -243,6 +245,7 @@ nsDocLoader::Stop(void) {
   // after this, since mDocumentRequest will be null after the
   // DocLoaderIsEmpty() call.
   mChildrenInOnload.Clear();
+  mOOPChildrenLoading.Clear();
 
   // Make sure to call DocLoaderIsEmpty now so that we reset mDocumentRequest,
   // etc, as needed.  We could be getting into here from a subframe onload, in
@@ -277,7 +280,8 @@ bool nsDocLoader::IsBusy() {
   //   3. It's currently flushing layout in DocLoaderIsEmpty().
   //
 
-  if (mChildrenInOnload.Count() || mIsFlushingLayout) {
+  if (!mChildrenInOnload.IsEmpty() || !mOOPChildrenLoading.IsEmpty() ||
+      mIsFlushingLayout) {
     return true;
   }
 
@@ -286,6 +290,7 @@ bool nsDocLoader::IsBusy() {
     return false;
   }
 
+  // Check if any in-process sub-document is awaiting its 'load' event:
   bool busy;
   rv = mLoadGroup->IsPending(&busy);
   if (NS_FAILED(rv)) {
@@ -725,9 +730,7 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) {
         //
         doStopDocumentLoad(docRequest, loadGroupStatus);
 
-        if (parent) {
-          parent->ChildDoneWithOnload(this);
-        }
+        NotifyDoneWithOnload(parent);
       }
     } else {
       MOZ_ASSERT(mDocumentOpenedButNotLoaded);
@@ -790,14 +793,24 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) {
             }
           }
         }
-        if (parent) {
-          parent->ChildDoneWithOnload(this);
-        }
+        NotifyDoneWithOnload(parent);
       }
     }
   }
 }
 
+void nsDocLoader::NotifyDoneWithOnload(nsDocLoader* aParent) {
+  if (aParent) {
+    // In-process parent:
+    aParent->ChildDoneWithOnload(this);
+  } else if (BrowserChild* browserChild =
+                 BrowserChild::GetFrom(static_cast<nsDocShell*>(this))) {
+    // OOP parent:
+    mozilla::Unused << browserChild->SendMaybeFireEmbedderLoadEvents(
+        /*aIsTrusted*/ true, /*aFireLoadAtEmbeddingElement*/ false);
+  }
+}
+
 void nsDocLoader::doStartDocumentLoad(void) {
 #if defined(DEBUG)
   nsAutoCString buffer;

uriloader/base/nsDocLoader.h

@@ -31,6 +31,12 @@
 
 #include "mozilla/LinkedList.h"
 
+namespace mozilla {
+namespace dom {
+class BrowserBridgeChild;
+}  // namespace dom
+}  // namespace mozilla
+
 /****************************************************************************
  * nsDocLoader implementation...
  ****************************************************************************/
@@ -51,6 +57,8 @@ class nsDocLoader : public nsIDocumentLoader,
                     public nsIChannelEventSink,
                     public nsISupportsPriority {
  public:
+  using BrowserBridgeChild = mozilla::dom::BrowserBridgeChild;
+
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_THIS_DOCLOADER_IMPL_CID)
 
   nsDocLoader();
@@ -135,6 +143,26 @@ class nsDocLoader : public nsIDocumentLoader,
     mTreatAsBackgroundLoad = false;
   };
 
+  // Inform a parent docloader that a BrowserBridgeChild has been created for
+  // an OOP sub-document.
+  // (This is the OOP counterpart to ChildEnteringOnload below.)
+  void OOPChildLoadStarted(BrowserBridgeChild* aChild) {
+    MOZ_DIAGNOSTIC_ASSERT(!mOOPChildrenLoading.Contains(aChild));
+    mOOPChildrenLoading.AppendElement(aChild);
+  }
+
+  // Inform a parent docloader that the BrowserBridgeChild for one of its
+  // OOP sub-documents is done calling its onload handler.
+  // (This is the OOP counterpart to ChildDoneWithOnload below.)
+  void OOPChildLoadDone(BrowserBridgeChild* aChild) {
+    // aChild will not be in the list if nsDocLoader::Stop() was called, since
+    // that clears mOOPChildrenLoading.  It also dispatches the 'load' event,
+    // so we don't need to call DocLoaderIsEmpty in that case.
+    if (mOOPChildrenLoading.RemoveElement(aChild)) {
+      DocLoaderIsEmpty(true);
+    }
+  }
+
  protected:
   virtual ~nsDocLoader();
 
@@ -201,6 +229,8 @@ class nsDocLoader : public nsIDocumentLoader,
   void doStopURLLoad(nsIRequest* request, nsresult aStatus);
   void doStopDocumentLoad(nsIRequest* request, nsresult aStatus);
 
+  void NotifyDoneWithOnload(nsDocLoader* aParent);
+
   // Inform a parent docloader that aChild is about to call its onload
   // handler.
   MOZ_MUST_USE bool ChildEnteringOnload(nsIDocumentLoader* aChild) {
@@ -341,6 +371,10 @@ class nsDocLoader : public nsIDocumentLoader,
   // loadgroup) unless this is empty.
   nsCOMArray<nsIDocumentLoader> mChildrenInOnload;
 
+  // The OOP counterpart to mChildrenInOnload.
+  // Not holding strong refs here since we don't actually use the BBCs.
+  nsTArray<const BrowserBridgeChild*> mOOPChildrenLoading;
+
   int64_t GetMaxTotalProgress();
 
   nsresult AddRequestInfo(nsIRequest* aRequest);