commit 43b0fd6d3b1d6f77b16edc4c22b4789d9e5a0be9 parent 5d0bdc0d2a6ebc1a3ca7ada59640ef7e4b88b711 Author: Jan-Niklas Jaeschke <jjaschke@mozilla.com> Date: Thu, 9 Oct 2025 07:49:47 +0000 Bug 1991911 - Navigation API: Restore upcoming API method tracker when aborting. r=smaug Triggering a second navigation before the first one finished will abort the first navigation and emit `navigateerror`. However, `navigateerror` can run script, and therefore can trigger another navigation. This would run in a (per current status of the HTML spec) unexpected state. The third navigation would be reentrant, and expects a clean internal state (no `mUpcomingNonTraverseAPIMethodTracker`). otherwise it would crash. This patch moves the upcoming API method tracker into a temporary before aborting ongoing navigations, and reinstating it after the abort loop is finished. See also discussion at https://github.com/whatwg/html/issues/11735 Additionally, this patch fixes a bug where the unload counter was set too late, and therefore navigations triggered from pagehide event would not work correctly. Differential Revision: https://phabricator.services.mozilla.com/D267787 Diffstat:
5 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/dom/navigation/Navigation.cpp b/dom/navigation/Navigation.cpp @@ -770,6 +770,20 @@ void Navigation::Reload(JSContext* aCx, const NavigationReloadOptions& aOptions, MOZ_ASSERT(docShell); docShell->ReloadNavigable(Some(WrapNotNullUnchecked(aCx)), nsIWebNavigation::LOAD_FLAGS_NONE, serializedState); + // This is stolen from #dom-navigation-navigate. If the upcoming non-traverse + // API method tracker is still apiMethodTracker, this means that the reload + // algorithm bailed out before ever getting to the inner navigate event + // firing algorithm which would promote that upcoming API method tracker to + // ongoing. This is done here because gecko's reload implementation has + // additional abort code paths which may return early before firing the + // navigate event. + if (mUpcomingNonTraverseAPIMethodTracker == apiMethodTracker) { + mUpcomingNonTraverseAPIMethodTracker = nullptr; + ErrorResult rv; + rv.ThrowAbortError("Reload aborted."); + SetEarlyErrorResult(aCx, aResult, std::move(rv)); + return; + } // 10. Return a navigation API method tracker-derived result for // apiMethodTracker. @@ -1621,9 +1635,20 @@ void Navigation::InnerInformAboutAbortingNavigation(JSContext* aCx) { // As per https://github.com/whatwg/html/issues/11579, we should abort all // ongoing navigate events within "inform the navigation API about aborting // navigation". + + // As per https://github.com/whatwg/html/issues/11735, we should move out + // the non-traverse api method tracker while aborting ongoing navigations. + // Aborting allows to run script (onnavigateerror), which could start a new + // navigation, which asserts if there's an upcoming non-traverse api method + // tracker. + RefPtr upcomingNonTraverseAPIMethodTracker = + std::move(mUpcomingNonTraverseAPIMethodTracker); while (HasOngoingNavigateEvent()) { AbortOngoingNavigation(aCx); } + MOZ_DIAGNOSTIC_ASSERT(!mUpcomingNonTraverseAPIMethodTracker); + mUpcomingNonTraverseAPIMethodTracker = + std::move(upcomingNonTraverseAPIMethodTracker); } // https://html.spec.whatwg.org/#abort-the-ongoing-navigation diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp @@ -1340,6 +1340,12 @@ nsDocumentViewer::PageHide(bool aIsUnload) { StaticPrefs::javascript_options_gc_delay() * 2)); } + // https://html.spec.whatwg.org/multipage/browsing-the-web.html#unload-a-document + // Create an RAII object on mDocument that will increment the + // should-ignore-opens-during-unload counter on initialization + // and decrement it again when it goes out of scope. + IgnoreOpensDuringUnload ignoreOpens(mDocument); + mDocument->OnPageHide(!aIsUnload, nullptr); // inform the window so that the focus state is reset. @@ -1362,12 +1368,6 @@ nsDocumentViewer::PageHide(bool aIsUnload) { return NS_ERROR_NULL_POINTER; } - // https://html.spec.whatwg.org/multipage/browsing-the-web.html#unload-a-document - // Create an RAII object on mDocument that will increment the - // should-ignore-opens-during-unload counter on initialization - // and decrement it again when it goes out of scope. - IgnoreOpensDuringUnload ignoreOpens(mDocument); - // Now, fire an Unload event to the document... nsEventStatus status = nsEventStatus_eIgnore; WidgetEvent event(true, eUnload); diff --git a/testing/web-platform/meta/navigation-api/navigate-event/navigate-multiple-navigation-navigate.html.ini b/testing/web-platform/meta/navigation-api/navigate-event/navigate-multiple-navigation-navigate.html.ini @@ -1,4 +0,0 @@ -[navigate-multiple-navigation-navigate.html] - expected: - if (os == "mac") and not debug: [CRASH, ERROR] - CRASH diff --git a/testing/web-platform/meta/navigation-api/navigation-methods/return-value/navigate-pagehide.html.ini b/testing/web-platform/meta/navigation-api/navigation-methods/return-value/navigate-pagehide.html.ini @@ -1,4 +0,0 @@ -[navigate-pagehide.html] - expected: ERROR - [navigate() inside onpagehide] - expected: TIMEOUT diff --git a/testing/web-platform/meta/navigation-api/navigation-methods/return-value/reload-pagehide.html.ini b/testing/web-platform/meta/navigation-api/navigation-methods/return-value/reload-pagehide.html.ini @@ -1,4 +0,0 @@ -[reload-pagehide.html] - expected: TIMEOUT - [reload() inside onpagehide] - expected: TIMEOUT