Skip to content

Commit

Permalink
[Navigation] Implement the "abort the ongoing navigation" algorithm
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=274170

Reviewed by Alex Christensen.

Implement the "abort the ongoing navigation" algorithm [1].

Use it to abort ongoing navigations when a new one starts and to
abort events with defaultPrevented.

[1] https://html.spec.whatwg.org/multipage/nav-history-apis.html#abort-the-ongoing-navigation

* LayoutTests/imported/w3c/web-platform-tests/navigation-api/currententrychange-event/navigation-navigate-preventDefault-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/return-value/navigate-interrupted-within-onnavigate-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/return-value/navigate-preventDefault-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entry-after-detach-expected.txt:
* Source/WebCore/page/Navigation.cpp:
(WebCore::Navigation::rejectFinishedPromise):
(WebCore::Navigation::abortOngoingNavigation):
(WebCore::Navigation::innerDispatchNavigateEvent):
* Source/WebCore/page/Navigation.h:

Canonical link: https://commits.webkit.org/278874@main
  • Loading branch information
TingPing committed May 16, 2024
1 parent 616f42b commit 4cce2f2
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@

Harness Error (TIMEOUT), message = null

TIMEOUT currententrychange does not fire when onnavigate preventDefault() is called Test timed out
PASS currententrychange does not fire when onnavigate preventDefault() is called

Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
FAIL: Timed out waiting for notifyDone to be called
CONSOLE MESSAGE: Unhandled Promise Rejection: AbortError: Navigation aborted

FAIL if navigate() is called inside onnavigate, the previous navigation and navigate event are cancelled assert_array_equals: expected property 1 to be "#2" but got "" (expected array ["", "#2"] got ["", ""])

Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@

Harness Error (TIMEOUT), message = null

TIMEOUT navigate() when the onnavigate handler calls preventDefault() Test timed out
PASS navigate() when the onnavigate handler calls preventDefault()

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@

FAIL NavigationHistoryEntry properties after detach assert_equals: expected (object) null but got (string) "http://web-platform.test:8800/common/blank.html"
PASS NavigationHistoryEntry properties after detach

59 changes: 53 additions & 6 deletions Source/WebCore/page/Navigation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,15 @@ void Navigation::resolveFinishedPromise(NavigationAPIMethodTracker* apiMethodTra
cleanupAPIMethodTracker(apiMethodTracker);
}

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#reject-the-finished-promise
void Navigation::rejectFinishedPromise(NavigationAPIMethodTracker* apiMethodTracker, Exception&& exception, JSC::JSValue exceptionObject)
{
// finished is already marked as handled at this point so don't overwrite that.
apiMethodTracker->finishedPromise->reject(exception, RejectAsHandled::Yes, exceptionObject);
apiMethodTracker->committedPromise->reject(exception, RejectAsHandled::No, exceptionObject);
cleanupAPIMethodTracker(apiMethodTracker);
}

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#notify-about-the-committed-to-entry
void Navigation::notifyCommittedToEntry(NavigationAPIMethodTracker* apiMethodTracker, NavigationHistoryEntry* entry)
{
Expand Down Expand Up @@ -478,6 +487,36 @@ void Navigation::cleanupAPIMethodTracker(NavigationAPIMethodTracker* apiMethodTr
}
}

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#abort-the-ongoing-navigation
void Navigation::abortOngoingNavigation(NavigateEvent& event)
{
m_focusChangedDuringOnoingNavigation = false;
m_suppressNormalScrollRestorationDuringOngoingNavigation = false;

if (event.isBeingDispatched())
event.preventDefault();

auto& globalObject = *scriptExecutionContext()->globalObject();
JSC::JSLockHolder locker(globalObject.vm());
auto exception = Exception(ExceptionCode::AbortError, "Navigation aborted"_s);
auto domException = createDOMException(globalObject, exception.isolatedCopy());

event.signal()->signalAbort(domException);

m_ongoingNavigateEvent = nullptr;

// FIXME: Fill in exception information.
dispatchEvent(ErrorEvent::create(eventNames().navigateerrorEvent, { }, 0, 0, { globalObject.vm(), domException }));

if (m_ongoingAPIMethodTracker)
rejectFinishedPromise(m_ongoingAPIMethodTracker.get(), WTFMove(exception), domException);

if (m_transition) {
// FIXME: Reject navigation's transition's finished promise with error.
m_transition = nullptr;
}
}

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#inner-navigate-event-firing-algorithm
bool Navigation::innerDispatchNavigateEvent(NavigationNavigationType navigationType, Ref<NavigationDestination>&& destination, const String& downloadRequestFilename)
{
Expand All @@ -490,6 +529,9 @@ bool Navigation::innerDispatchNavigateEvent(NavigationNavigationType navigationT
return true;
}

if (m_ongoingNavigateEvent)
abortOngoingNavigation(*m_ongoingNavigateEvent);

promoteUpcomingAPIMethodTracker(destination->key());

RefPtr document = window()->protectedDocument();
Expand Down Expand Up @@ -531,8 +573,10 @@ bool Navigation::innerDispatchNavigateEvent(NavigationNavigationType navigationT

if (event->defaultPrevented()) {
// FIXME: If navigationType is "traverse", then consume history-action user activation.
// FIXME: If event's abort controller's signal is not aborted, then abort the ongoing navigation given navigation.
m_ongoingNavigateEvent = nullptr;
if (!event->signal()->aborted())
abortOngoingNavigation(event);
else
m_ongoingNavigateEvent = nullptr;
return false;
}

Expand Down Expand Up @@ -572,7 +616,10 @@ bool Navigation::innerDispatchNavigateEvent(NavigationNavigationType navigationT
// FIXME: Step 33.4: We need to wait for all promises.

if (document->isFullyActive() && !abortController->signal().aborted()) {
ASSERT(m_ongoingNavigateEvent == event.ptr());
// If a new event has been dispatched in our event handler then we were aborted above.
if (m_ongoingNavigateEvent != event.ptr())
return false;

m_ongoingNavigateEvent = nullptr;

event->finish();
Expand All @@ -589,10 +636,10 @@ bool Navigation::innerDispatchNavigateEvent(NavigationNavigationType navigationT
// FIXME: Fill in error information.
dispatchEvent(ErrorEvent::create(eventNames().navigateerrorEvent, { }, { }, 0, 0, { }));
}
} else {
// FIXME: and the following failure steps given reason rejectionReason:
m_ongoingNavigateEvent = nullptr;
}

// FIXME: and the following failure steps given reason rejectionReason:
m_ongoingNavigateEvent = nullptr;
} else if (apiMethodTracker)
cleanupAPIMethodTracker(apiMethodTracker.get());

Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/page/Navigation.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ class Navigation final : public RefCounted<Navigation>, public EventTarget, publ
RefPtr<NavigationAPIMethodTracker> addUpcomingTrarveseAPIMethodTracker(Ref<DeferredPromise>&& committed, Ref<DeferredPromise>&& finished, const String& key, JSC::JSValue info);
void cleanupAPIMethodTracker(NavigationAPIMethodTracker*);
void resolveFinishedPromise(NavigationAPIMethodTracker*);
void rejectFinishedPromise(NavigationAPIMethodTracker*, Exception&&, JSC::JSValue exceptionObject);
void abortOngoingNavigation(NavigateEvent&);
void promoteUpcomingAPIMethodTracker(const String& destinationKey);
void notifyCommittedToEntry(NavigationAPIMethodTracker*, NavigationHistoryEntry*);
Result apiMethodTrackerDerivedResult(const NavigationAPIMethodTracker&);
Expand Down

0 comments on commit 4cce2f2

Please sign in to comment.