Bug 1620679 - Don't fire load event from within Stop(). r=smaug

This matches what the spec says, and what blink does.

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

dom/base/Document.cpp

@@ -1311,6 +1311,7 @@ Document::Document(const char* aContentType)
       mHasBeenEditable(false),
       mHasWarnedAboutZoom(false),
       mIsRunningExecCommand(false),
+      mSetCompleteAfterDOMContentLoaded(false),
       mPendingFullscreenRequests(0),
       mXMLDeclarationBits(0),
       mOnloadBlockCount(0),
@@ -7275,6 +7276,11 @@ void Document::DispatchContentLoadedEvents() {
     }
   }
 
+  if (mSetCompleteAfterDOMContentLoaded) {
+    SetReadyStateInternal(ReadyState::READYSTATE_COMPLETE);
+    mSetCompleteAfterDOMContentLoaded = false;
+  }
+
   UnblockOnload(true);
 }
 
@@ -11257,6 +11263,22 @@ void Document::SuppressEventHandling(uint32_t aIncrease) {
   EnumerateSubDocuments(suppressInSubDoc);
 }
 
+void Document::NotifyAbortedLoad() {
+  // If we still have outstanding work blocking DOMContentLoaded,
+  // then don't try to change the readystate now, but wait until
+  // they finish and then do so.
+  if (mBlockDOMContentLoaded > 0 && !mDidFireDOMContentLoaded) {
+    mSetCompleteAfterDOMContentLoaded = true;
+    return;
+  }
+
+  // Otherwise we're fully done at this point, so set the
+  // readystate to complete.
+  if (GetReadyStateEnum() == Document::READYSTATE_INTERACTIVE) {
+    SetReadyStateInternal(Document::READYSTATE_COMPLETE);
+  }
+}
+
 static void FireOrClearDelayedEvents(nsTArray<nsCOMPtr<Document>>& aDocuments,
                                      bool aFireEvents) {
   nsIFocusManager* fm = nsFocusManager::GetFocusManager();

dom/base/Document.h

@@ -2021,6 +2021,8 @@ class Document : public nsINode,
   void NotifyLoading(bool aNewParentIsLoading, const ReadyState& aCurrentState,
                      ReadyState aNewState);
 
+  void NotifyAbortedLoad();
+
   // notify that a content node changed state.  This must happen under
   // a scriptblocker but NOT within a begin/end update.
   void ContentStateChanged(nsIContent* aContent, EventStates aStateMask);
@@ -4586,6 +4588,13 @@ class Document : public nsINode,
   // While we're handling an execCommand call, set to true.
   bool mIsRunningExecCommand : 1;
 
+  // True if we should change the readystate to complete after we fire
+  // DOMContentLoaded. This happens when we abort a load and
+  // nsDocumentViewer::EndLoad runs while we still have things blocking
+  // DOMContentLoaded. We wait for those to complete, and then update the
+  // readystate when they finish.
+  bool mSetCompleteAfterDOMContentLoaded : 1;
+
   uint8_t mPendingFullscreenRequests;
 
   uint8_t mXMLDeclarationBits;

layout/base/nsDocumentViewer.cpp

@@ -1162,6 +1162,12 @@ nsDocumentViewer::LoadComplete(nsresult aStatus) {
     }
   } else {
     // XXX: Should fire error event to the document...
+
+    // If our load was explicitly aborted, then we want to set our
+    // readyState to COMPLETE, and fire a readystatechange event.
+    if (aStatus == NS_BINDING_ABORTED && mDocument) {
+      mDocument->NotifyAbortedLoad();
+    }
   }
 
   // Notify the document that it has been shown (regardless of whether
@@ -1173,7 +1179,10 @@ nsDocumentViewer::LoadComplete(nsresult aStatus) {
   // pageshow, and that's pretty broken... Fortunately, this should be rare.
   // (It requires us to spin the event loop in onload handler, e.g. via sync
   // XHR, in order for the navigation-away to happen before onload completes.)
-  if (mDocument && mDocument->IsCurrentActiveDocument()) {
+  // We skip firing pageshow if we're currently handling unload, or if loading
+  // was explicitly aborted.
+  if (mDocument && mDocument->IsCurrentActiveDocument() &&
+      aStatus != NS_BINDING_ABORTED) {
     // Re-get window, since it might have changed during above firing of onload
     window = mDocument->GetWindow();
     if (window) {

testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/abort-document-load-1.html

@@ -0,0 +1,32 @@
+<!doctype html>
+<script>
+parent.postMessage(document.readyState, "*");
+let f = document.createElement("iframe");
+f.onload = function() {
+  parent.postMessage("stop", "*");
+  window.stop();
+};
+document.documentElement.appendChild(f);
+
+window.addEventListener("load", (event) => {
+    parent.postMessage("load", "*");
+});
+window.addEventListener("error", (event) => {
+    parent.postMessage("error", "*");
+});
+window.addEventListener("abort", (event) => {
+    parent.postMessage("abort", "*");
+});
+window.addEventListener("pageshow", (event) => {
+    parent.postMessage("pageshow", "*");
+});
+window.addEventListener("DOMContentLoaded", (event) => {
+    parent.postMessage("DOMContentLoaded", "*");
+});
+document.addEventListener("readystatechange", (event) => {
+    if (document.readyState === "complete") {
+        parent.postMessage("complete", "*");
+    }
+});
+
+</script>

testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/abort-document-load-2.html

@@ -0,0 +1,32 @@
+<!doctype html>
+<script>
+parent.postMessage(document.readyState, "*");
+
+window.addEventListener("load", (event) => {
+    parent.postMessage("load", "*");
+});
+window.addEventListener("error", (event) => {
+    parent.postMessage("error", "*");
+});
+window.addEventListener("abort", (event) => {
+    parent.postMessage("abort", "*");
+});
+window.addEventListener("pageshow", (event) => {
+    parent.postMessage("pageshow", "*");
+});
+window.addEventListener("DOMContentLoaded", (event) => {
+    parent.postMessage("DOMContentLoaded", "*");
+});
+document.addEventListener("readystatechange", (event) => {
+    if (document.readyState === "complete") {
+        parent.postMessage("complete", "*");
+    }
+});
+
+window.setTimeout(function() {
+  parent.postMessage("stop", "*");
+  window.stop();
+}, 100);
+
+</script>
+<link rel="stylesheet" href="/common/slow.py"></link>

testing/web-platform/tests/html/browsers/browsing-the-web/navigating-across-documents/abort-document-load.html

@@ -0,0 +1,30 @@
+<!doctype html>
+<title>Aborting a Document load</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="help" href="https://html.spec.whatwg.org/multipage/browsing-the-web.html#aborting-a-document-load">
+<div id="log"></div>
+<script>
+var events = [];
+onmessage = function(e) {
+  events.push(e.data);
+};
+async_test(test => {
+  test.step_timeout(() => {
+    const frame = document.querySelector('iframe');
+    const child = frame.contentWindow;
+    assert_equals(child.document.readyState, 'complete', 'readyState is complete');
+    assert_array_equals(events, ["loading", "DOMContentLoaded", "stop", "complete"], 'no load event was fired');
+    events = [];
+    frame.src = "abort-document-load-2.html";
+
+    test.step_timeout(() => {
+      const child = frame.contentWindow;
+      assert_equals(child.document.readyState, 'complete', 'readyState is complete');
+      assert_array_equals(events, ["loading", "DOMContentLoaded", "stop", "complete"], 'no load event was fired');
+      test.done();
+    }, 1000);
+  }, 1000);
+});
+</script>
+<iframe src="abort-document-load-1.html"></iframe>

uriloader/base/nsDocLoader.cpp

@@ -256,8 +256,8 @@ nsDocLoader::Stop(void) {
   // Stop call.
   mIsFlushingLayout = false;
 
-  // Clear out mChildrenInOnload.  We want to make sure to fire our
-  // onload at this point, and there's no issue with mChildrenInOnload
+  // Clear out mChildrenInOnload.  We're not going to fire our onload
+  // anyway at this point, and there's no issue with mChildrenInOnload
   // after this, since mDocumentRequest will be null after the
   // DocLoaderIsEmpty() call.
   mChildrenInOnload.Clear();
@@ -274,7 +274,12 @@ nsDocLoader::Stop(void) {
   // we wouldn't need the call here....
 
   NS_ASSERTION(!IsBusy(), "Shouldn't be busy here");
-  DocLoaderIsEmpty(false);
+
+  // If Cancelling the load group only had pending subresource requests, then
+  // the group status will still be success, and we would fire the load event.
+  // We want to avoid that when we're aborting the load, so override the status
+  // with an explicit NS_BINDING_ABORTED value.
+  DocLoaderIsEmpty(false, Some(NS_BINDING_ABORTED));
 
   return rv;
 }
@@ -660,7 +665,8 @@ NS_IMETHODIMP nsDocLoader::GetDocumentChannel(nsIChannel** aChannel) {
   return CallQueryInterface(mDocumentRequest, aChannel);
 }
 
-void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) {
+void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout,
+                                   const Maybe<nsresult>& aOverrideStatus) {
   if (IsBlockingLoadEvent()) {
     /* In the unimagineably rude circumstance that onload event handlers
        triggered by this function actually kill the window ... ok, it's
@@ -724,7 +730,11 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) {
       mProgressStateFlags = nsIWebProgressListener::STATE_STOP;
 
       nsresult loadGroupStatus = NS_OK;
-      mLoadGroup->GetStatus(&loadGroupStatus);
+      if (aOverrideStatus) {
+        loadGroupStatus = *aOverrideStatus;
+      } else {
+        mLoadGroup->GetStatus(&loadGroupStatus);
+      }
 
       //
       // New code to break the circular reference between

uriloader/base/nsDocLoader.h

@@ -248,7 +248,11 @@ class nsDocLoader : public nsIDocumentLoader,
   // fact empty.  This method _does_ make sure that layout is flushed if our
   // loadgroup has no active requests before checking for "real" emptiness if
   // aFlushLayout is true.
-  void DocLoaderIsEmpty(bool aFlushLayout);
+  // @param aOverrideStatus An optional status to use when notifying listeners
+  // of the completed load, instead of using the load group's status.
+  void DocLoaderIsEmpty(
+      bool aFlushLayout,
+      const Maybe<nsresult>& aOverrideStatus = mozilla::Nothing());
 
  protected:
   struct nsStatusInfo : public mozilla::LinkedListElement<nsStatusInfo> {