LibWeb: Don't leak entire realm after parsing CSS @supports rule

Before this change, every CSS @supports rule would keep the containing
JS realm alive via a JS::Handle. This led to a GC reference cycle and
the whole realm leaked.

Since we only need the realm at construction time, we can take it as a
parameter instead, and stop storing it.
This commit is contained in:
Andreas Kling 2024-04-04 12:44:43 +02:00
commit b2a099e647
Notes: sideshowbarker 2024-07-17 07:31:31 +09:00
3 changed files with 27 additions and 29 deletions

View file

@ -178,7 +178,7 @@ RefPtr<Supports> Parser::parse_a_supports(TokenStream<T>& tokens)
auto maybe_condition = parse_supports_condition(token_stream);
token_stream.skip_whitespace();
if (maybe_condition && !token_stream.has_next_token())
return Supports::create(maybe_condition.release_nonnull());
return Supports::create(m_context.realm(), maybe_condition.release_nonnull());
return {};
}
@ -303,7 +303,7 @@ Optional<Supports::Feature> Parser::parse_supports_feature(TokenStream<Component
if (auto declaration = consume_a_declaration(block_tokens); declaration.has_value()) {
transaction.commit();
return Supports::Feature {
Supports::Declaration { declaration->to_string(), JS::make_handle(m_context.realm()) }
Supports::Declaration { declaration->to_string() }
};
}
}
@ -316,7 +316,7 @@ Optional<Supports::Feature> Parser::parse_supports_feature(TokenStream<Component
builder.append(item.to_string());
transaction.commit();
return Supports::Feature {
Supports::Selector { builder.to_string().release_value_but_fixme_should_propagate_errors(), JS::make_handle(m_context.realm()) }
Supports::Selector { builder.to_string().release_value_but_fixme_should_propagate_errors() }
};
}

View file

@ -10,26 +10,26 @@
namespace Web::CSS {
Supports::Supports(NonnullOwnPtr<Condition>&& condition)
Supports::Supports(JS::Realm& realm, NonnullOwnPtr<Condition>&& condition)
: m_condition(move(condition))
{
m_matches = m_condition->evaluate();
m_matches = m_condition->evaluate(realm);
}
bool Supports::Condition::evaluate() const
bool Supports::Condition::evaluate(JS::Realm& realm) const
{
switch (type) {
case Type::Not:
return !children.first().evaluate();
return !children.first().evaluate(realm);
case Type::And:
for (auto& child : children) {
if (!child.evaluate())
if (!child.evaluate(realm))
return false;
}
return true;
case Type::Or:
for (auto& child : children) {
if (child.evaluate())
if (child.evaluate(realm))
return true;
}
return false;
@ -37,40 +37,40 @@ bool Supports::Condition::evaluate() const
VERIFY_NOT_REACHED();
}
bool Supports::InParens::evaluate() const
bool Supports::InParens::evaluate(JS::Realm& realm) const
{
return value.visit(
[&](NonnullOwnPtr<Condition> const& condition) {
return condition->evaluate();
return condition->evaluate(realm);
},
[&](Feature const& feature) {
return feature.evaluate();
return feature.evaluate(realm);
},
[&](GeneralEnclosed const&) {
return false;
});
}
bool Supports::Declaration::evaluate() const
bool Supports::Declaration::evaluate(JS::Realm& realm) const
{
auto style_property = parse_css_supports_condition(Parser::ParsingContext { *realm }, declaration);
auto style_property = parse_css_supports_condition(Parser::ParsingContext { realm }, declaration);
return style_property.has_value();
}
bool Supports::Selector::evaluate() const
bool Supports::Selector::evaluate(JS::Realm& realm) const
{
auto style_property = parse_selector(Parser::ParsingContext { *realm }, selector);
auto style_property = parse_selector(Parser::ParsingContext { realm }, selector);
return style_property.has_value();
}
bool Supports::Feature::evaluate() const
bool Supports::Feature::evaluate(JS::Realm& realm) const
{
return value.visit(
[&](Declaration const& declaration) {
return declaration.evaluate();
return declaration.evaluate(realm);
},
[&](Selector const& selector) {
return selector.evaluate();
return selector.evaluate(realm);
});
}

View file

@ -23,21 +23,19 @@ class Supports final : public RefCounted<Supports> {
public:
struct Declaration {
String declaration;
JS::Handle<JS::Realm> realm;
bool evaluate() const;
[[nodiscard]] bool evaluate(JS::Realm&) const;
String to_string() const;
};
struct Selector {
String selector;
JS::Handle<JS::Realm> realm;
bool evaluate() const;
[[nodiscard]] bool evaluate(JS::Realm&) const;
String to_string() const;
};
struct Feature {
Variant<Declaration, Selector> value;
bool evaluate() const;
[[nodiscard]] bool evaluate(JS::Realm&) const;
String to_string() const;
};
@ -45,7 +43,7 @@ public:
struct InParens {
Variant<NonnullOwnPtr<Condition>, Feature, GeneralEnclosed> value;
bool evaluate() const;
[[nodiscard]] bool evaluate(JS::Realm&) const;
String to_string() const;
};
@ -58,20 +56,20 @@ public:
Type type;
Vector<InParens> children;
bool evaluate() const;
[[nodiscard]] bool evaluate(JS::Realm&) const;
String to_string() const;
};
static NonnullRefPtr<Supports> create(NonnullOwnPtr<Condition>&& condition)
static NonnullRefPtr<Supports> create(JS::Realm& realm, NonnullOwnPtr<Condition>&& condition)
{
return adopt_ref(*new Supports(move(condition)));
return adopt_ref(*new Supports(realm, move(condition)));
}
bool matches() const { return m_matches; }
String to_string() const;
private:
Supports(NonnullOwnPtr<Condition>&&);
Supports(JS::Realm&, NonnullOwnPtr<Condition>&&);
NonnullOwnPtr<Condition> m_condition;
bool m_matches { false };