LibWeb: Commit same-document navigations synchronously
Finalize fragment navigations and URL/history updates immediately when no traversal state is active. Keep the queued same-document finalizer as the fallback for reentrant traversal work and child navigables whose nested history is not installed yet. Share the entry-list portion of same-document navigation finalization between the fast path and queued fallback, so append and replace bookkeeping cannot drift. Preserve unrelated ongoing cross-document navigations when a page starts a load and then performs a same-document history update in the same task. This matches Chromium, WebKit, and Gecko: the same-document update must not cancel the pending real navigation. The session-history mirror tests now observe synchronous UI updates. A navigation test covers the pending-load plus pushState race.
This commit is contained in:
Andreas Kling
Andreas Kling
parent
340d87efd6
commit
327437cfc6
7 changed files with 195 additions and 70 deletions
|
|
@ -2455,18 +2455,24 @@ void Navigable::navigate_to_a_fragment(URL::URL const& url, HistoryHandlingBehav
|
|||
// 16. Let traversable be navigable's traversable navigable.
|
||||
auto traversable = traversable_navigable();
|
||||
|
||||
// AD-HOC: Browser engines commit same-document navigations synchronously when no traversal state is active. Keep
|
||||
// the spec's queued synchronous-navigation steps as the fallback for reentrant traversal work and child
|
||||
// navigables whose nested history is not ready yet.
|
||||
// 17. Append the following session history synchronous navigation steps involving navigable to traversable:
|
||||
traversable->append_session_history_synchronous_navigation_steps(*this, GC::create_function(heap(), [this, traversable, history_entry, entry_to_replace, navigation_id, history_handling, user_involvement](NonnullRefPtr<Core::Promise<Empty>> signal) {
|
||||
// 1. Finalize a same-document navigation given traversable, navigable, historyEntry, entryToReplace, historyHandling, and userInvolvement.
|
||||
finalize_a_same_document_navigation(*traversable, *this, history_entry, entry_to_replace, history_handling, user_involvement,
|
||||
GC::create_function(heap(), [signal](HistoryStepResult) {
|
||||
signal->resolve({});
|
||||
}));
|
||||
if (!traversable->try_to_synchronously_commit_same_document_navigation(*this, history_entry, entry_to_replace)) {
|
||||
traversable->append_session_history_synchronous_navigation_steps(*this, GC::create_function(heap(), [this, traversable, history_entry, entry_to_replace, navigation_id, history_handling, user_involvement](NonnullRefPtr<Core::Promise<Empty>> signal) {
|
||||
// 1. Finalize a same-document navigation given traversable, navigable, historyEntry, entryToReplace,
|
||||
// historyHandling, and userInvolvement.
|
||||
finalize_a_same_document_navigation(*traversable, *this, history_entry, entry_to_replace, history_handling, user_involvement,
|
||||
GC::create_function(heap(), [signal](HistoryStepResult) {
|
||||
signal->resolve({});
|
||||
}));
|
||||
|
||||
// FIXME: 2. Invoke WebDriver BiDi fragment navigated with navigable and a new WebDriver BiDi
|
||||
// navigation status whose id is navigationId, url is url, and status is "complete".
|
||||
(void)navigation_id;
|
||||
}));
|
||||
// FIXME: 2. Invoke WebDriver BiDi fragment navigated with navigable and a new WebDriver BiDi
|
||||
// navigation status whose id is navigationId, url is url, and status is "complete".
|
||||
(void)navigation_id;
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#evaluate-a-javascript:-url
|
||||
|
|
@ -3018,15 +3024,21 @@ void perform_url_and_history_update_steps(DOM::Document& document, URL::URL new_
|
|||
// 12. Let traversable be navigable's traversable navigable.
|
||||
auto traversable = navigable->traversable_navigable();
|
||||
|
||||
// AD-HOC: Browser engines commit same-document navigations synchronously when no traversal state is active. Keep
|
||||
// the spec's queued synchronous-navigation steps as the fallback for reentrant traversal work and child
|
||||
// navigables whose nested history is not ready yet.
|
||||
// 13. Append the following session history synchronous navigation steps involving navigable to traversable:
|
||||
traversable->append_session_history_synchronous_navigation_steps(*navigable, GC::create_function(document.realm().heap(), [traversable, navigable, new_entry, entry_to_replace, history_handling](NonnullRefPtr<Core::Promise<Empty>> signal) {
|
||||
// 1. Finalize a same-document navigation given traversable, navigable, newEntry, entryToReplace, historyHandling, and "none".
|
||||
finalize_a_same_document_navigation(*traversable, *navigable, new_entry, entry_to_replace, history_handling, UserNavigationInvolvement::None,
|
||||
GC::create_function(traversable->heap(), [signal](HistoryStepResult) {
|
||||
signal->resolve({});
|
||||
}));
|
||||
// 2. FIXME: Invoke WebDriver BiDi history updated with navigable.
|
||||
}));
|
||||
if (!traversable->try_to_synchronously_commit_same_document_navigation(*navigable, new_entry, entry_to_replace)) {
|
||||
traversable->append_session_history_synchronous_navigation_steps(*navigable, GC::create_function(document.realm().heap(), [traversable, navigable, new_entry, entry_to_replace, history_handling](NonnullRefPtr<Core::Promise<Empty>> signal) {
|
||||
// 1. Finalize a same-document navigation given traversable, navigable, newEntry, entryToReplace,
|
||||
// historyHandling, and "none".
|
||||
finalize_a_same_document_navigation(*traversable, *navigable, new_entry, entry_to_replace, history_handling, UserNavigationInvolvement::None,
|
||||
GC::create_function(traversable->heap(), [signal](HistoryStepResult) {
|
||||
signal->resolve({});
|
||||
}));
|
||||
// 2. FIXME: Invoke WebDriver BiDi history updated with navigable.
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
void Navigable::scroll_offset_did_change()
|
||||
|
|
|
|||
|
|
@ -321,6 +321,15 @@ static NonnullRefPtr<SessionHistoryEntry> create_session_history_entry_from_ui_p
|
|||
return entry;
|
||||
}
|
||||
|
||||
static bool synchronous_same_document_navigation_must_preserve_ongoing_navigation(Navigable const& navigable)
|
||||
{
|
||||
// AD-HOC: The spec queues same-document history updates because they happen synchronously, outside the traversal
|
||||
// queue, and must later resolve races with the current history step. If another navigation has already
|
||||
// claimed the navigable, leave that navigation ID alone. This matches Chromium, WebKit, and Gecko:
|
||||
// a same-document history update from the same task does not cancel a later cross-document navigation.
|
||||
return navigable.ongoing_navigation().has<String>();
|
||||
}
|
||||
|
||||
bool TraversableNavigable::replace_top_level_session_history_entries_from_ui_process(Vector<SessionHistoryEntryDescriptor> entries_from_ui_process, size_t current_top_level_entry_index)
|
||||
{
|
||||
if (entries_from_ui_process.is_empty() || current_top_level_entry_index >= entries_from_ui_process.size())
|
||||
|
|
@ -839,14 +848,8 @@ void ApplyHistoryStepState::start()
|
|||
// 8. For each navigable of changingNavigables:
|
||||
auto changing_navigables = m_traversable->get_all_navigables_whose_current_session_history_entry_will_change_or_reload(m_target_step);
|
||||
for (auto& navigable : changing_navigables) {
|
||||
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#finalize-a-same-document-navigation
|
||||
// AD-HOC: The spec queues same-document history updates because they happen synchronously, outside the
|
||||
// traversal queue, and must later resolve races with the current history step. If another navigation
|
||||
// has already claimed the navigable by the time this queued reconciliation runs, leave that navigation
|
||||
// ID alone. This matches the observable behavior of Chromium, WebKit, and Gecko: a same-document
|
||||
// history update does not cancel a later cross-document navigation from the same task.
|
||||
if (m_synchronous_navigation == TraversableNavigable::SynchronousNavigation::Yes
|
||||
&& navigable->ongoing_navigation().has<String>()) {
|
||||
&& synchronous_same_document_navigation_must_preserve_ongoing_navigation(*navigable)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -860,6 +863,18 @@ void ApplyHistoryStepState::start()
|
|||
// 1. Let targetEntry be the result of getting the target history entry given navigable and targetStep.
|
||||
auto target_entry = navigable->get_the_target_history_entry(m_target_step);
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/nav-history-apis.html#fire-a-traverse-navigate-event
|
||||
// NB: Same-document traversals are synchronous in browser engines, but the specification routes them through
|
||||
// the traversal queue. If a later cross-document navigation has already claimed the navigable by the time
|
||||
// this queued same-document traversal reaches its bookkeeping step, do not replace that navigation's ID
|
||||
// with "traversal". The queued traversal is stale reconciliation at that point, and must not cancel the
|
||||
// newer navigation.
|
||||
if (m_navigation_type == Bindings::NavigationType::Traverse
|
||||
&& navigable->ongoing_navigation().has<String>()
|
||||
&& target_entry->document_state()->document_id() == navigable->active_document_id()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// 2. Set navigable's current session history entry to targetEntry.
|
||||
navigable->set_current_session_history_entry(target_entry);
|
||||
|
||||
|
|
@ -2119,6 +2134,108 @@ void TraversableNavigable::apply_the_push_or_replace_history_step(int step, Hist
|
|||
apply_the_history_step(step, false, {}, {}, user_involvement, navigation_type, synchronous_navigation, Navigable::NavigationAPIAbortBehavior::Abort, pending_document, on_complete);
|
||||
}
|
||||
|
||||
static Optional<int> update_session_history_entries_for_same_document_navigation(TraversableNavigable& traversable, GC::Ref<Navigable> target_navigable, NonnullRefPtr<SessionHistoryEntry> target_entry, RefPtr<SessionHistoryEntry> entry_to_replace)
|
||||
{
|
||||
// NB: This is the entry-list portion of the "finalize a same-document navigation" algorithm. Keep the synchronous
|
||||
// commit path and the queued fallback sharing it so the two paths cannot drift.
|
||||
|
||||
// 2. If targetNavigable's active session history entry is not targetEntry, then return.
|
||||
// FIXME: This is a workaround for a spec issue where the early return loses replace entries.
|
||||
// Revisit when https://github.com/whatwg/html/issues/10232 is resolved.
|
||||
if (target_navigable->active_session_history_entry() != target_entry) {
|
||||
if (entry_to_replace) {
|
||||
auto& target_entries = target_navigable->get_session_history_entries();
|
||||
if (auto it = target_entries.find(*entry_to_replace); it != target_entries.end()) {
|
||||
target_entry->set_step(entry_to_replace->step());
|
||||
*it = target_entry;
|
||||
}
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
// 3. Let targetStep be null.
|
||||
Optional<int> target_step;
|
||||
|
||||
// 4. Let targetEntries be the result of getting session history entries for targetNavigable.
|
||||
auto& target_entries = target_navigable->get_session_history_entries();
|
||||
|
||||
// 5. If entryToReplace is null, then:
|
||||
// FIXME: Checking containment of entryToReplace should not be needed.
|
||||
// For more details see https://github.com/whatwg/html/issues/10232#issuecomment-2037543137
|
||||
if (!entry_to_replace || !target_entries.contains_slow(NonnullRefPtr { *entry_to_replace })) {
|
||||
// 1. Clear the forward session history of traversable.
|
||||
traversable.clear_the_forward_session_history();
|
||||
|
||||
// 2. Set targetStep to traversable's current session history step + 1.
|
||||
target_step = traversable.current_session_history_step() + 1;
|
||||
|
||||
// 3. Set targetEntry's step to targetStep.
|
||||
target_entry->set_step(*target_step);
|
||||
|
||||
// 4. Append targetEntry to targetEntries.
|
||||
target_entries.append(target_entry);
|
||||
} else {
|
||||
// 1. Replace entryToReplace with targetEntry in targetEntries.
|
||||
*(target_entries.find(*entry_to_replace)) = target_entry;
|
||||
|
||||
// 2. Set targetEntry's step to entryToReplace's step.
|
||||
target_entry->set_step(entry_to_replace->step());
|
||||
|
||||
// 3. Set targetStep to traversable's current session history step.
|
||||
target_step = traversable.current_session_history_step();
|
||||
}
|
||||
|
||||
return target_step;
|
||||
}
|
||||
|
||||
bool TraversableNavigable::try_to_synchronously_commit_same_document_navigation(GC::Ref<Navigable> target_navigable, NonnullRefPtr<SessionHistoryEntry> target_entry, RefPtr<SessionHistoryEntry> entry_to_replace)
|
||||
{
|
||||
if (m_apply_history_step_state || m_paused_apply_history_step_state)
|
||||
return false;
|
||||
|
||||
if (target_navigable->has_been_destroyed())
|
||||
return true;
|
||||
|
||||
if (!target_navigable->has_session_history_entry_and_ready_for_navigation())
|
||||
return false;
|
||||
|
||||
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#finalize-a-same-document-navigation
|
||||
auto target_step = update_session_history_entries_for_same_document_navigation(*this, target_navigable, target_entry, entry_to_replace);
|
||||
if (!target_step.has_value())
|
||||
return true;
|
||||
|
||||
target_navigable->set_current_session_history_entry(target_entry);
|
||||
m_current_session_history_step = get_the_used_step(*target_step);
|
||||
|
||||
// NB: The queued apply-history-step path clears the ongoing navigation when the history step finishes. The
|
||||
// synchronous fast path has already committed the same-document navigation and the Navigation API entry update
|
||||
// owns settling its promises/events, so do the same cleanup without reporting an abort to the Navigation API.
|
||||
if (!synchronous_same_document_navigation_must_preserve_ongoing_navigation(*target_navigable))
|
||||
target_navigable->set_ongoing_navigation({}, Navigable::NavigationAPIAbortBehavior::Preserve);
|
||||
|
||||
auto history_object_length_and_index = get_the_history_object_length_and_index(m_current_session_history_step);
|
||||
if (auto active_document = this->active_document()) {
|
||||
for (auto const& navigable : active_document->inclusive_descendant_navigables()) {
|
||||
if (navigable->has_been_destroyed() || !navigable->active_window() || !navigable->active_document()->is_fully_active())
|
||||
continue;
|
||||
|
||||
auto document = navigable->active_document();
|
||||
document->history()->m_index = history_object_length_and_index.script_history_index;
|
||||
document->history()->m_length = history_object_length_and_index.script_history_length;
|
||||
}
|
||||
}
|
||||
|
||||
if (page().client().should_report_session_history_updates()) {
|
||||
auto session_history_snapshot = create_session_history_snapshot(SaveActiveEntryPersistedState::Yes);
|
||||
page().client().page_did_update_session_history(session_history_snapshot.top_level_session_history_entries, session_history_snapshot.used_session_history_steps, session_history_snapshot.current_used_step_index);
|
||||
}
|
||||
|
||||
VERIFY(session_history_entries().size() > 0);
|
||||
page().client().page_did_update_navigation_buttons_state(can_go_back(), can_go_forward());
|
||||
page().client().page_did_change_url(current_session_history_entry()->url());
|
||||
return true;
|
||||
}
|
||||
|
||||
void TraversableNavigable::apply_the_traverse_history_step(int step, GC::Ptr<SourceSnapshotParams> source_snapshot_params, GC::Ptr<Navigable> initiator_to_check, UserNavigationInvolvement user_involvement, GC::Ref<GC::Function<void(HistoryStepResult)>> on_complete)
|
||||
{
|
||||
// 1. Return the result of applying the history step step to traversable given true, sourceSnapshotParams, initiatorToCheck, userInvolvement, and "traverse".
|
||||
|
|
@ -2225,53 +2342,12 @@ void finalize_a_same_document_navigation(GC::Ref<TraversableNavigable> traversab
|
|||
|
||||
// FIXME: 1. Assert: this is running on traversable's session history traversal queue.
|
||||
|
||||
// 2. If targetNavigable's active session history entry is not targetEntry, then return.
|
||||
// FIXME: This is a workaround for a spec issue where the early return loses replace entries.
|
||||
// Revisit when https://github.com/whatwg/html/issues/10232 is resolved.
|
||||
if (target_navigable->active_session_history_entry() != target_entry) {
|
||||
if (entry_to_replace) {
|
||||
auto& target_entries = target_navigable->get_session_history_entries();
|
||||
if (auto it = target_entries.find(*entry_to_replace); it != target_entries.end()) {
|
||||
target_entry->set_step(entry_to_replace->step());
|
||||
*it = target_entry;
|
||||
}
|
||||
}
|
||||
auto target_step = update_session_history_entries_for_same_document_navigation(*traversable, target_navigable, target_entry, entry_to_replace);
|
||||
if (!target_step.has_value()) {
|
||||
on_complete->function()(HistoryStepResult::Applied);
|
||||
return;
|
||||
}
|
||||
|
||||
// 3. Let targetStep be null.
|
||||
Optional<int> target_step;
|
||||
|
||||
// 4. Let targetEntries be the result of getting session history entries for targetNavigable.
|
||||
auto& target_entries = target_navigable->get_session_history_entries();
|
||||
|
||||
// 5. If entryToReplace is null, then:
|
||||
// FIXME: Checking containment of entryToReplace should not be needed.
|
||||
// For more details see https://github.com/whatwg/html/issues/10232#issuecomment-2037543137
|
||||
if (!entry_to_replace || !target_entries.contains_slow(NonnullRefPtr { *entry_to_replace })) {
|
||||
// 1. Clear the forward session history of traversable.
|
||||
traversable->clear_the_forward_session_history();
|
||||
|
||||
// 2. Set targetStep to traversable's current session history step + 1.
|
||||
target_step = traversable->current_session_history_step() + 1;
|
||||
|
||||
// 3. Set targetEntry's step to targetStep.
|
||||
target_entry->set_step(*target_step);
|
||||
|
||||
// 4. Append targetEntry to targetEntries.
|
||||
target_entries.append(target_entry);
|
||||
} else {
|
||||
// 1. Replace entryToReplace with targetEntry in targetEntries.
|
||||
*(target_entries.find(*entry_to_replace)) = target_entry;
|
||||
|
||||
// 2. Set targetEntry's step to entryToReplace's step.
|
||||
target_entry->set_step(entry_to_replace->step());
|
||||
|
||||
// 3. Set targetStep to traversable's current session history step.
|
||||
target_step = traversable->current_session_history_step();
|
||||
}
|
||||
|
||||
// 6. Apply the push/replace history step targetStep to traversable given historyHandling and userInvolvement.
|
||||
traversable->apply_the_push_or_replace_history_step(*target_step, history_handling, user_involvement, TraversableNavigable::SynchronousNavigation::Yes, nullptr, on_complete);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -76,6 +76,7 @@ public:
|
|||
Yes,
|
||||
No,
|
||||
};
|
||||
[[nodiscard]] bool try_to_synchronously_commit_same_document_navigation(GC::Ref<Navigable>, NonnullRefPtr<SessionHistoryEntry>, RefPtr<SessionHistoryEntry> entry_to_replace);
|
||||
void apply_the_push_or_replace_history_step(int step, HistoryHandlingBehavior history_handling, UserNavigationInvolvement, SynchronousNavigation, GC::Ptr<DOM::Document> pending_document, GC::Ref<OnApplyHistoryStepComplete> on_complete);
|
||||
void update_for_navigable_creation_or_destruction(GC::Ref<OnApplyHistoryStepComplete> on_complete);
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
PASS
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
<!DOCTYPE html>
|
||||
<script src="../include.js"></script>
|
||||
<script>
|
||||
asyncTest(async done => {
|
||||
const server = httpTestServer();
|
||||
const page2Body = `<!DOCTYPE html><script>parent.postMessage("loaded-page2", "*");<\/script>`;
|
||||
const page2Url = await server.createEcho("GET", "/location-navigate-then-push-state-page2", {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "text/html" },
|
||||
body: page2Body,
|
||||
});
|
||||
|
||||
const page1Body = `<!DOCTYPE html>
|
||||
<script>
|
||||
location.href = "${page2Url}";
|
||||
history.pushState({}, "", "?same-document");
|
||||
<\/script>`;
|
||||
const page1Url = await server.createEcho("GET", "/location-navigate-then-push-state-page1", {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "text/html" },
|
||||
body: page1Body,
|
||||
});
|
||||
|
||||
const iframe = document.createElement("iframe");
|
||||
document.body.appendChild(iframe);
|
||||
const result = await new Promise(resolve => {
|
||||
window.addEventListener("message", event => {
|
||||
if (event.source === iframe.contentWindow && event.data === "loaded-page2")
|
||||
resolve("PASS");
|
||||
});
|
||||
iframe.src = page1Url;
|
||||
});
|
||||
iframe.remove();
|
||||
println(result);
|
||||
done();
|
||||
});
|
||||
</script>
|
||||
|
|
@ -32,7 +32,6 @@
|
|||
asyncTest(async done => {
|
||||
history.replaceState({ replaced: true }, "", "?same-document-back-replaced");
|
||||
history.pushState({ pushed: true }, "", "?same-document-back-pushed");
|
||||
await internals.flushSessionHistoryTraversalQueue();
|
||||
|
||||
const initialUIHistory = currentUIHistory();
|
||||
if (!internals.dumpSessionHistory().includes("ui-process-session-history-same-document-back.html?same-document-back-pushed (current)")
|
||||
|
|
|
|||
|
|
@ -28,7 +28,6 @@
|
|||
asyncTest(async done => {
|
||||
history.replaceState({ replaced: true }, "", "?same-document-replaced");
|
||||
history.pushState({ pushed: true }, "", "?same-document-pushed");
|
||||
await internals.flushSessionHistoryTraversalQueue();
|
||||
|
||||
const uiHistory = currentUIHistory();
|
||||
if (!internals.dumpSessionHistory().includes("ui-process-session-history-same-document.html?same-document-pushed (current)")
|
||||
|
|
|
|||
Loading…
Reference in a new issue