Skip to content

Commit b12716e

Browse files
committed
LibWeb: Change backup imcumbent stack to hold Realm instead of Settings
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.
1 parent 6880b07 commit b12716e

File tree

10 files changed

+114
-117
lines changed

10 files changed

+114
-117
lines changed

Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp

+8-7
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,15 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
199199
};
200200

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

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

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

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

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

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

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

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

319320
HTML::clean_up_after_running_script(*realm);
320321
} else {

Userland/Libraries/LibWeb/FileAPI/Blob.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -335,17 +335,17 @@ JS::NonnullGCPtr<Streams::ReadableStream> Blob::get_stream()
335335

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

Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ void EventLoop::visit_edges(Visitor& visitor)
4242
visitor.visit(m_task_queue);
4343
visitor.visit(m_microtask_queue);
4444
visitor.visit(m_currently_running_task);
45-
visitor.visit(m_backup_incumbent_settings_object_stack);
45+
visitor.visit(m_backup_incumbent_realm_stack);
4646
}
4747

4848
void EventLoop::schedule()
@@ -529,19 +529,19 @@ void EventLoop::unregister_document(Badge<DOM::Document>, DOM::Document& documen
529529
VERIFY(did_remove);
530530
}
531531

532-
void EventLoop::push_onto_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject& environment_settings_object)
532+
void EventLoop::push_onto_backup_incumbent_realm_stack(JS::Realm& realm)
533533
{
534-
m_backup_incumbent_settings_object_stack.append(environment_settings_object);
534+
m_backup_incumbent_realm_stack.append(realm);
535535
}
536536

537-
void EventLoop::pop_backup_incumbent_settings_object_stack(Badge<EnvironmentSettingsObject>)
537+
void EventLoop::pop_backup_incumbent_realm_stack()
538538
{
539-
m_backup_incumbent_settings_object_stack.take_last();
539+
m_backup_incumbent_realm_stack.take_last();
540540
}
541541

542-
EnvironmentSettingsObject& EventLoop::top_of_backup_incumbent_settings_object_stack()
542+
JS::Realm& EventLoop::top_of_backup_incumbent_realm_stack()
543543
{
544-
return m_backup_incumbent_settings_object_stack.last();
544+
return m_backup_incumbent_realm_stack.last();
545545
}
546546

547547
void EventLoop::register_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject& environment_settings_object)

Userland/Libraries/LibWeb/HTML/EventLoop/EventLoop.h

+6-5
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ class EventLoop : public JS::Cell {
6262

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

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

7070
void register_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject&);
7171
void unregister_environment_settings_object(Badge<EnvironmentSettingsObject>, EnvironmentSettingsObject&);
@@ -106,7 +106,8 @@ class EventLoop : public JS::Cell {
106106
Vector<RawPtr<EnvironmentSettingsObject>> m_related_environment_settings_objects;
107107

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

111112
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#termination-nesting-level
112113
size_t m_termination_nesting_level { 0 };

Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp

+26-22
Original file line numberDiff line numberDiff line change
@@ -175,14 +175,14 @@ static JS::ExecutionContext* top_most_script_having_execution_context(JS::VM& vm
175175
}
176176

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

182-
// 1. Push settings onto the backup incumbent settings object stack.
182+
// 1. Push realm onto the backup incumbent settings object stack.
183183
// 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.
184184
// 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.
185-
HTML::main_thread_event_loop().push_onto_backup_incumbent_settings_object_stack({}, *this);
185+
HTML::main_thread_event_loop().push_onto_backup_incumbent_realm_stack(realm);
186186

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

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

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

223-
// 3. Assert: the topmost entry of the backup incumbent settings object stack is settings.
224+
// 3. Assert: the topmost entry of the backup incumbent realm stack is realm.
224225
auto& event_loop = HTML::main_thread_event_loop();
225-
VERIFY(&event_loop.top_of_backup_incumbent_settings_object_stack() == this);
226+
VERIFY(&event_loop.top_of_backup_incumbent_realm_stack() == &realm);
226227

227-
// 4. Remove settings from the backup incumbent settings object stack.
228-
event_loop.pop_backup_incumbent_settings_object_stack({});
228+
// 4. Remove realm from the backup incumbent realm stack.
229+
event_loop.pop_backup_incumbent_realm_stack();
229230
}
230231

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

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

303-
// 2. Return the topmost entry of the backup incumbent settings object stack.
304-
return event_loop.top_of_backup_incumbent_settings_object_stack();
306+
// 2. Return the topmost entry of the backup incumbent realm stack.
307+
return event_loop.top_of_backup_incumbent_realm_stack();
305308
}
306309

307-
// 3. Return context's Realm component's settings object.
308-
return Bindings::host_defined_environment_settings_object(*context->realm);
310+
// 3. Return context's Realm component.
311+
return *context->realm;
309312
}
310313

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

318322
// https://html.spec.whatwg.org/multipage/webappapis.html#concept-incumbent-global

Userland/Libraries/LibWeb/HTML/Scripting/Environments.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ struct EnvironmentSettingsObject : public Environment {
9696
// https://fetch.spec.whatwg.org/#concept-fetch-group
9797
Vector<JS::NonnullGCPtr<Fetch::Infrastructure::FetchRecord>>& fetch_group() { return m_fetch_group; }
9898

99-
void prepare_to_run_callback();
100-
void clean_up_after_running_callback();
101-
10299
bool module_type_allowed(StringView module_type) const;
103100

104101
void disallow_further_import_maps();
@@ -142,6 +139,8 @@ bool is_scripting_enabled(JS::Realm const&);
142139
bool is_scripting_disabled(JS::Realm const&);
143140
void prepare_to_run_script(JS::Realm&);
144141
void clean_up_after_running_script(JS::Realm const&);
142+
void prepare_to_run_callback(JS::Realm&);
143+
void clean_up_after_running_callback(JS::Realm const&);
145144

146145
EnvironmentSettingsObject& incumbent_settings_object();
147146
JS::Realm& incumbent_realm();

Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ void fetch_descendants_of_and_link_a_module_script(JS::Realm& realm,
958958
// resulting in the event loop hanging forever awaiting for the script to be ready for parser
959959
// execution.
960960
realm.vm().push_execution_context(fetch_client.realm_execution_context());
961-
fetch_client.prepare_to_run_callback();
961+
prepare_to_run_callback(fetch_client.realm());
962962

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

998-
fetch_client.clean_up_after_running_callback();
998+
clean_up_after_running_callback(realm);
999+
9991000
realm.vm().pop_execution_context();
10001001
}
10011002

Userland/Libraries/LibWeb/HTML/Scripting/TemporaryExecutionContext.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ TemporaryExecutionContext::TemporaryExecutionContext(EnvironmentSettingsObject&
1515
{
1616
HTML::prepare_to_run_script(m_environment_settings->realm());
1717
if (m_callbacks_enabled == CallbacksEnabled::Yes)
18-
m_environment_settings->prepare_to_run_callback();
18+
prepare_to_run_callback(m_environment_settings->realm());
1919
}
2020

2121
TemporaryExecutionContext::~TemporaryExecutionContext()
2222
{
2323
clean_up_after_running_script(m_environment_settings->realm());
2424
if (m_callbacks_enabled == CallbacksEnabled::Yes)
25-
m_environment_settings->clean_up_after_running_callback();
25+
clean_up_after_running_callback(m_environment_settings->realm());
2626
}
2727

2828
}

Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ static JS::ThrowCompletionOr<JS::Value> execute_a_function_body(HTML::BrowsingCo
271271
HTML::prepare_to_run_script(realm);
272272

273273
// 7. Prepare to run a callback with environment settings.
274-
environment_settings.prepare_to_run_callback();
274+
HTML::prepare_to_run_callback(realm);
275275

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

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

297297
// 11. Clean up after running a script with realm.
298298
HTML::clean_up_after_running_script(realm);

0 commit comments

Comments
 (0)