Skip to content

Commit

Permalink
LibWeb: Stop passing Realm unnecessarily to parse CSS properties
Browse files Browse the repository at this point in the history
Also use the parse_css_value() helper in cases where we previously
constructed a Parser manually.
  • Loading branch information
AtkinsSJ committed Dec 5, 2024
1 parent 93d766f commit 13733a7
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 46 deletions.
8 changes: 3 additions & 5 deletions Libraries/LibWeb/Animations/AnimationEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ WebIDL::ExceptionOr<void> AnimationEffect::update_timing(OptionalEffectTiming ti
// [CSS-EASING-1], throw a TypeError and abort this procedure.
RefPtr<CSS::CSSStyleValue const> easing_value;
if (timing.easing.has_value()) {
easing_value = parse_easing_string(realm(), timing.easing.value());
easing_value = parse_easing_string(timing.easing.value());
if (!easing_value)
return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, "Invalid easing function"sv };
VERIFY(easing_value->is_easing());
Expand Down Expand Up @@ -589,11 +589,9 @@ Optional<double> AnimationEffect::transformed_progress() const
return m_timing_function.evaluate_at(directed_progress.value(), before_flag);
}

RefPtr<CSS::CSSStyleValue const> AnimationEffect::parse_easing_string(JS::Realm& realm, StringView value)
RefPtr<CSS::CSSStyleValue const> AnimationEffect::parse_easing_string(StringView value)
{
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);

if (auto style_value = parser.parse_as_css_value(CSS::PropertyID::AnimationTimingFunction)) {
if (auto style_value = parse_css_value(CSS::Parser::ParsingContext(), value, CSS::PropertyID::AnimationTimingFunction)) {
if (style_value->is_easing())
return style_value;
}
Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibWeb/Animations/AnimationEffect.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class AnimationEffect : public Bindings::PlatformObject {
GC_DECLARE_ALLOCATOR(AnimationEffect);

public:
static RefPtr<CSS::CSSStyleValue const> parse_easing_string(JS::Realm& realm, StringView value);
static RefPtr<CSS::CSSStyleValue const> parse_easing_string(StringView value);

EffectTiming get_timing() const;
ComputedEffectTiming get_computed_timing() const;
Expand Down
8 changes: 3 additions & 5 deletions Libraries/LibWeb/Animations/KeyframeEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,7 @@ static WebIDL::ExceptionOr<Vector<BaseKeyframe>> process_a_keyframes_argument(JS
if (!property_id.has_value())
continue;

auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value_string);

if (auto style_value = parser.parse_as_css_value(*property_id)) {
if (auto style_value = parse_css_value(CSS::Parser::ParsingContext(), value_string, *property_id)) {
// Handle 'initial' here so we don't have to get the default value of the property every frame in StyleComputer
if (style_value->is_initial())
style_value = CSS::property_initial_value(*property_id);
Expand All @@ -558,7 +556,7 @@ static WebIDL::ExceptionOr<Vector<BaseKeyframe>> process_a_keyframes_argument(JS
//
// If parsing the "easing" property fails, throw a TypeError and abort this procedure.
auto easing_string = keyframe.easing.get<String>();
auto easing_value = AnimationEffect::parse_easing_string(realm, easing_string);
auto easing_value = AnimationEffect::parse_easing_string(easing_string);

if (!easing_value)
return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, MUST(String::formatted("Invalid animation easing value: \"{}\"", easing_string)) };
Expand All @@ -570,7 +568,7 @@ static WebIDL::ExceptionOr<Vector<BaseKeyframe>> process_a_keyframes_argument(JS
// interface, and if any of the values fail to parse, throw a TypeError and abort this procedure.
for (auto& unused_easing : unused_easings) {
auto easing_string = unused_easing.get<String>();
auto easing_value = AnimationEffect::parse_easing_string(realm, easing_string);
auto easing_value = AnimationEffect::parse_easing_string(easing_string);
if (!easing_value)
return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, MUST(String::formatted("Invalid animation easing value: \"{}\"", easing_string)) };
}
Expand Down
6 changes: 2 additions & 4 deletions Libraries/LibWeb/CSS/CSS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ WebIDL::ExceptionOr<String> escape(JS::VM&, StringView identifier)
}

// https://www.w3.org/TR/css-conditional-3/#dom-css-supports
bool supports(JS::VM& vm, StringView property, StringView value)
bool supports(JS::VM&, StringView property, StringView value)
{
auto& realm = *vm.current_realm();

// 1. If property is an ASCII case-insensitive match for any defined CSS property that the UA supports,
// and value successfully parses according to that property’s grammar, return true.
if (auto property_id = property_id_from_string(property); property_id.has_value()) {
if (parse_css_value(Parser::ParsingContext { realm }, value, property_id.value()))
if (parse_css_value(Parser::ParsingContext {}, value, property_id.value()))
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ WebIDL::ExceptionOr<void> PropertyOwningCSSStyleDeclaration::set_property(String
// 5. Let component value list be the result of parsing value for property property.
auto component_value_list = is<ElementInlineCSSStyleDeclaration>(this)
? parse_css_value(CSS::Parser::ParsingContext { static_cast<ElementInlineCSSStyleDeclaration&>(*this).element()->document() }, value, property_id)
: parse_css_value(CSS::Parser::ParsingContext { realm() }, value, property_id);
: parse_css_value(CSS::Parser::ParsingContext {}, value, property_id);

// 6. If component value list is null, then return.
if (!component_value_list)
Expand Down
15 changes: 4 additions & 11 deletions Libraries/LibWeb/CSS/FontFace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ static NonnullRefPtr<Core::Promise<NonnullRefPtr<Gfx::Typeface>>> load_vector_fo

GC_DEFINE_ALLOCATOR(FontFace);

template<CSS::PropertyID PropertyID>
RefPtr<CSSStyleValue const> parse_property_string(JS::Realm& realm, StringView value)
{
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), value);
return parser.parse_as_css_value(PropertyID);
}

// https://drafts.csswg.org/css-font-loading/#font-face-constructor
GC::Ref<FontFace> FontFace::construct_impl(JS::Realm& realm, String family, FontFaceSource source, FontFaceDescriptors const& descriptors)
{
Expand Down Expand Up @@ -213,7 +206,7 @@ GC::Ref<WebIDL::Promise> FontFace::loaded() const
// https://drafts.csswg.org/css-font-loading/#dom-fontface-family
WebIDL::ExceptionOr<void> FontFace::set_family(String const& string)
{
auto property = parse_property_string<CSS::PropertyID::FontFamily>(realm(), string);
auto property = parse_css_value(Parser::ParsingContext(), string, CSS::PropertyID::FontFamily);
if (!property)
return WebIDL::SyntaxError::create(realm(), "FontFace.family setter: Invalid font descriptor"_string);

Expand All @@ -229,7 +222,7 @@ WebIDL::ExceptionOr<void> FontFace::set_family(String const& string)
// https://drafts.csswg.org/css-font-loading/#dom-fontface-style
WebIDL::ExceptionOr<void> FontFace::set_style(String const& string)
{
auto property = parse_property_string<CSS::PropertyID::FontStyle>(realm(), string);
auto property = parse_css_value(Parser::ParsingContext(), string, CSS::PropertyID::FontStyle);
if (!property)
return WebIDL::SyntaxError::create(realm(), "FontFace.style setter: Invalid font descriptor"_string);

Expand All @@ -245,7 +238,7 @@ WebIDL::ExceptionOr<void> FontFace::set_style(String const& string)
// https://drafts.csswg.org/css-font-loading/#dom-fontface-weight
WebIDL::ExceptionOr<void> FontFace::set_weight(String const& string)
{
auto property = parse_property_string<CSS::PropertyID::FontWeight>(realm(), string);
auto property = parse_css_value(Parser::ParsingContext(), string, CSS::PropertyID::FontWeight);
if (!property)
return WebIDL::SyntaxError::create(realm(), "FontFace.weight setter: Invalid font descriptor"_string);

Expand All @@ -262,7 +255,7 @@ WebIDL::ExceptionOr<void> FontFace::set_weight(String const& string)
WebIDL::ExceptionOr<void> FontFace::set_stretch(String const& string)
{
// NOTE: font-stretch is now an alias for font-width
auto property = parse_property_string<CSS::PropertyID::FontWidth>(realm(), string);
auto property = parse_css_value(Parser::ParsingContext(), string, CSS::PropertyID::FontWidth);
if (!property)
return WebIDL::SyntaxError::create(realm(), "FontFace.stretch setter: Invalid font descriptor"_string);

Expand Down
3 changes: 1 addition & 2 deletions Libraries/LibWeb/CSS/FontFaceSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,7 @@ WebIDL::CallbackType* FontFaceSet::onloadingerror()
static WebIDL::ExceptionOr<GC::Ref<JS::Set>> find_matching_font_faces(JS::Realm& realm, FontFaceSet& font_face_set, String const& font, String const&)
{
// 1. Parse font using the CSS value syntax of the font property. If a syntax error occurs, return a syntax error.
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), font);
auto property = parser.parse_as_css_value(PropertyID::Font);
auto property = parse_css_value(CSS::Parser::ParsingContext(), font, PropertyID::Font);
if (!property)
return WebIDL::SyntaxError::create(realm, "Unable to parse font"_string);

Expand Down
3 changes: 1 addition & 2 deletions Libraries/LibWeb/Geometry/DOMMatrixReadOnly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -946,8 +946,7 @@ WebIDL::ExceptionOr<ParsedMatrix> parse_dom_matrix_init_string(JS::Realm& realm,
// 2. Parse transformList into parsedValue given the grammar for the CSS transform property.
// The result will be a <transform-list>, the keyword none, or failure.
// If parsedValue is failure, or any <transform-function> has <length> values without absolute length units, or any keyword other than none is used, then return failure. [CSS3-SYNTAX] [CSS3-TRANSFORMS]
auto parsing_context = CSS::Parser::ParsingContext { realm };
auto transform_style_value = parse_css_value(parsing_context, transform_list, CSS::PropertyID::Transform);
auto transform_style_value = parse_css_value(CSS::Parser::ParsingContext {}, transform_list, CSS::PropertyID::Transform);
if (!transform_style_value)
return WebIDL::SyntaxError::create(realm, "Failed to parse CSS transform string."_string);
auto parsed_value = CSS::StyleProperties::transformations_for_style_value(*transform_style_value);
Expand Down
10 changes: 2 additions & 8 deletions Libraries/LibWeb/HTML/Canvas/CanvasFillStrokeStyles.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,16 @@ class CanvasFillStrokeStyles {

void set_fill_style(FillOrStrokeStyleVariant style)
{
auto& realm = static_cast<IncludingClass&>(*this).realm();

// https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-fillstyle
style.visit(
// 1. If the given value is a string, then:
[&](String const& string) {
// 1. Let context be this's canvas attribute's value, if that is an element; otherwise null.
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);

// 2. Let parsedValue be the result of parsing the given value with context if non-null.
// FIXME: Parse a color value
// https://drafts.csswg.org/css-color/#parse-a-css-color-value
auto style_value = parser.parse_as_css_value(CSS::PropertyID::Color);
auto style_value = parse_css_value(CSS::Parser::ParsingContext(), string, CSS::PropertyID::Color);
if (style_value && style_value->has_color()) {
auto parsedValue = style_value->to_color(OptionalNone());

Expand Down Expand Up @@ -67,20 +64,17 @@ class CanvasFillStrokeStyles {

void set_stroke_style(FillOrStrokeStyleVariant style)
{
auto& realm = static_cast<IncludingClass&>(*this).realm();

// https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-strokestyle

style.visit(
// 1. If the given value is a string, then:
[&](String const& string) {
// 1. Let context be this's canvas attribute's value, if that is an element; otherwise null.
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), string);

// 2. Let parsedValue be the result of parsing the given value with context if non-null.
// FIXME: Parse a color value
// https://drafts.csswg.org/css-color/#parse-a-css-color-value
auto style_value = parser.parse_as_css_value(CSS::PropertyID::Color);
auto style_value = parse_css_value(CSS::Parser::ParsingContext(), string, CSS::PropertyID::Color);
if (style_value && style_value->has_color()) {
auto parsedValue = style_value->to_color(OptionalNone());

Expand Down
3 changes: 1 addition & 2 deletions Libraries/LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ class CanvasTextDrawingStyles {
// and with system fonts being computed to explicit values.
// FIXME: with the 'line-height' component forced to 'normal'
// FIXME: with the 'font-size' component converted to CSS pixels
auto parsing_context = CSS::Parser::ParsingContext { reinterpret_cast<IncludingClass&>(*this).realm() };
auto font_style_value_result = parse_css_value(parsing_context, font, CSS::PropertyID::Font);
auto font_style_value_result = parse_css_value(CSS::Parser::ParsingContext {}, font, CSS::PropertyID::Font);

// If the new value is syntactically incorrect (including using property-independent style sheet syntax like 'inherit' or 'initial'), then it must be ignored, without assigning a new font value.
// NOTE: ShorthandStyleValue should be the only valid option here. We implicitly VERIFY this below.
Expand Down
6 changes: 1 addition & 5 deletions Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -790,14 +790,10 @@ String CanvasRenderingContext2D::shadow_color() const

void CanvasRenderingContext2D::set_shadow_color(String color)
{
auto& realm = static_cast<CanvasRenderingContext2D&>(*this).realm();

// 1. Let context be this's canvas attribute's value, if that is an element; otherwise null.
auto parser = CSS::Parser::Parser::create(CSS::Parser::ParsingContext(realm), color);

auto style_value = parser.parse_as_css_value(CSS::PropertyID::Color);

// 2. Let parsedValue be the result of parsing the given value with context if non-null.
auto style_value = parse_css_value(CSS::Parser::ParsingContext(), color, CSS::PropertyID::Color);
if (style_value && style_value->has_color()) {
auto parsedValue = style_value->to_color(OptionalNone());

Expand Down

0 comments on commit 13733a7

Please sign in to comment.