From 03613dc14d0462f411a2913f634ccc4143e2cf90 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@serenityos.org>
Date: Wed, 10 May 2023 12:43:29 +0100
Subject: [PATCH] LibWeb: Make `value_id_from_string()` return Optional

---
 .../CodeGenerators/LibWeb/GenerateCSSValueID.cpp |  7 +++----
 Tests/LibWeb/TestCSSIDSpeed.cpp                  | 16 ++++++++--------
 Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp  | 10 +++++-----
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSValueID.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSValueID.cpp
index fbc53f7882..a03d1c71ca 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSValueID.cpp
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateCSSValueID.cpp
@@ -67,7 +67,7 @@ enum class ValueID {
     generator.append(R"~~~(
 };
 
-ValueID value_id_from_string(StringView);
+Optional<ValueID> value_id_from_string(StringView);
 StringView string_from_value_id(ValueID);
 
 }
@@ -105,10 +105,9 @@ HashMap<StringView, ValueID, AK::CaseInsensitiveASCIIStringViewTraits> g_stringv
     generator.append(R"~~~(
 };
 
-ValueID value_id_from_string(StringView string)
+Optional<ValueID> value_id_from_string(StringView string)
 {
-    auto maybe_value_id = g_stringview_to_value_id_map.get(string);
-    return maybe_value_id.value_or(ValueID::Invalid);
+    return g_stringview_to_value_id_map.get(string);
 }
 
 StringView string_from_value_id(ValueID value_id) {
diff --git a/Tests/LibWeb/TestCSSIDSpeed.cpp b/Tests/LibWeb/TestCSSIDSpeed.cpp
index e42e7da8f7..ce28de604b 100644
--- a/Tests/LibWeb/TestCSSIDSpeed.cpp
+++ b/Tests/LibWeb/TestCSSIDSpeed.cpp
@@ -10,18 +10,18 @@
 
 TEST_CASE(basic)
 {
-    EXPECT_EQ(Web::CSS::value_id_from_string("italic"sv), Web::CSS::ValueID::Italic);
-    EXPECT_EQ(Web::CSS::value_id_from_string("inline"sv), Web::CSS::ValueID::Inline);
-    EXPECT_EQ(Web::CSS::value_id_from_string("small"sv), Web::CSS::ValueID::Small);
-    EXPECT_EQ(Web::CSS::value_id_from_string("smalL"sv), Web::CSS::ValueID::Small);
-    EXPECT_EQ(Web::CSS::value_id_from_string("SMALL"sv), Web::CSS::ValueID::Small);
-    EXPECT_EQ(Web::CSS::value_id_from_string("Small"sv), Web::CSS::ValueID::Small);
-    EXPECT_EQ(Web::CSS::value_id_from_string("smALl"sv), Web::CSS::ValueID::Small);
+    EXPECT_EQ(Web::CSS::value_id_from_string("italic"sv).value(), Web::CSS::ValueID::Italic);
+    EXPECT_EQ(Web::CSS::value_id_from_string("inline"sv).value(), Web::CSS::ValueID::Inline);
+    EXPECT_EQ(Web::CSS::value_id_from_string("small"sv).value(), Web::CSS::ValueID::Small);
+    EXPECT_EQ(Web::CSS::value_id_from_string("smalL"sv).value(), Web::CSS::ValueID::Small);
+    EXPECT_EQ(Web::CSS::value_id_from_string("SMALL"sv).value(), Web::CSS::ValueID::Small);
+    EXPECT_EQ(Web::CSS::value_id_from_string("Small"sv).value(), Web::CSS::ValueID::Small);
+    EXPECT_EQ(Web::CSS::value_id_from_string("smALl"sv).value(), Web::CSS::ValueID::Small);
 }
 
 BENCHMARK_CASE(value_id_from_string)
 {
     for (size_t i = 0; i < 10'000'000; ++i) {
-        EXPECT_EQ(Web::CSS::value_id_from_string("inline"sv), Web::CSS::ValueID::Inline);
+        EXPECT_EQ(Web::CSS::value_id_from_string("inline"sv).value(), Web::CSS::ValueID::Inline);
     }
 }
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
index de1a196102..7fb3862da3 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -1260,9 +1260,9 @@ Optional<MediaFeatureValue> Parser::parse_media_feature_value(MediaFeatureID med
         auto transaction = tokens.begin_transaction();
         tokens.skip_whitespace();
         auto ident = value_id_from_string(tokens.next_token().token().ident());
-        if (ident != ValueID::Invalid && media_feature_accepts_identifier(media_feature, ident)) {
+        if (ident.has_value() && media_feature_accepts_identifier(media_feature, ident.value())) {
             transaction.commit();
-            return MediaFeatureValue(ident);
+            return MediaFeatureValue(ident.value());
         }
     }
 
@@ -3720,8 +3720,8 @@ ErrorOr<RefPtr<StyleValue>> Parser::parse_identifier_value(ComponentValue const&
 {
     if (component_value.is(Token::Type::Ident)) {
         auto value_id = value_id_from_string(component_value.token().ident());
-        if (value_id != ValueID::Invalid)
-            return IdentifierStyleValue::create(value_id);
+        if (value_id.has_value())
+            return IdentifierStyleValue::create(value_id.value());
     }
 
     return nullptr;
@@ -5653,7 +5653,7 @@ CSSRule* Parser::parse_font_face_rule(TokenStream<ComponentValue>& tokens)
                         break;
                     }
                     auto value_id = value_id_from_string(part.token().ident());
-                    if (is_generic_font_family(value_id)) {
+                    if (value_id.has_value() && is_generic_font_family(value_id.value())) {
                         dbgln_if(CSS_PARSER_DEBUG, "CSSParser: @font-face font-family format invalid; discarding.");
                         had_syntax_error = true;
                         break;