0ct0pu5/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

LibWeb: Fix always hanging Navigable::reload()

Browse source 
See spec issue https://github.com/whatwg/html/issues/9869

Previous attempt on fixing reload had to be reverted because it broke
Soundcloud and GitHub, but this change does not seem to introduce new
crashes.
This commit is contained in:
Aliaksandr Kalenik 2024-04-12 20:13:54 +02:00 • committed by Alexander Kalenik
parent f7e57881ad
commit 91377f3ab9
Notes: sideshowbarker 2024-07-17 08:42:05 +09:00
10 changed files with 162 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
Tests/LibWeb/Text/data/iframe-reload.html Normal file
View file

@ -0,0 +1,12 @@
<script>
window.addEventListener('message', event => {
if (event.data && event.data.action === 'reload') {
window.parent.postMessage({ action: 'acknowledge-asked-to-reload' });
location.reload();
}
});
window.addEventListener('load', () => {
window.parent.postMessage({ action: 'loaded' });
});
</script>

3
Tests/LibWeb/Text/expected/navigation/location-reload-fetch.txt Normal file
View file

@ -0,0 +1,3 @@
iframe is loaded
iframe is going to reload
iframe is loaded

3
Tests/LibWeb/Text/expected/navigation/location-reload-srcdoc.txt Normal file
View file

@ -0,0 +1,3 @@
iframe is loaded
iframe is going to reload
iframe is loaded

29
Tests/LibWeb/Text/input/navigation/location-reload-fetch.html Normal file
View file

@ -0,0 +1,29 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<script>
let reloaded = false;
window.addEventListener('message', event => {
switch (event.data.action) {
case "loaded":
println("iframe is loaded");
if (!reloaded) {
event.source.postMessage({ action: 'reload' });
reloaded = true;
} else {
internals.signalTextTestIsDone();
}
break;
case "acknowledge-asked-to-reload":
println("iframe is going to reload");
break;
default:
break;
}
});
document.addEventListener("DOMContentLoaded", () => {
const iframe = document.createElement('iframe');
iframe.src = "../../data/iframe-reload.html"
document.body.appendChild(iframe);
});
</script>

41
Tests/LibWeb/Text/input/navigation/location-reload-srcdoc.html Normal file
View file

@ -0,0 +1,41 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<script>
let reloaded = false;
window.addEventListener('message', event => {
switch (event.data.action) {
case "loaded":
println("iframe is loaded");
if (!reloaded) {
event.source.postMessage({ action: 'reload' });
reloaded = true;
} else {
internals.signalTextTestIsDone();
}
break;
case "acknowledge-asked-to-reload":
println("iframe is going to reload");
break;
default:
break;
}
});
const iframeScript = `
window.addEventListener('message', event => {
if (event.data && event.data.action === 'reload') {
window.parent.postMessage({ action: 'acknowledge-asked-to-reload' });
location.reload();
}
});
window.addEventListener('load', () => {
window.parent.postMessage({ action: 'loaded' });
});
`;
document.addEventListener("DOMContentLoaded", () => {
const iframe = document.createElement('iframe');
iframe.srcdoc = "<script>" + iframeScript + "<\/script>";
document.body.appendChild(iframe);
});
</script>

18
Userland/Libraries/LibWeb/HTML/DocumentState.cpp
View file

@ -16,6 +16,24 @@ DocumentState::DocumentState() = default;
DocumentState::~DocumentState() = default;
JS::NonnullGCPtr<DocumentState> DocumentState::clone() const
{
JS::NonnullGCPtr<DocumentState> cloned = *heap().allocate_without_realm<DocumentState>();
cloned->m_document = m_document;
cloned->m_history_policy_container = m_history_policy_container;
cloned->m_request_referrer = m_request_referrer;
cloned->m_request_referrer_policy = m_request_referrer_policy;
cloned->m_initiator_origin = m_initiator_origin;
cloned->m_origin = m_origin;
cloned->m_about_base_url = m_about_base_url;
cloned->m_nested_histories = m_nested_histories;
cloned->m_resource = m_resource;
cloned->m_reload_pending = m_reload_pending;
cloned->m_ever_populated = m_ever_populated;
cloned->m_navigable_target_name = m_navigable_target_name;
return cloned;
}
void DocumentState::visit_edges(Cell::Visitor& visitor)
{
Base::visit_edges(visitor);

2
Userland/Libraries/LibWeb/HTML/DocumentState.h
View file

@ -31,6 +31,8 @@ public:
virtual ~DocumentState();
JS::NonnullGCPtr<DocumentState> clone() const;
enum class Client {
Tag,
};

17
Userland/Libraries/LibWeb/HTML/SessionHistoryEntry.cpp
View file

@ -28,6 +28,23 @@ SessionHistoryEntry::SessionHistoryEntry()
{
}
JS::NonnullGCPtr<SessionHistoryEntry> SessionHistoryEntry::clone() const
{
JS::NonnullGCPtr<SessionHistoryEntry> entry = *heap().allocate_without_realm<SessionHistoryEntry>();
entry->m_step = m_step;
entry->m_url = m_url;
entry->m_document_state = m_document_state->clone();
entry->m_classic_history_api_state = m_classic_history_api_state;
entry->m_navigation_api_state = m_navigation_api_state;
entry->m_navigation_api_key = m_navigation_api_key;
entry->m_navigation_api_id = m_navigation_api_id;
entry->m_scroll_restoration_mode = m_scroll_restoration_mode;
entry->m_policy_container = m_policy_container;
entry->m_browsing_context_name = m_browsing_context_name;
entry->m_original_source_browsing_context = m_original_source_browsing_context;
return entry;
}
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#she-document
JS::GCPtr<DOM::Document> SessionHistoryEntry::document() const
{

2
Userland/Libraries/LibWeb/HTML/SessionHistoryEntry.h
View file

@ -35,6 +35,8 @@ class SessionHistoryEntry final : public JS::Cell {
public:
SessionHistoryEntry();
JS::NonnullGCPtr<SessionHistoryEntry> clone() const;
void visit_edges(Cell::Visitor&) override;
JS::GCPtr<DOM::Document> document() const;

46
Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp
View file

@ -447,6 +447,9 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
JS::Handle<SessionHistoryEntry> target_entry;
JS::Handle<Navigable> navigable;
bool update_only = false;
JS::Handle<SessionHistoryEntry> populated_target_entry;
bool populated_cloned_target_session_history_entry = false;
};
// 11. Let changingNavigableContinuations be an empty queue of changing navigable continuation states.
@ -473,7 +476,9 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
.displayed_document = displayed_entry->document(),
.target_entry = target_entry,
.navigable = navigable,
.update_only = false
.update_only = false,
.populated_target_entry = {},
.populated_cloned_target_session_history_entry = false,
};
// 4. If displayedEntry is targetEntry and targetEntry's document state's reload pending is false, then:
@ -519,7 +524,10 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
// and oldOrigin is the same as navigable's current session history entry's document state's origin,
// then fire a traverse navigate event given targetEntry and userInvolvementForNavigateEvents.
auto after_document_populated = [old_origin, target_entry, changing_navigable_continuation, &changing_navigable_continuations, &vm, &navigable]() mutable {
auto after_document_populated = [old_origin, changing_navigable_continuation, &changing_navigable_continuations, &vm, &navigable](bool populated_cloned_target_she, JS::NonnullGCPtr<SessionHistoryEntry> target_entry) mutable {
changing_navigable_continuation.populated_target_entry = target_entry;
changing_navigable_continuation.populated_cloned_target_session_history_entry = populated_cloned_target_she;
// 1. If targetEntry's document is null, then set changingNavigableContinuation's update-only to true.
if (!target_entry->document()) {
changing_navigable_continuation.update_only = true;
@ -568,13 +576,21 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
// 6. Let allowPOST be targetEntry's document state's reload pending.
auto allow_POST = target_entry->document_state()->reload_pending();
// https://github.com/whatwg/html/issues/9869
// Reloading requires population of the active session history entry, making it inactive.
// This results in a situation where tasks that unload the previous document and activate a new
// document cannot run. To resolve this, the target entry is cloned before it is populated.
// After the unloading of the previous document is completed, all fields potentially affected by the
// population are copied from the cloned target entry to the actual target entry.
auto populated_target_entry = target_entry->clone();
// 7. In parallel, attempt to populate the history entry's document for targetEntry, given navigable, potentiallyTargetSpecificSourceSnapshotParams,
// targetSnapshotParams, with allowPOST set to allowPOST and completionSteps set to queue a global task on the navigation and traversal task source given
// navigable's active window to run afterDocumentPopulated.
Platform::EventLoopPlugin::the().deferred_invoke([target_entry, potentially_target_specific_source_snapshot_params, target_snapshot_params, this, allow_POST, navigable, after_document_populated] {
navigable->populate_session_history_entry_document(target_entry, *potentially_target_specific_source_snapshot_params, target_snapshot_params, {}, Empty {}, CSPNavigationType::Other, allow_POST, [this, after_document_populated]() mutable {
queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), [after_document_populated]() mutable {
after_document_populated();
Platform::EventLoopPlugin::the().deferred_invoke([populated_target_entry, potentially_target_specific_source_snapshot_params, target_snapshot_params, this, allow_POST, navigable, after_document_populated] {
navigable->populate_session_history_entry_document(populated_target_entry, *potentially_target_specific_source_snapshot_params, target_snapshot_params, {}, Empty {}, CSPNavigationType::Other, allow_POST, [this, after_document_populated, populated_target_entry]() mutable {
queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), [after_document_populated, populated_target_entry]() mutable {
after_document_populated(true, populated_target_entry);
});
})
.release_value_but_fixme_should_propagate_errors();
@ -582,7 +598,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
}
// Otherwise, run afterDocumentPopulated immediately.
else {
after_document_populated();
after_document_populated(false, *target_entry);
}
});
}
@ -638,7 +654,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
auto displayed_document = changing_navigable_continuation.displayed_document;
// 5. Let targetEntry be changingNavigableContinuation's target entry.
auto target_entry = changing_navigable_continuation.target_entry;
auto& populated_target_entry = changing_navigable_continuation.populated_target_entry;
// 6. Let navigable be changingNavigableContinuation's navigable.
auto navigable = changing_navigable_continuation.navigable;
@ -659,7 +675,15 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
auto entries_for_navigation_api = get_session_history_entries_for_the_navigation_api(*navigable, target_step);
// 12. In both cases, let afterPotentialUnloads be the following steps:
auto after_potential_unload = JS::SafeFunction<void()>([changing_navigable_continuation, target_entry, displayed_document, &completed_change_jobs, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api)] {
auto after_potential_unload = JS::SafeFunction<void()>([changing_navigable_continuation, displayed_document, &completed_change_jobs, script_history_length, script_history_index, entries_for_navigation_api = move(entries_for_navigation_api)] {
auto const& target_entry = changing_navigable_continuation.target_entry;
if (changing_navigable_continuation.populated_cloned_target_session_history_entry) {
auto const& populating_target_entry = changing_navigable_continuation.populated_target_entry;
target_entry->set_document_state(populating_target_entry->document_state());
target_entry->set_url(populating_target_entry->url());
target_entry->set_classic_history_api_state(populating_target_entry->classic_history_api_state());
}
// 1. If changingNavigableContinuation's update-only is false, then activate history entry targetEntry for navigable.
if (!changing_navigable_continuation.update_only)
changing_navigable_continuation.navigable->activate_history_entry(*changing_navigable_continuation.target_entry);
@ -687,7 +711,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
});
// 10. If changingNavigableContinuation's update-only is true, or targetEntry's document is displayedDocument, then:
if (changing_navigable_continuation.update_only || target_entry->document().ptr() == displayed_document.ptr()) {
if (changing_navigable_continuation.update_only || populated_target_entry->document().ptr() == displayed_document.ptr()) {
// 1. Set the ongoing navigation for navigable to null.
navigable->set_ongoing_navigation({});
@ -700,7 +724,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
VERIFY(navigation_type.has_value());
// 2. Deactivate displayedDocument, given userNavigationInvolvement, targetEntry, navigationType, and afterPotentialUnloads.
deactivate_a_document_for_cross_document_navigation(*displayed_document, user_involvement_for_navigate_events, *target_entry, move(after_potential_unload));
deactivate_a_document_for_cross_document_navigation(*displayed_document, user_involvement_for_navigate_events, *populated_target_entry, move(after_potential_unload));
}
}