LibWeb: Make HTML::Environment a GC-allocated type

The only subclass was already GC-allocated, so let's hoist the JS::Cell
inheritance up one level. This ends up simplifying a bit of rather
dubious looking code where we were previously slicing ESOs.
This commit is contained in:
Andreas Kling 2024-04-24 11:09:36 +02:00
parent 7fa45c5fdf
commit f60d82eb85
9 changed files with 29 additions and 26 deletions

View file

@ -263,13 +263,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Document>> Document::create_and_initialize(
browsing_context->page(),
creation_url.value(),
move(realm_execution_context),
navigation_params.reserved_environment.visit(
// FIXME: Environment is virtual. We *really* shouldn't be slicing it here
[](Empty const&) -> Optional<HTML::Environment> { return {}; },
[](HTML::Environment* env) -> Optional<HTML::Environment> { if (env) return *env; return {}; },
[](JS::NonnullGCPtr<HTML::EnvironmentSettingsObject>) -> Optional<HTML::Environment> {
TODO();
}),
navigation_params.reserved_environment,
top_level_creation_url.value(),
top_level_origin);
}

View file

@ -28,9 +28,7 @@ void Request::visit_edges(JS::Cell::Visitor& visitor)
m_body.visit(
[&](JS::NonnullGCPtr<Body>& body) { visitor.visit(body); },
[](auto&) {});
m_reserved_client.visit(
[&](JS::GCPtr<HTML::EnvironmentSettingsObject> const& value) { visitor.visit(value); },
[](auto const&) {});
visitor.visit(m_reserved_client);
m_window.visit(
[&](JS::GCPtr<HTML::EnvironmentSettingsObject> const& value) { visitor.visit(value); },
[](auto const&) {});

View file

@ -165,7 +165,7 @@ public:
using OriginType = Variant<Origin, HTML::Origin>;
using PolicyContainerType = Variant<PolicyContainer, HTML::PolicyContainer>;
using ReferrerType = Variant<Referrer, URL::URL>;
using ReservedClientType = Variant<Empty, HTML::Environment*, JS::GCPtr<HTML::EnvironmentSettingsObject>>;
using ReservedClientType = JS::GCPtr<HTML::Environment>;
using WindowType = Variant<Window, JS::GCPtr<HTML::EnvironmentSettingsObject>>;
[[nodiscard]] static JS::NonnullGCPtr<Request> create(JS::VM&);

View file

@ -553,13 +553,10 @@ static CrossOriginOpenerPolicy obtain_a_cross_origin_opener_policy(JS::NonnullGC
CrossOriginOpenerPolicy policy = {};
// AD-HOC: We don't yet setup environments in all cases
if (reserved_client.has<Empty>())
if (!reserved_client)
return policy;
auto& reserved_environment = reserved_client.visit(
[](Empty const&) -> Environment& { VERIFY_NOT_REACHED(); },
[](Environment* env) -> Environment& { return *env; },
[](JS::GCPtr<EnvironmentSettingsObject> eso) -> Environment& { return *eso; });
auto& reserved_environment = *reserved_client;
// 2. If reservedEnvironment is a non-secure context, then return policy.
if (is_non_secure_context(reserved_environment))

View file

@ -7,6 +7,7 @@
#include <LibWeb/Fetch/Infrastructure/FetchController.h>
#include <LibWeb/HTML/Navigable.h>
#include <LibWeb/HTML/NavigationParams.h>
#include <LibWeb/HTML/Scripting/Environments.h>
namespace Web::HTML {
@ -20,6 +21,7 @@ void NavigationParams::visit_edges(Visitor& visitor)
visitor.visit(request);
visitor.visit(response);
visitor.visit(fetch_controller);
visitor.visit(reserved_environment);
}
void NonFetchSchemeNavigationParams::visit_edges(Visitor& visitor)

View file

@ -19,6 +19,14 @@
namespace Web::HTML {
Environment::~Environment() = default;
void Environment::visit_edges(Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(target_browsing_context);
}
EnvironmentSettingsObject::EnvironmentSettingsObject(NonnullOwnPtr<JS::ExecutionContext> realm_execution_context)
: m_realm_execution_context(move(realm_execution_context))
{
@ -43,7 +51,6 @@ void EnvironmentSettingsObject::visit_edges(Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_responsible_event_loop);
visitor.visit(target_browsing_context);
visitor.visit(m_module_map);
visitor.ignore(m_outstanding_rejected_promises_weak_set);
m_realm_execution_context->visit_edges(visitor);

View file

@ -17,8 +17,11 @@
namespace Web::HTML {
// https://html.spec.whatwg.org/multipage/webappapis.html#environment
struct Environment {
virtual ~Environment() = default;
struct Environment : public JS::Cell {
JS_CELL(Environment, JS::Cell);
public:
virtual ~Environment() override;
// An id https://html.spec.whatwg.org/multipage/webappapis.html#concept-environment-id
String id;
@ -39,6 +42,9 @@ struct Environment {
// https://html.spec.whatwg.org/multipage/webappapis.html#concept-environment-execution-ready-flag
bool execution_ready { false };
protected:
virtual void visit_edges(Cell::Visitor&) override;
};
enum class RunScriptDecision {
@ -47,11 +53,10 @@ enum class RunScriptDecision {
};
// https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object
struct EnvironmentSettingsObject
: public JS::Cell
, public Environment {
JS_CELL(EnvironmentSettingsObject, JS::Cell);
struct EnvironmentSettingsObject : public Environment {
JS_CELL(EnvironmentSettingsObject, Environment);
public:
virtual ~EnvironmentSettingsObject() override;
virtual void initialize(JS::Realm&) override;

View file

@ -29,7 +29,7 @@ void WindowEnvironmentSettingsObject::visit_edges(JS::Cell::Visitor& visitor)
}
// https://html.spec.whatwg.org/multipage/window-object.html#set-up-a-window-environment-settings-object
void WindowEnvironmentSettingsObject::setup(Page& page, URL::URL const& creation_url, NonnullOwnPtr<JS::ExecutionContext> execution_context, Optional<Environment> reserved_environment, URL::URL top_level_creation_url, Origin top_level_origin)
void WindowEnvironmentSettingsObject::setup(Page& page, URL::URL const& creation_url, NonnullOwnPtr<JS::ExecutionContext> execution_context, JS::GCPtr<Environment> reserved_environment, URL::URL top_level_creation_url, Origin top_level_origin)
{
// 1. Let realm be the value of execution context's Realm component.
auto realm = execution_context->realm;
@ -43,7 +43,7 @@ void WindowEnvironmentSettingsObject::setup(Page& page, URL::URL const& creation
auto settings_object = realm->heap().allocate<WindowEnvironmentSettingsObject>(*realm, window, move(execution_context));
// 4. If reservedEnvironment is non-null, then:
if (reserved_environment.has_value()) {
if (reserved_environment) {
// FIXME: 1. Set settings object's id to reservedEnvironment's id,
// target browsing context to reservedEnvironment's target browsing context,
// and active service worker to reservedEnvironment's active service worker.

View file

@ -16,7 +16,7 @@ class WindowEnvironmentSettingsObject final : public EnvironmentSettingsObject {
JS_DECLARE_ALLOCATOR(WindowEnvironmentSettingsObject);
public:
static void setup(Page&, URL::URL const& creation_url, NonnullOwnPtr<JS::ExecutionContext>, Optional<Environment>, URL::URL top_level_creation_url, Origin top_level_origin);
static void setup(Page&, URL::URL const& creation_url, NonnullOwnPtr<JS::ExecutionContext>, JS::GCPtr<Environment>, URL::URL top_level_creation_url, Origin top_level_origin);
virtual ~WindowEnvironmentSettingsObject() override;