From 80a20be176d2b2274613ec92cd6e74f37592bca8 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@gmail.com>
Date: Fri, 9 Aug 2024 14:42:45 +0100
Subject: [PATCH] LibWeb/CSS: Bring previous CSSRule parsing up to standard

GCPtrs instead of raw pointers, and logging when the media rule is
invalid.
---
 .../LibWeb/CSS/Parser/MediaParsing.cpp        | 20 ++++++++++++++-----
 .../Libraries/LibWeb/CSS/Parser/Parser.cpp    |  8 ++++----
 Userland/Libraries/LibWeb/CSS/Parser/Parser.h |  6 +++---
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/Userland/Libraries/LibWeb/CSS/Parser/MediaParsing.cpp b/Userland/Libraries/LibWeb/CSS/Parser/MediaParsing.cpp
index bfa436a2b14..6393c6928dc 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/MediaParsing.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/MediaParsing.cpp
@@ -614,18 +614,28 @@ Optional<MediaFeatureValue> Parser::parse_media_feature_value(MediaFeatureID med
     return {};
 }
 
-CSSMediaRule* Parser::convert_to_media_rule(NonnullRefPtr<Web::CSS::Parser::Rule> rule)
+JS::GCPtr<CSSMediaRule> Parser::convert_to_media_rule(Rule& rule)
 {
-    auto media_query_tokens = TokenStream { rule->prelude() };
+    if (rule.prelude().is_empty()) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @media rule: Empty prelude.");
+        return {};
+    }
+
+    if (!rule.block()) {
+        dbgln_if(CSS_PARSER_DEBUG, "Failed to parse @media rule: No block.");
+        return {};
+    }
+
+    auto media_query_tokens = TokenStream { rule.prelude() };
     auto media_query_list = parse_a_media_query_list(media_query_tokens);
-    if (media_query_list.is_empty() || !rule->block())
+    if (media_query_list.is_empty())
         return {};
 
-    auto child_tokens = TokenStream { rule->block()->values() };
+    auto child_tokens = TokenStream { rule.block()->values() };
     auto parser_rules = parse_a_list_of_rules(child_tokens);
     JS::MarkedVector<CSSRule*> child_rules(m_context.realm().heap());
     for (auto& raw_rule : parser_rules) {
-        if (auto* child_rule = convert_to_rule(raw_rule))
+        if (auto child_rule = convert_to_rule(raw_rule))
             child_rules.append(child_rule);
     }
     auto media_list = MediaList::create(m_context.realm(), move(media_query_list));
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
index e158af0d537..7e9478293f2 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -149,7 +149,7 @@ CSSStyleSheet* Parser::parse_as_css_stylesheet(Optional<URL::URL> location)
     // Interpret all of the resulting top-level qualified rules as style rules, defined below.
     JS::MarkedVector<CSSRule*> rules(m_context.realm().heap());
     for (auto& raw_rule : style_sheet.rules) {
-        auto* rule = convert_to_rule(raw_rule);
+        auto rule = convert_to_rule(raw_rule);
         // If any style rule is invalid, or any at-rule is not recognized or is invalid according to its grammar or context, it’s a parse error. Discard that rule.
         if (rule)
             rules.append(rule);
@@ -1244,7 +1244,7 @@ RefPtr<StyleValue> Parser::parse_basic_shape_value(TokenStream<ComponentValue>&
     return BasicShapeStyleValue::create(Polygon { FillRule::Nonzero, move(points) });
 }
 
-CSSRule* Parser::convert_to_rule(NonnullRefPtr<Rule> rule)
+JS::GCPtr<CSSRule> Parser::convert_to_rule(NonnullRefPtr<Rule> rule)
 {
     if (rule->is_at_rule()) {
         if (has_ignored_vendor_prefix(rule->at_rule_name()))
@@ -1547,7 +1547,7 @@ JS::GCPtr<CSSSupportsRule> Parser::convert_to_supports_rule(Rule& rule)
     auto parser_rules = parse_a_list_of_rules(child_tokens);
     JS::MarkedVector<CSSRule*> child_rules { m_context.realm().heap() };
     for (auto& raw_rule : parser_rules) {
-        if (auto* child_rule = convert_to_rule(raw_rule))
+        if (auto child_rule = convert_to_rule(raw_rule))
             child_rules.append(child_rule);
     }
 
@@ -5126,7 +5126,7 @@ RefPtr<StyleValue> Parser::parse_font_family_value(TokenStream<ComponentValue>&
     return StyleValueList::create(move(font_families), StyleValueList::Separator::Comma);
 }
 
-CSSRule* Parser::parse_font_face_rule(TokenStream<ComponentValue>& tokens)
+JS::GCPtr<CSSFontFaceRule> Parser::parse_font_face_rule(TokenStream<ComponentValue>& tokens)
 {
     auto declarations_and_at_rules = parse_a_list_of_declarations(tokens);
 
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
index 7d49ed6279d..9427414d963 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
@@ -215,13 +215,13 @@ private:
 
     Optional<GeneralEnclosed> parse_general_enclosed(TokenStream<ComponentValue>&);
 
-    CSSRule* parse_font_face_rule(TokenStream<ComponentValue>&);
+    JS::GCPtr<CSSFontFaceRule> parse_font_face_rule(TokenStream<ComponentValue>&);
 
     template<typename T>
     Vector<ParsedFontFace::Source> parse_font_face_src(TokenStream<T>&);
 
-    CSSRule* convert_to_rule(NonnullRefPtr<Rule>);
-    CSSMediaRule* convert_to_media_rule(NonnullRefPtr<Rule>);
+    JS::GCPtr<CSSRule> convert_to_rule(NonnullRefPtr<Rule>);
+    JS::GCPtr<CSSMediaRule> convert_to_media_rule(Rule&);
     JS::GCPtr<CSSKeyframesRule> convert_to_keyframes_rule(Rule&);
     JS::GCPtr<CSSImportRule> convert_to_import_rule(Rule&);
     JS::GCPtr<CSSNamespaceRule> convert_to_namespace_rule(Rule&);