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

LibWeb: Prepare to run callback in host_enqueue_promise_job()

Browse source 
...and clean up afterwards, of course. 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.

Running the following JS no longer crashes after this change:

```js
new Promise((resolve) => {
    setTimeout(resolve, 0);
}).then(() => {
    return Promise.reject();
});
```

See further discussion/investigation here:
https://discord.com/channels/830522505605283862/830525031720943627/995019647214694511
https://discord.com/channels/830522505605283862/830525031720943627/1026824624358576158
https://discord.com/channels/830522505605283862/830525031720943627/1026922985581457458

Many thanks to Luke for doing the hard work here, tracking this down,
and suggesting the fix!

Co-authored-by: Luke Wilde <lukew@serenityos.org>
This commit is contained in:
Linus Groh 2022-10-04 21:00:08 +01:00
parent 2af028132a
commit 25909dcc05
1 changed files with 8 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp
View file

@ -234,6 +234,11 @@ JS::VM& main_thread_vm()
// 2. If job settings is not null, then prepare to run script with job settings.
job_settings->prepare_to_run_script();
// 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();
// 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.
job_settings->realm_execution_context().script_or_module = script_or_module;
@ -257,6 +262,9 @@ JS::VM& main_thread_vm()
// IMPLEMENTATION DEFINED: Disassociate the realm execution context from the script or module.
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();
job_settings->clean_up_after_running_script();
} else {
// Pop off the dummy execution context. See the above FIXME block about why this is done.