Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: ShadowRealms part 4 - enough for it to not crash #1993

Merged
merged 5 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PASS: Exception thrown 'TypeError: Wrapped value must be primitive or a function object, got [object Promise]'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Shadow realm evaluation returned: 2
1 change: 1 addition & 0 deletions Tests/LibWeb/Text/expected/all-window-properties.txt
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ ServiceWorkerContainer
ServiceWorkerRegistration
Set
ShadowRealm
ShadowRealmGlobalScope
ShadowRoot
SharedArrayBuffer
SourceBuffer
Expand Down
13 changes: 13 additions & 0 deletions Tests/LibWeb/Text/input/HTML/shadow-realm-async-evaluate.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script src="../include.js"></script>
<script>
test(() => {
const realm = new ShadowRealm()
const aSync = realm.evaluate(`async () => 1 + 1`);
try {
aSync();
println('Fail! No exception thrown');
} catch (e) {
println(`PASS: Exception thrown '${e}'`);
}
})
</script>
8 changes: 8 additions & 0 deletions Tests/LibWeb/Text/input/HTML/shadow-realm-sync-evaluate.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script src="../include.js"></script>
<script>
test(() => {
const realm = new ShadowRealm();
const result = realm.evaluate(`() => 1 + 1`)();
println(`Shadow realm evaluation returned: ${result}`);
});
</script>
3 changes: 3 additions & 0 deletions Userland/Libraries/LibJS/Runtime/Realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ class Realm final : public Cell {
static ThrowCompletionOr<NonnullOwnPtr<ExecutionContext>> initialize_host_defined_realm(VM&, Function<Object*(Realm&)> create_global_object, Function<Object*(Realm&)> create_global_this_value);

[[nodiscard]] Object& global_object() const { return *m_global_object; }
void set_global_object(NonnullGCPtr<Object> global) { m_global_object = global; }

[[nodiscard]] GlobalEnvironment& global_environment() const { return *m_global_environment; }
void set_global_environment(NonnullGCPtr<GlobalEnvironment> environment) { m_global_environment = environment; }

[[nodiscard]] Intrinsics const& intrinsics() const { return *m_intrinsics; }
[[nodiscard]] Intrinsics& intrinsics() { return *m_intrinsics; }
Expand Down
46 changes: 0 additions & 46 deletions Userland/Libraries/LibWeb/Bindings/HostDefined.h

This file was deleted.

4 changes: 2 additions & 2 deletions Userland/Libraries/LibWeb/Bindings/Intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <LibJS/Heap/Cell.h>
#include <LibJS/Heap/Heap.h>
#include <LibJS/Runtime/VM.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/Bindings/PrincipalHostDefined.h>

#define WEB_SET_PROTOTYPE_FOR_INTERFACE_WITH_CUSTOM_NAME(interface_class, interface_name) \
do { \
Expand Down Expand Up @@ -84,7 +84,7 @@ class Intrinsics final : public JS::Cell {

[[nodiscard]] inline Intrinsics& host_defined_intrinsics(JS::Realm& realm)
{
return *verify_cast<HostDefined>(realm.host_defined())->intrinsics;
return *verify_cast<PrincipalHostDefined>(realm.host_defined())->intrinsics;
}

template<typename T>
Expand Down
44 changes: 44 additions & 0 deletions Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@
#include <LibJS/Runtime/Array.h>
#include <LibJS/Runtime/Environment.h>
#include <LibJS/Runtime/FinalizationRegistry.h>
#include <LibJS/Runtime/GlobalEnvironment.h>
#include <LibJS/Runtime/ModuleRequest.h>
#include <LibJS/Runtime/NativeFunction.h>
#include <LibJS/Runtime/ShadowRealm.h>
#include <LibJS/Runtime/VM.h>
#include <LibJS/SourceTextModule.h>
#include <LibWeb/Bindings/ExceptionOrUtils.h>
#include <LibWeb/Bindings/Intrinsics.h>
#include <LibWeb/Bindings/MainThreadVM.h>
#include <LibWeb/Bindings/SyntheticHostDefined.h>
#include <LibWeb/Bindings/WindowExposedInterfaces.h>
#include <LibWeb/DOM/Document.h>
#include <LibWeb/DOM/MutationType.h>
Expand All @@ -35,7 +38,9 @@
#include <LibWeb/HTML/Scripting/Fetching.h>
#include <LibWeb/HTML/Scripting/ModuleScript.h>
#include <LibWeb/HTML/Scripting/Script.h>
#include <LibWeb/HTML/Scripting/SyntheticRealmSettings.h>
#include <LibWeb/HTML/Scripting/TemporaryExecutionContext.h>
#include <LibWeb/HTML/ShadowRealmGlobalScope.h>
#include <LibWeb/HTML/TagNames.h>
#include <LibWeb/HTML/Window.h>
#include <LibWeb/HTML/WindowProxy.h>
Expand Down Expand Up @@ -560,6 +565,45 @@ ErrorOr<void> initialize_main_thread_vm(HTML::EventLoop::Type type)
HTML::fetch_single_imported_module_script(*module_map_realm, url.release_value(), *fetch_client, destination, fetch_options, *module_map_realm, fetch_referrer, module_request, perform_fetch, on_single_fetch_complete);
};

// https://whatpr.org/html/9893/webappapis.html#hostinitializeshadowrealm(realm,-context,-o)
// 8.1.6.8 HostInitializeShadowRealm(realm, context, O)
s_main_thread_vm->host_initialize_shadow_realm = [](JS::Realm& realm, NonnullOwnPtr<JS::ExecutionContext> context, JS::ShadowRealm& object) -> JS::ThrowCompletionOr<void> {
// FIXME: 1. Set realm's is global prototype chain mutable to true.

// 2. Let globalObject be a new ShadowRealmGlobalScope object with realm.
auto global_object = HTML::ShadowRealmGlobalScope::create(realm);

// 3. Let settings be a new synthetic realm settings object that this algorithm will subsequently initialize.
auto settings = HTML::SyntheticRealmSettings {
// 4. Set settings's execution context to context.
.execution_context = move(context),

// 5. Set settings's principal realm to O's associated realm
.principal_realm = object.shape().realm(),

// 6. Set settings's underlying realm to realm.
.underlying_realm = realm,

// 7. Set settings's module map to a new module map, initially empty.
.module_map = realm.heap().allocate<HTML::ModuleMap>(realm),
};

// 8. Set realm.[[HostDefined]] to settings.
realm.set_host_defined(make<Bindings::SyntheticHostDefined>(move(settings)));

// 9. Set realm.[[GlobalObject]] to globalObject.
realm.set_global_object(global_object);

// 10. Set realm.[[GlobalEnv]] to NewGlobalEnvironment(globalObject, globalObject).
realm.set_global_environment(realm.vm().heap().allocate_without_realm<JS::GlobalEnvironment>(global_object, global_object));

// 11. Perform ? SetDefaultGlobalBindings(realm).
set_default_global_bindings(realm);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should call ShadowRealmGlobalScopeGlobalMixin::initialize(realm, *this); here, as well as add_shadow_realm_exposed_interfaces(*this) (in a helper on the global scope/global object variable)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

speaking of which, did the spec folks decide what things should be exposed to shadow realms? We'll need to go around sprinkling annotations all over our idl files for the idl generator to pick them up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, spec folks have done so - those are all of the crashing IDL tests that we have currently. Here's a meta issue that shows what has been done: tc39/proposal-shadowrealm#393

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not crashing after these changes, but still doesn't work :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah perfect, then it should be straightforward to add ShadowRealm as an option in GenerateWindowOrWorkerInterfaces ... which I guess should be renamed to GenerateGlobalExposedInterfaces or some such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, yeah that's why I've left those other FIXME's you mentioned in the initialize steps. I need to implement that universal global scope and reshuffling some implementations from windoworworkerglobal scope mixin to a new universalglobalscope mixin

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha. well, given that, I think we're good to go on this part once CI is happy. Feel free to add more fixmes if you want though.

// 12. Return NormalCompletion(unused).
return {};
};

s_main_thread_vm->host_unrecognized_date_string = [](StringView date) {
dbgln("Unable to parse date string: \"{}\"", date);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

#include <LibJS/Heap/Cell.h>
#include <LibJS/Runtime/Realm.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/Bindings/Intrinsics.h>
#include <LibWeb/Bindings/PrincipalHostDefined.h>
#include <LibWeb/HTML/Scripting/Environments.h>
#include <LibWeb/Page/Page.h>

namespace Web::Bindings {

void HostDefined::visit_edges(JS::Cell::Visitor& visitor)
void PrincipalHostDefined::visit_edges(JS::Cell::Visitor& visitor)
{
JS::Realm::HostDefined::visit_edges(visitor);
visitor.visit(environment_settings_object);
Expand Down
46 changes: 46 additions & 0 deletions Userland/Libraries/LibWeb/Bindings/PrincipalHostDefined.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2022, Andrew Kaster <akaster@serenityos.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#pragma once

#include <AK/TypeCasts.h>
#include <LibJS/Heap/GCPtr.h>
#include <LibJS/Runtime/Realm.h>
#include <LibWeb/Forward.h>

namespace Web::Bindings {

struct PrincipalHostDefined : public JS::Realm::HostDefined {
PrincipalHostDefined(JS::NonnullGCPtr<HTML::EnvironmentSettingsObject> eso, JS::NonnullGCPtr<Intrinsics> intrinsics, JS::NonnullGCPtr<Page> page)
: environment_settings_object(eso)
, intrinsics(intrinsics)
, page(page)
{
}
virtual ~PrincipalHostDefined() override = default;
virtual void visit_edges(JS::Cell::Visitor& visitor) override;

JS::NonnullGCPtr<HTML::EnvironmentSettingsObject> environment_settings_object;
JS::NonnullGCPtr<Intrinsics> intrinsics;
JS::NonnullGCPtr<Page> page;
};

[[nodiscard]] inline HTML::EnvironmentSettingsObject& principal_host_defined_environment_settings_object(JS::Realm& realm)
{
return *verify_cast<PrincipalHostDefined>(realm.host_defined())->environment_settings_object;
}

[[nodiscard]] inline HTML::EnvironmentSettingsObject const& principal_host_defined_environment_settings_object(JS::Realm const& realm)
{
return *verify_cast<PrincipalHostDefined>(realm.host_defined())->environment_settings_object;
}

[[nodiscard]] inline Page& principal_host_defined_page(JS::Realm& realm)
{
return *verify_cast<PrincipalHostDefined>(realm.host_defined())->page;
}

}
20 changes: 20 additions & 0 deletions Userland/Libraries/LibWeb/Bindings/SyntheticHostDefined.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (c) 2024, Shannon Booth <shannon@serenityos.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <LibJS/Heap/Cell.h>
#include <LibJS/Runtime/Realm.h>
#include <LibWeb/Bindings/SyntheticHostDefined.h>
#include <LibWeb/HTML/Scripting/Environments.h>

namespace Web::Bindings {

void SyntheticHostDefined::visit_edges(JS::Cell::Visitor& visitor)
{
JS::Realm::HostDefined::visit_edges(visitor);
synthetic_realm_settings.visit_edges(visitor);
}

}
27 changes: 27 additions & 0 deletions Userland/Libraries/LibWeb/Bindings/SyntheticHostDefined.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2024, Shannon Booth <shannon@serenityos.org>
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#pragma once

#include <LibJS/Runtime/Realm.h>
#include <LibWeb/Forward.h>
#include <LibWeb/HTML/Scripting/SyntheticRealmSettings.h>

namespace Web::Bindings {

struct SyntheticHostDefined : public JS::Realm::HostDefined {
SyntheticHostDefined(HTML::SyntheticRealmSettings synthetic_realm_settings)
: synthetic_realm_settings(move(synthetic_realm_settings))
{
}

virtual ~SyntheticHostDefined() override = default;
virtual void visit_edges(JS::Cell::Visitor& visitor) override;

HTML::SyntheticRealmSettings synthetic_realm_settings;
};

}
5 changes: 4 additions & 1 deletion Userland/Libraries/LibWeb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ set(SOURCES
ARIA/RoleType.cpp
ARIA/StateAndProperties.cpp
Bindings/AudioConstructor.cpp
Bindings/HostDefined.cpp
Bindings/ImageConstructor.cpp
Bindings/Intrinsics.cpp
Bindings/LocationConstructor.cpp
Bindings/MainThreadVM.cpp
Bindings/OptionConstructor.cpp
Bindings/PlatformObject.cpp
Bindings/PrincipalHostDefined.cpp
Bindings/SyntheticHostDefined.cpp
Clipboard/Clipboard.cpp
Clipboard/ClipboardEvent.cpp
Crypto/Crypto.cpp
Expand Down Expand Up @@ -449,6 +450,7 @@ set(SOURCES
HTML/Scripting/ModuleMap.cpp
HTML/Scripting/ModuleScript.cpp
HTML/Scripting/Script.cpp
HTML/Scripting/SyntheticRealmSettings.cpp
HTML/Scripting/TemporaryExecutionContext.cpp
HTML/Scripting/WindowEnvironmentSettingsObject.cpp
HTML/Scripting/WorkerEnvironmentSettingsObject.cpp
Expand All @@ -460,6 +462,7 @@ set(SOURCES
HTML/ServiceWorkerRegistration.cpp
HTML/SessionHistoryEntry.cpp
HTML/SessionHistoryTraversalQueue.cpp
HTML/ShadowRealmGlobalScope.cpp
HTML/SharedResourceRequest.cpp
HTML/SourceSet.cpp
HTML/Storage.cpp
Expand Down
1 change: 0 additions & 1 deletion Userland/Libraries/LibWeb/Clipboard/Clipboard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <LibJS/Runtime/Realm.h>
#include <LibTextCodec/Decoder.h>
#include <LibWeb/Bindings/ClipboardPrototype.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/Clipboard/Clipboard.h>
#include <LibWeb/FileAPI/Blob.h>
#include <LibWeb/HTML/Scripting/Environments.h>
Expand Down
6 changes: 3 additions & 3 deletions Userland/Libraries/LibWeb/DOM/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ JS::NonnullGCPtr<Document> Document::create_for_fragment_parsing(JS::Realm& real

Document::Document(JS::Realm& realm, const URL::URL& url, TemporaryDocumentForFragmentParsing temporary_document_for_fragment_parsing)
: ParentNode(realm, *this, NodeType::DOCUMENT_NODE)
, m_page(Bindings::host_defined_page(realm))
, m_page(Bindings::principal_host_defined_page(realm))
, m_style_computer(make<CSS::StyleComputer>(*this))
, m_url(url)
, m_temporary_document_for_fragment_parsing(temporary_document_for_fragment_parsing)
Expand Down Expand Up @@ -1649,7 +1649,7 @@ Color Document::visited_link_color() const
HTML::EnvironmentSettingsObject& Document::relevant_settings_object() const
{
// Then, the relevant settings object for a platform object o is the environment settings object of the relevant Realm for o.
return Bindings::host_defined_environment_settings_object(realm());
return Bindings::principal_host_defined_environment_settings_object(realm());
}

// https://dom.spec.whatwg.org/#dom-document-createelement
Expand Down Expand Up @@ -4171,7 +4171,7 @@ void Document::start_intersection_observing_a_lazy_loading_element(Element& elem
// Spec Note: This allows for fetching the image during scrolling, when it does not yet — but is about to — intersect the viewport.
auto options = IntersectionObserver::IntersectionObserverInit {};

auto wrapped_callback = realm.heap().allocate_without_realm<WebIDL::CallbackType>(callback, Bindings::host_defined_environment_settings_object(realm));
auto wrapped_callback = realm.heap().allocate_without_realm<WebIDL::CallbackType>(callback, Bindings::principal_host_defined_environment_settings_object(realm));
m_lazy_load_intersection_observer = IntersectionObserver::IntersectionObserver::construct_impl(realm, wrapped_callback, options).release_value_but_fixme_should_propagate_errors();
}

Expand Down
2 changes: 1 addition & 1 deletion Userland/Libraries/LibWeb/DOM/EventTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ void EventTarget::activate_event_handler(FlyString const& name, HTML::EventHandl
0, "", &realm);

// NOTE: As per the spec, the callback context is arbitrary.
auto callback = realm.heap().allocate_without_realm<WebIDL::CallbackType>(*callback_function, Bindings::host_defined_environment_settings_object(realm));
auto callback = realm.heap().allocate_without_realm<WebIDL::CallbackType>(*callback_function, Bindings::principal_host_defined_environment_settings_object(realm));

// 5. Let listener be a new event listener whose type is the event handler event type corresponding to eventHandler and callback is callback.
auto listener = realm.heap().allocate_without_realm<DOMEventListener>();
Expand Down
1 change: 0 additions & 1 deletion Userland/Libraries/LibWeb/Fetch/Body.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include <LibJS/Runtime/TypedArray.h>
#include <LibTextCodec/Decoder.h>
#include <LibWeb/Bindings/ExceptionOrUtils.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/Bindings/MainThreadVM.h>
#include <LibWeb/DOMURL/URLSearchParams.h>
#include <LibWeb/Fetch/Body.h>
Expand Down
1 change: 0 additions & 1 deletion Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <AK/TypeCasts.h>
#include <LibJS/Runtime/PromiseCapability.h>
#include <LibWeb/Bindings/ExceptionOrUtils.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/DOM/AbortSignal.h>
#include <LibWeb/Fetch/FetchMethod.h>
#include <LibWeb/Fetch/Fetching/Fetching.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <LibJS/Heap/HeapFunction.h>
#include <LibWeb/Bindings/ExceptionOrUtils.h>
#include <LibWeb/Bindings/HostDefined.h>
#include <LibWeb/Fetch/Fetching/FetchedDataReceiver.h>
#include <LibWeb/Fetch/Infrastructure/FetchParams.h>
#include <LibWeb/Fetch/Infrastructure/Task.h>
Expand Down
Loading
Loading