Skip to content

Commit

Permalink
LibWeb: Change backup imcumbent stack to hold Realm instead of Settings
Browse files Browse the repository at this point in the history
This is a bit of a chonkier commit as it results in both:

clean_up_after_running_callback and prepare_to_run_callback being
changed to accept a realm instead of an environment settings object,
which has a bunch of fallout, particuarly for IDL abstract operations.
  • Loading branch information
shannonbooth committed Oct 30, 2024
1 parent b5e5b02 commit 101aa4e
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 117 deletions.
15 changes: 8 additions & 7 deletions Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,15 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
};

// 8.1.5.4.1 HostCallJobCallback(callback, V, argumentsList), https://html.spec.whatwg.org/multipage/webappapis.html#hostcalljobcallback
// https://whatpr.org/html/9893/webappapis.html#hostcalljobcallback
s_main_thread_vm->host_call_job_callback = [](JS::JobCallback& callback, JS::Value this_value, ReadonlySpan<JS::Value> arguments_list) {
auto& callback_host_defined = verify_cast<WebEngineCustomJobCallbackData>(*callback.custom_data());

// 1. Let incumbent settings be callback.[[HostDefined]].[[IncumbentSettings]]. (NOTE: Not necessary)
// 1. Let incumbent realm be callback.[[HostDefined]].[[IncumbentRealm]]. (NOTE: Not necessary)
// 2. Let script execution context be callback.[[HostDefined]].[[ActiveScriptContext]]. (NOTE: Not necessary)

// 3. Prepare to run a callback with incumbent settings.
callback_host_defined.incumbent_settings->prepare_to_run_callback();
// 3. Prepare to run a callback with incumbent realm.
HTML::prepare_to_run_callback(callback_host_defined.incumbent_settings->realm());

// 4. If script execution context is not null, then push script execution context onto the JavaScript execution context stack.
if (callback_host_defined.active_script_context)
Expand All @@ -221,8 +222,8 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
s_main_thread_vm->pop_execution_context();
}

// 7. Clean up after running a callback with incumbent settings.
callback_host_defined.incumbent_settings->clean_up_after_running_callback();
// 7. Clean up after running a callback with incumbent realm.
HTML::clean_up_after_running_callback(callback_host_defined.incumbent_settings->realm());

// 8. Return result.
return result;
Expand Down Expand Up @@ -291,7 +292,7 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
// IMPLEMENTATION DEFINED: Additionally to preparing to run a script, we also prepare to run a callback here. This matches WebIDL's
// invoke_callback() / call_user_object_operation() functions, and prevents a crash in host_make_job_callback()
// when getting the incumbent settings object.
job_settings->prepare_to_run_callback();
HTML::prepare_to_run_callback(*realm);

// IMPLEMENTATION DEFINED: Per the previous "implementation defined" comment, we must now make the script or module the active script or module.
// Since the only active execution context currently is the realm execution context of job settings, lets attach it here.
Expand All @@ -314,7 +315,7 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
job_settings->realm_execution_context().script_or_module = Empty {};

// IMPLEMENTATION DEFINED: See comment above, we need to clean up the non-standard prepare_to_run_callback() call.
job_settings->clean_up_after_running_callback();
HTML::clean_up_after_running_callback(*realm);

HTML::clean_up_after_running_script(*realm);
} else {
Expand Down
14 changes: 7 additions & 7 deletions Userland/Libraries/LibWeb/FileAPI/Blob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,17 +335,17 @@ JS::NonnullGCPtr<Streams::ReadableStream> Blob::get_stream()

// 2. Queue a global task on the file reading task source given blob’s relevant global object to perform the following steps:
HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), JS::create_heap_function(heap(), [stream, bytes = move(bytes)]() {
// NOTE: Using an TemporaryExecutionContext here results in a crash in the method HTML::incumbent_settings_object()
// NOTE: Using an TemporaryExecutionContext here results in a crash in the method HTML::incumbent_realm()
// since we end up in a state where we have no execution context + an event loop with an empty incumbent
// settings object stack. We still need an execution context therefore we push the realm's execution context
// onto the realm's VM, and we need an incumbent settings object which is pushed onto the incumbent settings
// object stack by EnvironmentSettings::prepare_to_run_callback().
// realm stack. We still need an execution context therefore we push the realm's execution context
// onto the realm's VM, and we need an incumbent realm which is pushed onto the incumbent realm stack
// by HTML::prepare_to_run_callback().
auto& realm = stream->realm();
auto& environment_settings = Bindings::host_defined_environment_settings_object(realm);
realm.vm().push_execution_context(environment_settings.realm_execution_context());
environment_settings.prepare_to_run_callback();
ScopeGuard const guard = [&environment_settings, &realm] {
environment_settings.clean_up_after_running_callback();
HTML::prepare_to_run_callback(realm);
ScopeGuard const guard = [&realm] {
HTML::clean_up_after_running_callback(realm);
realm.vm().pop_execution_context();
};

Expand Down
14 changes: 7 additions & 7 deletions Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void EventLoop::visit_edges(Visitor& visitor)
visitor.visit(m_task_queue);
visitor.visit(m_microtask_queue);
visitor.visit(m_currently_running_task);
visitor.visit(m_backup_incumbent_settings_object_stack);
visitor.visit(m_backup_incumbent_realm_stack);
visitor.visit(m_rendering_task_function);
}

Expand Down Expand Up @@ -537,19 +537,19 @@ void EventLoop::unregister_document(Badge<DOM::Document>, DOM::Document& documen
VERIFY(did_remove);
}

void EventLoop::push_onto_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject& environment_settings_object)
void EventLoop::push_onto_backup_incumbent_realm_stack(JS::Realm& realm)
{
m_backup_incumbent_settings_object_stack.append(environment_settings_object);
m_backup_incumbent_realm_stack.append(realm);
}

void EventLoop::pop_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>)
void EventLoop::pop_backup_incumbent_realm_stack()
{
m_backup_incumbent_settings_object_stack.take_last();
m_backup_incumbent_realm_stack.take_last();
}

EnvironmentSettingsObject& EventLoop::top_of_backup_incumbent_settings_object_stack()
JS::Realm& EventLoop::top_of_backup_incumbent_realm_stack()
{
return m_backup_incumbent_settings_object_stack.last();
return m_backup_incumbent_realm_stack.last();
}

void EventLoop::register_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject& environment_settings_object)
Expand Down
11 changes: 6 additions & 5 deletions Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ class EventLoop : public JS::Cell {

Vector<JS::Handle<HTML::Window>> same_loop_windows() const;

void push_onto_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject& environment_settings_object);
void pop_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>);
EnvironmentSettingsObject& top_of_backup_incumbent_settings_object_stack();
bool is_backup_incumbent_settings_object_stack_empty() const { return m_backup_incumbent_settings_object_stack.is_empty(); }
void push_onto_backup_incumbent_realm_stack(JS::Realm&);
void pop_backup_incumbent_realm_stack();
JS::Realm& top_of_backup_incumbent_realm_stack();
bool is_backup_incumbent_realm_stack_empty() const { return m_backup_incumbent_realm_stack.is_empty(); }

void register_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject&);
void unregister_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject&);
Expand Down Expand Up @@ -108,7 +108,8 @@ class EventLoop : public JS::Cell {
Vector<RawPtr<EnvironmentSettingsObject>> m_related_environment_settings_objects;

// https://html.spec.whatwg.org/multipage/webappapis.html#backup-incumbent-settings-object-stack
Vector<JS::NonnullGCPtr<EnvironmentSettingsObject>> m_backup_incumbent_settings_object_stack;
// https://whatpr.org/html/9893/webappapis.html#backup-incumbent-realm-stack
Vector<JS::NonnullGCPtr<JS::Realm>> m_backup_incumbent_realm_stack;

// https://html.spec.whatwg.org/multipage/browsing-the-web.html#termination-nesting-level
size_t m_termination_nesting_level { 0 };
Expand Down
48 changes: 26 additions & 22 deletions Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ static JS::ExecutionContext* top_most_script_having_execution_context(JS::VM& vm
}

// https://html.spec.whatwg.org/multipage/webappapis.html#prepare-to-run-a-callback
void EnvironmentSettingsObject::prepare_to_run_callback()
void prepare_to_run_callback(JS::Realm& realm)
{
auto& vm = global_object().vm();
auto& vm = realm.global_object().vm();

// 1. Push settings onto the backup incumbent settings object stack.
// 1. Push realm onto the backup incumbent settings object stack.
// NOTE: The spec doesn't say which event loop's stack to put this on. However, all the examples of the incumbent settings object use iframes and cross browsing context communication to demonstrate the concept.
// This means that it must rely on some global state that can be accessed by all browsing contexts, which is the main thread event loop.
HTML::main_thread_event_loop().push_onto_backup_incumbent_settings_object_stack({}, *this);
HTML::main_thread_event_loop().push_onto_backup_incumbent_realm_stack(realm);

// 2. Let context be the topmost script-having execution context.
auto* context = top_most_script_having_execution_context(vm);
Expand All @@ -209,9 +209,10 @@ URL::URL EnvironmentSettingsObject::parse_url(StringView url)
}

// https://html.spec.whatwg.org/multipage/webappapis.html#clean-up-after-running-a-callback
void EnvironmentSettingsObject::clean_up_after_running_callback()
// https://whatpr.org/html/9893/b8ea975...df5706b/webappapis.html#clean-up-after-running-a-callback
void clean_up_after_running_callback(JS::Realm const& realm)
{
auto& vm = global_object().vm();
auto& vm = realm.global_object().vm();

// 1. Let context be the topmost script-having execution context.
auto* context = top_most_script_having_execution_context(vm);
Expand All @@ -220,12 +221,12 @@ void EnvironmentSettingsObject::clean_up_after_running_callback()
if (context)
context->skip_when_determining_incumbent_counter--;

// 3. Assert: the topmost entry of the backup incumbent settings object stack is settings.
// 3. Assert: the topmost entry of the backup incumbent realm stack is realm.
auto& event_loop = HTML::main_thread_event_loop();
VERIFY(&event_loop.top_of_backup_incumbent_settings_object_stack() == this);
VERIFY(&event_loop.top_of_backup_incumbent_realm_stack() == &realm);

// 4. Remove settings from the backup incumbent settings object stack.
event_loop.pop_backup_incumbent_settings_object_stack({});
// 4. Remove realm from the backup incumbent realm stack.
event_loop.pop_backup_incumbent_realm_stack();
}

// https://html.spec.whatwg.org/multipage/webappapis.html#concept-environment-script
Expand Down Expand Up @@ -285,8 +286,9 @@ void EnvironmentSettingsObject::disallow_further_import_maps()
verify_cast<Window>(global).set_import_maps_allowed(false);
}

// https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object
EnvironmentSettingsObject& incumbent_settings_object()
// https://html.spec.whatwg.org/multipage/webappapis.html#concept-incumbent-realm
// https://whatpr.org/html/9893/b8ea975...df5706b/webappapis.html#concept-incumbent-realm
JS::Realm& incumbent_realm()
{
auto& event_loop = HTML::main_thread_event_loop();
auto& vm = event_loop.vm();
Expand All @@ -297,22 +299,24 @@ EnvironmentSettingsObject& incumbent_settings_object()
// 2. If context is null, or if context's skip-when-determining-incumbent counter is greater than zero, then:
if (!context || context->skip_when_determining_incumbent_counter > 0) {
// 1. Assert: the backup incumbent settings object stack is not empty.
// NOTE: If this assertion fails, it's because the incumbent settings object was used with no involvement of JavaScript.
VERIFY(!event_loop.is_backup_incumbent_settings_object_stack_empty());
// 1. Assert: the backup incumbent realm stack is not empty.
// NOTE: If this assertion fails, it's because the incumbent realm was used with no involvement of JavaScript.
VERIFY(!event_loop.is_backup_incumbent_realm_stack_empty());

// 2. Return the topmost entry of the backup incumbent settings object stack.
return event_loop.top_of_backup_incumbent_settings_object_stack();
// 2. Return the topmost entry of the backup incumbent realm stack.
return event_loop.top_of_backup_incumbent_realm_stack();
}

// 3. Return context's Realm component's settings object.
return Bindings::host_defined_environment_settings_object(*context->realm);
// 3. Return context's Realm component.
return *context->realm;
}

// https://html.spec.whatwg.org/multipage/webappapis.html#concept-incumbent-realm
JS::Realm& incumbent_realm()
// https://html.spec.whatwg.org/multipage/webappapis.html#incumbent-settings-object
// https://whatpr.org/html/9893/b8ea975...df5706b/webappapis.html#incumbent-settings-object
EnvironmentSettingsObject& incumbent_settings_object()
{
// Then, the incumbent Realm is the Realm of the incumbent settings object.
return incumbent_settings_object().realm();
// FIXME: Then, the incumbent settings object is the incumbent realm's principal realm settings object.
return Bindings::host_defined_environment_settings_object(incumbent_realm());
}

// https://html.spec.whatwg.org/multipage/webappapis.html#concept-incumbent-global
Expand Down
5 changes: 2 additions & 3 deletions Userland/Libraries/LibWeb/HTML/Scripting/Environments.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ struct EnvironmentSettingsObject : public Environment {
// https://fetch.spec.whatwg.org/#concept-fetch-group
Vector<JS::NonnullGCPtr<Fetch::Infrastructure::FetchRecord>>& fetch_group() { return m_fetch_group; }

void prepare_to_run_callback();
void clean_up_after_running_callback();

bool module_type_allowed(StringView module_type) const;

void disallow_further_import_maps();
Expand Down Expand Up @@ -142,6 +139,8 @@ bool is_scripting_enabled(JS::Realm const&);
bool is_scripting_disabled(JS::Realm const&);
void prepare_to_run_script(JS::Realm&);
void clean_up_after_running_script(JS::Realm const&);
void prepare_to_run_callback(JS::Realm&);
void clean_up_after_running_callback(JS::Realm const&);

EnvironmentSettingsObject& incumbent_settings_object();
JS::Realm& incumbent_realm();
Expand Down
5 changes: 3 additions & 2 deletions Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ void fetch_descendants_of_and_link_a_module_script(JS::Realm& realm,
// resulting in the event loop hanging forever awaiting for the script to be ready for parser
// execution.
realm.vm().push_execution_context(fetch_client.realm_execution_context());
fetch_client.prepare_to_run_callback();
prepare_to_run_callback(realm);

// 5. Let loadingPromise be record.LoadRequestedModules(state).
auto& loading_promise = record->load_requested_modules(state);
Expand Down Expand Up @@ -995,7 +995,8 @@ void fetch_descendants_of_and_link_a_module_script(JS::Realm& realm,
return JS::js_undefined();
}));

fetch_client.clean_up_after_running_callback();
clean_up_after_running_callback(realm);

realm.vm().pop_execution_context();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ TemporaryExecutionContext::TemporaryExecutionContext(EnvironmentSettingsObject&
{
prepare_to_run_script(m_environment_settings->realm());
if (m_callbacks_enabled == CallbacksEnabled::Yes)
m_environment_settings->prepare_to_run_callback();
prepare_to_run_callback(m_environment_settings->realm());
}

TemporaryExecutionContext::~TemporaryExecutionContext()
{
clean_up_after_running_script(m_environment_settings->realm());
if (m_callbacks_enabled == CallbacksEnabled::Yes)
m_environment_settings->clean_up_after_running_callback();
clean_up_after_running_callback(m_environment_settings->realm());
}

}
4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ static JS::ThrowCompletionOr<JS::Value> execute_a_function_body(HTML::BrowsingCo
HTML::prepare_to_run_script(realm);

// 7. Prepare to run a callback with environment settings.
environment_settings.prepare_to_run_callback();
HTML::prepare_to_run_callback(realm);

// 8. Let function be the result of calling FunctionCreate, with arguments:
// kind
Expand All @@ -293,7 +293,7 @@ static JS::ThrowCompletionOr<JS::Value> execute_a_function_body(HTML::BrowsingCo
auto completion = function->internal_call(window, move(parameters));

// 10. Clean up after running a callback with environment settings.
environment_settings.clean_up_after_running_callback();
HTML::clean_up_after_running_callback(realm);

// 11. Clean up after running a script with realm.
HTML::clean_up_after_running_script(realm);
Expand Down
Loading

0 comments on commit 101aa4e

Please sign in to comment.