From 98ad5a06920e7fc05158e43bc35967e0055b4a79 Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Tue, 25 Jul 2023 10:00:59 +0000 Subject: [PATCH] Bug 1844295 - Don't duplicate ParsingMode in rust and C++. r=emilio Differential Revision: https://phabricator.services.mozilla.com/D184409 --- dom/animation/KeyframeUtils.cpp | 3 ++- dom/canvas/CanvasRenderingContext2D.cpp | 9 ++++--- dom/smil/SMILCSSValueType.cpp | 8 +++--- dom/svg/SVGElement.cpp | 2 +- layout/style/ServoBindings.toml | 1 - layout/style/ServoCSSParser.cpp | 3 ++- layout/style/ServoCSSParser.h | 6 ++--- layout/style/ServoTypes.h | 23 ---------------- layout/style/StyleAnimationValue.cpp | 3 ++- layout/style/nsDOMCSSDeclaration.cpp | 6 ++--- layout/style/test/gtest/StyloParsingBench.cpp | 4 +-- servo/components/style/parser.rs | 27 ------------------- servo/components/style_traits/lib.rs | 1 + servo/ports/geckolib/cbindgen.toml | 1 - servo/ports/geckolib/glue.rs | 16 +++++------ 15 files changed, 32 insertions(+), 81 deletions(-) diff --git a/dom/animation/KeyframeUtils.cpp b/dom/animation/KeyframeUtils.cpp index 35234e0a9dfe..f8a81e4a4e27 100644 --- a/dom/animation/KeyframeUtils.cpp +++ b/dom/animation/KeyframeUtils.cpp @@ -652,7 +652,8 @@ static Maybe MakePropertyValuePair( ServoCSSParser::ParsingEnvironment env = ServoCSSParser::GetParsingEnvironment(aDocument); RefPtr servoDeclarationBlock = - ServoCSSParser::ParseProperty(aProperty, aStringValue, env); + ServoCSSParser::ParseProperty(aProperty, aStringValue, env, + StyleParsingMode::DEFAULT); if (servoDeclarationBlock) { result.emplace(aProperty, std::move(servoDeclarationBlock)); diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp index e9ea23d34ffd..f82a3f4b2dd2 100644 --- a/dom/canvas/CanvasRenderingContext2D.cpp +++ b/dom/canvas/CanvasRenderingContext2D.cpp @@ -2472,7 +2472,8 @@ static already_AddRefed CreateDeclarationForServo( aDocument->GetCompatibilityMode(), aDocument->CSSLoader()}; RefPtr servoDeclarations = - ServoCSSParser::ParseProperty(aProperty, aPropertyValue, env); + ServoCSSParser::ParseProperty(aProperty, aPropertyValue, env, + StyleParsingMode::DEFAULT); if (!servoDeclarations) { // We got a syntax error. The spec says this value must be ignored. @@ -2485,8 +2486,8 @@ static already_AddRefed CreateDeclarationForServo( const nsCString normalString = "normal"_ns; Servo_DeclarationBlock_SetPropertyById( servoDeclarations, eCSSProperty_line_height, &normalString, false, - env.mUrlExtraData, ParsingMode::Default, env.mCompatMode, env.mLoader, - env.mRuleType, {}); + env.mUrlExtraData, StyleParsingMode::DEFAULT, env.mCompatMode, + env.mLoader, env.mRuleType, {}); } return servoDeclarations.forget(); @@ -2555,7 +2556,7 @@ static already_AddRefed GetFontStyleForServo( sc->GetComputedPropertyValue(eCSSProperty_font_size, computedFontSize); Servo_DeclarationBlock_SetPropertyById( declarations, eCSSProperty_font_size, &computedFontSize, false, nullptr, - ParsingMode::Default, eCompatibility_FullStandards, nullptr, + StyleParsingMode::DEFAULT, eCompatibility_FullStandards, nullptr, StyleCssRuleType::Style, {}); } diff --git a/dom/smil/SMILCSSValueType.cpp b/dom/smil/SMILCSSValueType.cpp index a93aeffffa0a..8707637bae58 100644 --- a/dom/smil/SMILCSSValueType.cpp +++ b/dom/smil/SMILCSSValueType.cpp @@ -423,10 +423,10 @@ static ServoAnimationValues ValueFromStringHelper( ServoCSSParser::ParsingEnvironment env = ServoCSSParser::GetParsingEnvironment(doc); RefPtr servoDeclarationBlock = - ServoCSSParser::ParseProperty(aPropID, NS_ConvertUTF16toUTF8(aString), - env, - ParsingMode::AllowUnitlessLength | - ParsingMode::AllowAllNumericValues); + ServoCSSParser::ParseProperty( + aPropID, NS_ConvertUTF16toUTF8(aString), env, + StyleParsingMode::ALLOW_UNITLESS_LENGTH | + StyleParsingMode::ALLOW_ALL_NUMERIC_VALUES); if (!servoDeclarationBlock) { return result; } diff --git a/dom/svg/SVGElement.cpp b/dom/svg/SVGElement.cpp index 49821f7aae63..19a4aaed195a 100644 --- a/dom/svg/SVGElement.cpp +++ b/dom/svg/SVGElement.cpp @@ -1190,7 +1190,7 @@ void MappedAttrParser::ParseMappedAttrValue(nsAtom* aMappedAttrName, auto* doc = mElement.OwnerDoc(); changed = Servo_DeclarationBlock_SetPropertyById( &EnsureDeclarationBlock(), propertyID, &value, false, - &EnsureExtraData(), ParsingMode::AllowUnitlessLength, + &EnsureExtraData(), StyleParsingMode::ALLOW_UNITLESS_LENGTH, doc->GetCompatibilityMode(), doc->CSSLoader(), StyleCssRuleType::Style, {}); diff --git a/layout/style/ServoBindings.toml b/layout/style/ServoBindings.toml index 53a5bb9342cb..51256db57a19 100644 --- a/layout/style/ServoBindings.toml +++ b/layout/style/ServoBindings.toml @@ -350,7 +350,6 @@ allowlist-types = [ "mozilla::DefaultDelete", "mozilla::Side", "mozilla::binding_danger::AssertAndSuppressCleanupPolicy", - "mozilla::ParsingMode", "mozilla::InheritTarget", "mozilla::dom::MediaList", "mozilla::StyleRuleInclusion", diff --git a/layout/style/ServoCSSParser.cpp b/layout/style/ServoCSSParser.cpp index 8e9f14f01800..5d56d0f5fc0b 100644 --- a/layout/style/ServoCSSParser.cpp +++ b/layout/style/ServoCSSParser.cpp @@ -34,7 +34,8 @@ bool ServoCSSParser::ComputeColor(ServoStyleSet* aStyleSet, /* static */ already_AddRefed ServoCSSParser::ParseProperty( nsCSSPropertyID aProperty, const nsACString& aValue, - const ParsingEnvironment& aParsingEnvironment, ParsingMode aParsingMode) { + const ParsingEnvironment& aParsingEnvironment, + const StyleParsingMode& aParsingMode) { return Servo_ParseProperty( aProperty, &aValue, aParsingEnvironment.mUrlExtraData, aParsingMode, aParsingEnvironment.mCompatMode, diff --git a/layout/style/ServoCSSParser.h b/layout/style/ServoCSSParser.h index 1cac71064fdd..4051567bf710 100644 --- a/layout/style/ServoCSSParser.h +++ b/layout/style/ServoCSSParser.h @@ -11,7 +11,6 @@ #include "mozilla/AlreadyAddRefed.h" #include "mozilla/gfx/Matrix.h" -#include "mozilla/ServoTypes.h" #include "nsColor.h" #include "nsCSSPropertyID.h" #include "nsDOMCSSDeclaration.h" @@ -30,6 +29,7 @@ struct StyleFontStretch; struct StyleFontWeight; struct StyleFontStyle; struct StyleLockedDeclarationBlock; +struct StyleParsingMode; union StyleComputedFontStyleDescriptor; template @@ -85,7 +85,7 @@ class ServoCSSParser { * @param aProperty The property to be parsed. * @param aValue The specified value. * @param aParsingEnvironment All the parsing environment data we need. - * @param aParsingMode The paring mode we apply. + * @param aParsingMode The parsing mode we apply. * @return The parsed value as a StyleLockedDeclarationBlock. We put the value * in a declaration block since that is how we represent specified values * in Servo. @@ -93,7 +93,7 @@ class ServoCSSParser { static already_AddRefed ParseProperty( nsCSSPropertyID aProperty, const nsACString& aValue, const ParsingEnvironment& aParsingEnvironment, - ParsingMode aParsingMode = ParsingMode::Default); + const StyleParsingMode& aParsingMode); /** * Parse a animation timing function. diff --git a/layout/style/ServoTypes.h b/layout/style/ServoTypes.h index 72d5e4b9fbf0..b767de8aca4e 100644 --- a/layout/style/ServoTypes.h +++ b/layout/style/ServoTypes.h @@ -80,29 +80,6 @@ enum class UpdateAnimationsTasks : uint8_t { MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(UpdateAnimationsTasks) -// The mode to use when parsing values. -// -// TODO(zrhoffman, bug 1844295): Generate ParsingMode using cbindgen -enum class ParsingMode : uint8_t { - // In CSS, lengths must have units, except for zero values, where the unit can - // be omitted. - // https://www.w3.org/TR/css3-values/#lengths - Default = 0, - // In SVG, a coordinate or length value without a unit identifier (e.g., "25") - // is assumed to be in user units (px). - // https://www.w3.org/TR/SVG/coords.html#Units - AllowUnitlessLength = 1 << 0, - // In SVG, out-of-range values are not treated as an error in parsing. - // https://www.w3.org/TR/SVG/implnote.html#RangeClamping - AllowAllNumericValues = 1 << 1, - // In CSS Properties and Values, the initial value must be computationally - // independent. - // - DisallowFontRelative = 1 << 2, -}; - -MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ParsingMode) - // The kind of style we're generating when requesting Servo to give us an // inherited style. enum class InheritTarget { diff --git a/layout/style/StyleAnimationValue.cpp b/layout/style/StyleAnimationValue.cpp index 292dc3f2f13f..56840f1690a9 100644 --- a/layout/style/StyleAnimationValue.cpp +++ b/layout/style/StyleAnimationValue.cpp @@ -208,7 +208,8 @@ AnimationValue AnimationValue::FromString(nsCSSPropertyID aProperty, RefPtr declarations = ServoCSSParser::ParseProperty(aProperty, aValue, - ServoCSSParser::GetParsingEnvironment(doc)); + ServoCSSParser::GetParsingEnvironment(doc), + StyleParsingMode::DEFAULT); if (!declarations) { return result; diff --git a/layout/style/nsDOMCSSDeclaration.cpp b/layout/style/nsDOMCSSDeclaration.cpp index d9b5ba6c6005..4203748df66f 100644 --- a/layout/style/nsDOMCSSDeclaration.cpp +++ b/layout/style/nsDOMCSSDeclaration.cpp @@ -284,8 +284,8 @@ nsresult nsDOMCSSDeclaration::ParsePropertyValue( [&](DeclarationBlock* decl, ParsingEnvironment& env) { return Servo_DeclarationBlock_SetPropertyById( decl->Raw(), aPropID, &aPropValue, aIsImportant, env.mUrlExtraData, - ParsingMode::Default, env.mCompatMode, env.mLoader, env.mRuleType, - closure); + StyleParsingMode::DEFAULT, env.mCompatMode, env.mLoader, + env.mRuleType, closure); }); } @@ -307,7 +307,7 @@ nsresult nsDOMCSSDeclaration::ParseCustomPropertyValue( [&](DeclarationBlock* decl, ParsingEnvironment& env) { return Servo_DeclarationBlock_SetProperty( decl->Raw(), &aPropertyName, &aPropValue, aIsImportant, - env.mUrlExtraData, ParsingMode::Default, env.mCompatMode, + env.mUrlExtraData, StyleParsingMode::DEFAULT, env.mCompatMode, env.mLoader, env.mRuleType, closure); }); } diff --git a/layout/style/test/gtest/StyloParsingBench.cpp b/layout/style/test/gtest/StyloParsingBench.cpp index f2f03b0f8568..6ee914a78974 100644 --- a/layout/style/test/gtest/StyloParsingBench.cpp +++ b/layout/style/test/gtest/StyloParsingBench.cpp @@ -66,7 +66,7 @@ static void ServoSetPropertyByIdBench(const nsACString& css) { for (int i = 0; i < SETPROPERTY_REPETITIONS; i++) { Servo_DeclarationBlock_SetPropertyById( block, eCSSProperty_width, &css, - /* is_important = */ false, data, ParsingMode::Default, + /* is_important = */ false, data, StyleParsingMode::DEFAULT, eCompatibility_FullStandards, nullptr, STYLE_RULE, {}); } } @@ -84,7 +84,7 @@ static void ServoGetPropertyValueById() { const nsACString& css = css_; Servo_DeclarationBlock_SetPropertyById( block, eCSSProperty_width, &css, - /* is_important = */ false, data, ParsingMode::Default, + /* is_important = */ false, data, StyleParsingMode::DEFAULT, eCompatibility_FullStandards, nullptr, STYLE_RULE, {}); for (int i = 0; i < GETPROPERTY_REPETITIONS; i++) { diff --git a/servo/components/style/parser.rs b/servo/components/style/parser.rs index 7b7d30d810a9..151c5de02a57 100644 --- a/servo/components/style/parser.rs +++ b/servo/components/style/parser.rs @@ -12,33 +12,6 @@ use cssparser::{Parser, SourceLocation, UnicodeRange}; use std::borrow::Cow; use style_traits::{OneOrMoreSeparated, ParseError, ParsingMode, Separator}; -/// Asserts that all ParsingMode flags have a matching ParsingMode value in gecko. -#[cfg(feature = "gecko")] -#[inline] -pub fn assert_parsing_mode_match() { - use crate::gecko_bindings::structs; - - macro_rules! check_parsing_modes { - ( $( $a:ident => $b:path ),*, ) => { - if cfg!(debug_assertions) { - let mut modes = ParsingMode::all(); - $( - assert_eq!(structs::$a as usize, $b.bits() as usize, stringify!($b)); - modes.remove($b); - )* - assert_eq!(modes, ParsingMode::empty(), "all ParsingMode bits should have an assertion"); - } - } - } - - check_parsing_modes! { - ParsingMode_Default => ParsingMode::DEFAULT, - ParsingMode_AllowUnitlessLength => ParsingMode::ALLOW_UNITLESS_LENGTH, - ParsingMode_AllowAllNumericValues => ParsingMode::ALLOW_ALL_NUMERIC_VALUES, - ParsingMode_DisallowFontRelative => ParsingMode::DISALLOW_FONT_RELATIVE, - } -} - /// The data that the parser needs from outside in order to parse a stylesheet. pub struct ParserContext<'a> { /// The `Origin` of the stylesheet, whether it's a user, author or diff --git a/servo/components/style_traits/lib.rs b/servo/components/style_traits/lib.rs index 029cff550b96..624d3152033b 100644 --- a/servo/components/style_traits/lib.rs +++ b/servo/components/style_traits/lib.rs @@ -243,6 +243,7 @@ pub enum PropertySyntaxParseError { bitflags! { /// The mode to use when parsing values. #[derive(Clone, Copy, Eq, PartialEq)] + #[repr(C)] pub struct ParsingMode: u8 { /// In CSS; lengths must have units, except for zero values, where the unit can be omitted. /// diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index 4175ebe3c785..8f41351ff8a0 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -301,7 +301,6 @@ renaming_overrides_prefixing = true "AnimationPropertySegment" = "AnimationPropertySegment" "RawServoAnimationValueTable" = "RawServoAnimationValueTable" "nsCSSUnit" = "nsCSSUnit" -"ParsingMode" = "ParsingMode" "InheritTarget" = "InheritTarget" "PseudoStyleType" = "PseudoStyleType" "DeclarationBlockMutationClosure" = "DeclarationBlockMutationClosure" diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 937fa054f614..f9efbace1808 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -97,7 +97,7 @@ use style::global_style_data::{ use style::invalidation::element::restyle_hints::RestyleHint; use style::invalidation::stylesheets::RuleChangeKind; use style::media_queries::MediaList; -use style::parser::{self, Parse, ParserContext}; +use style::parser::{Parse, ParserContext}; use style::properties::animated_properties::{AnimationValue, AnimationValueMap}; use style::properties::{parse_one_declaration_into, parse_style_attribute}; use style::properties::{ComputedValues, CountedUnknownProperty, Importance, NonCustomPropertyId}; @@ -189,7 +189,6 @@ pub unsafe extern "C" fn Servo_Initialize( // Perform some debug-only runtime assertions. origin_flags::assert_flags_match(); - parser::assert_parsing_mode_match(); traversal_flags::assert_traversal_flags_match(); specified::font::assert_variant_east_asian_matches(); specified::font::assert_variant_ligatures_matches(); @@ -4401,13 +4400,12 @@ fn parse_property_into( value: &nsACString, origin: Origin, url_data: &UrlExtraData, - parsing_mode: structs::ParsingMode, + parsing_mode: ParsingMode, quirks_mode: QuirksMode, rule_type: CssRuleType, reporter: Option<&dyn ParseErrorReporter>, ) -> Result<(), ()> { let value = unsafe { value.as_str_unchecked() }; - let parsing_mode = ParsingMode::from_bits_retain(parsing_mode); if let Some(non_custom) = property_id.non_custom_id() { if !non_custom.allowed_in_rule(rule_type.into()) { @@ -4433,7 +4431,7 @@ pub unsafe extern "C" fn Servo_ParseProperty( property: nsCSSPropertyID, value: &nsACString, data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, + parsing_mode: ParsingMode, quirks_mode: nsCompatibility, loader: *mut Loader, rule_type: CssRuleType, @@ -4802,7 +4800,7 @@ fn set_property( value: &nsACString, is_important: bool, data: &UrlExtraData, - parsing_mode: structs::ParsingMode, + parsing_mode: ParsingMode, quirks_mode: QuirksMode, loader: *mut Loader, rule_type: CssRuleType, @@ -4849,7 +4847,7 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetProperty( value: &nsACString, is_important: bool, data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, + parsing_mode: ParsingMode, quirks_mode: nsCompatibility, loader: *mut Loader, rule_type: CssRuleType, @@ -4894,7 +4892,7 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyById( value: &nsACString, is_important: bool, data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, + parsing_mode: ParsingMode, quirks_mode: nsCompatibility, loader: *mut Loader, rule_type: CssRuleType, @@ -5689,7 +5687,7 @@ pub extern "C" fn Servo_CSSSupports2(property: &nsACString, value: &nsACString) value, Origin::Author, unsafe { dummy_url_data() }, - structs::ParsingMode_Default, + ParsingMode::DEFAULT, QuirksMode::NoQuirks, CssRuleType::Style, None,