Skip to content

Commit 86a2114

Browse files
committed
imported/w3c/web-platform-tests/navigation-api/ordering-and-transition/navigate-204-205-download-then-same-document.html is a flaky crash
https://bugs.webkit.org/show_bug.cgi?id=301894 Reviewed by Ryosuke Niwa. The crash was due to the Navigation object not marking its associated ongoing NavigateEvent when getting visited. Similarly, the NavigateEvent would fail to mark its associated AbortSignal when visited. As a result, the AbortSignal's JS wrapper could get garbage collected and we would crash trying to dispatch the abort event on it. The test was flakily crashing in debug before this change and is now reliably passing. * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/bindings/js/JSNavigateEventCustom.cpp: (WebCore::JSNavigateEvent::visitAdditionalChildren): * Source/WebCore/bindings/js/JSNavigationCustom.cpp: Copied from Source/WebCore/bindings/js/JSNavigateEventCustom.cpp. (WebCore::JSNavigation::visitAdditionalChildren): * Source/WebCore/dom/Microtasks.h: * Source/WebCore/page/NavigateEvent.cpp: (WebCore::root): * Source/WebCore/page/NavigateEvent.h: * Source/WebCore/page/NavigateEvent.idl: * Source/WebCore/page/Navigation.h: * Source/WebCore/page/Navigation.idl: Canonical link: https://commits.webkit.org/302530@main
1 parent a051a95 commit 86a2114

File tree

10 files changed

+71
-11
lines changed

10 files changed

+71
-11
lines changed

Source/WebCore/Sources.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,7 @@ bindings/js/JSMessagePortCustom.cpp
737737
bindings/js/JSMutationObserverCustom.cpp
738738
bindings/js/JSMutationRecordCustom.cpp
739739
bindings/js/JSNavigateEventCustom.cpp
740+
bindings/js/JSNavigationCustom.cpp
740741
bindings/js/JSNavigatorCustom.cpp
741742
bindings/js/JSNodeCustom.cpp
742743
bindings/js/JSNodeIteratorCustom.cpp

Source/WebCore/WebCore.xcodeproj/project.pbxproj

Lines changed: 13 additions & 11 deletions
Large diffs are not rendered by default.

Source/WebCore/bindings/js/JSNavigateEventCustom.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ void JSNavigateEvent::visitAdditionalChildren(Visitor& visitor)
3333
{
3434
auto& event = wrapped();
3535
event.infoWrapper().visit(visitor);
36+
if (auto* signal = event.signal())
37+
addWebCoreOpaqueRoot(visitor, signal);
3638
}
3739

3840
DEFINE_VISIT_ADDITIONAL_CHILDREN(JSNavigateEvent);
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright (C) 2025 Apple Inc. All rights reserved.
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions
6+
* are met:
7+
* 1. Redistributions of source code must retain the above copyright
8+
* notice, this list of conditions and the following disclaimer.
9+
* 2. Redistributions in binary form must reproduce the above copyright
10+
* notice, this list of conditions and the following disclaimer in the
11+
* documentation and/or other materials provided with the distribution.
12+
*
13+
* THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
14+
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
15+
* THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
16+
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
17+
* BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
18+
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
19+
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
20+
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
21+
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
22+
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
23+
* THE POSSIBILITY OF SUCH DAMAGE.
24+
*/
25+
26+
#include "config.h"
27+
#include "JSNavigation.h"
28+
29+
#include "JSNavigateEvent.h"
30+
#include "NavigateEvent.h"
31+
32+
namespace WebCore {
33+
34+
template<typename Visitor>
35+
void JSNavigation::visitAdditionalChildren(Visitor& visitor)
36+
{
37+
// We cannot ref the event on the GC thread.
38+
SUPPRESS_UNCOUNTED_ARG if (auto* event = wrapped().ongoingNavigateEvent())
39+
addWebCoreOpaqueRoot(visitor, event);
40+
}
41+
42+
DEFINE_VISIT_ADDITIONAL_CHILDREN(JSNavigation);
43+
44+
} // namespace WebCore

Source/WebCore/dom/Microtasks.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <wtf/Forward.h>
2727
#include <wtf/TZoneMalloc.h>
2828
#include <wtf/Vector.h>
29+
#include <wtf/WeakHashMap.h>
2930
#include <wtf/WeakPtr.h>
3031

3132
namespace JSC {

Source/WebCore/page/NavigateEvent.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,9 @@ void NavigateEvent::finish(Document& document, InterceptionHandlersDidFulfill di
190190
m_interceptionState = InterceptionState::Finished;
191191
}
192192

193+
WebCoreOpaqueRoot root(NavigateEvent* event)
194+
{
195+
return WebCoreOpaqueRoot { event };
196+
}
197+
193198
} // namespace WebCore

Source/WebCore/page/NavigateEvent.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,4 +141,6 @@ class NavigateEvent final : public Event {
141141
RefPtr<AbortController> m_abortController;
142142
};
143143

144+
WebCoreOpaqueRoot root(NavigateEvent*);
145+
144146
} // namespace WebCore

Source/WebCore/page/NavigateEvent.idl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
[
2+
GenerateIsReachable=Impl,
23
JSCustomMarkFunction,
34
EnabledBySetting=NavigationAPIEnabled,
45
Exposed=Window

Source/WebCore/page/Navigation.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ class Navigation final : public RefCounted<Navigation>, public EventTarget, publ
186186
};
187187
Ref<AbortHandler> registerAbortHandler();
188188

189+
NavigateEvent* ongoingNavigateEvent() { return m_ongoingNavigateEvent.get(); } // This may get called on the GC thread.
189190
RefPtr<NavigateEvent> protectedOngoingNavigateEvent() { return m_ongoingNavigateEvent; }
190191
bool hasInterceptedOngoingNavigateEvent() const { return m_ongoingNavigateEvent && m_ongoingNavigateEvent->wasIntercepted(); }
191192

Source/WebCore/page/Navigation.idl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
[
22
EnabledBySetting=NavigationAPIEnabled,
33
GenerateIsReachable=ReachableFromDOMWindow,
4+
JSCustomMarkFunction,
45
Exposed=Window
56
] interface Navigation : EventTarget {
67
sequence<NavigationHistoryEntry> entries();

0 commit comments

Comments
 (0)