Auto merge of #19317 - emilio:property-allowed-in, r=SimonSapin

style: Move property allowance tests to PropertyId::parse_into.

It's not only more consistent (since we have a proper ParserContext there), but
also fixes a bunch of bugs where Gecko accidentally exposes and allows setting
internal state because of conversions from nsCSSPropertyID to PropertyId.

This adds the extra complexity of caring about aliases for longer, but that's
probably not a big deal in practice, since we have PropertyDeclarationId.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19317)
<!-- Reviewable:end -->

components/layout/query.rs

@@ -689,9 +689,11 @@ pub fn process_resolved_style_request<'a, N>(context: &LayoutContext,
     let styles = resolve_style(&mut context, element, RuleInclusion::All, false, pseudo.as_ref());
     let style = styles.primary();
     let longhand_id = match *property {
+        PropertyId::LonghandAlias(id, _) |
         PropertyId::Longhand(id) => id,
         // Firefox returns blank strings for the computed value of shorthands,
         // so this should be web-compatible.
+        PropertyId::ShorthandAlias(..) |
         PropertyId::Shorthand(_) => return String::new(),
         PropertyId::Custom(ref name) => {
             return style.computed_value_to_string(PropertyDeclarationId::Custom(name))
@@ -737,12 +739,12 @@ where
 
     let style = &*layout_el.resolved_style();
     let longhand_id = match *property {
+        PropertyId::LonghandAlias(id, _) |
         PropertyId::Longhand(id) => id,
-
         // Firefox returns blank strings for the computed value of shorthands,
         // so this should be web-compatible.
+        PropertyId::ShorthandAlias(..) |
         PropertyId::Shorthand(_) => return String::new(),
-
         PropertyId::Custom(ref name) => {
             return style.computed_value_to_string(PropertyDeclarationId::Custom(name))
         }

components/layout_thread/lib.rs

@@ -742,7 +742,7 @@ impl LayoutThread {
             Msg::RegisterPaint(name, mut properties, painter) => {
                 debug!("Registering the painter");
                 let properties = properties.drain(..)
-                    .filter_map(|name| PropertyId::parse(&*name, None)
+                    .filter_map(|name| PropertyId::parse(&*name)
                         .ok().map(|id| (name.clone(), id)))
                     .filter(|&(_, ref id)| id.as_shorthand().is_err())
                     .collect();

components/script/dom/cssstyledeclaration.rs

@@ -299,7 +299,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
     fn GetPropertyValue(&self, property: DOMString) -> DOMString {
-        let id = if let Ok(id) = PropertyId::parse(&property, None) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property
@@ -310,7 +310,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
     fn GetPropertyPriority(&self, property: DOMString) -> DOMString {
-        let id = if let Ok(id) = PropertyId::parse(&property, None) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property
@@ -334,7 +334,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                    priority: DOMString)
                    -> ErrorResult {
         // Step 3
-        let id = if let Ok(id) = PropertyId::parse(&property, None) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unknown property
@@ -351,7 +351,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         }
 
         // Step 2 & 3
-        let id = match PropertyId::parse(&property, None) {
+        let id = match PropertyId::parse(&property) {
             Ok(id) => id,
             Err(..) => return Ok(()), // Unkwown property
         };
@@ -383,7 +383,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             return Err(Error::NoModificationAllowed);
         }
 
-        let id = if let Ok(id) = PropertyId::parse(&property, None) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property, cannot be there to remove.

components/style/properties/declaration_block.rs

@@ -560,7 +560,8 @@ impl PropertyDeclarationBlock {
     ///
     /// Returns whether any declaration was actually removed.
     pub fn remove_property(&mut self, property: &PropertyId) -> bool {
-        if let PropertyId::Longhand(id) = *property {
+        let longhand_id = property.longhand_id();
+        if let Some(id) = longhand_id {
             if !self.longhands.contains(id) {
                 return false
             }
@@ -584,7 +585,7 @@ impl PropertyDeclarationBlock {
             !remove
         });
 
-        if let PropertyId::Longhand(_) = *property {
+        if longhand_id.is_some() {
             debug_assert!(removed_at_least_one);
         }
         removed_at_least_one
@@ -681,7 +682,7 @@ impl PropertyDeclarationBlock {
     /// property.
     #[cfg(feature = "gecko")]
     pub fn has_css_wide_keyword(&self, property: &PropertyId) -> bool {
-        if let PropertyId::Longhand(id) = *property {
+        if let Some(id) = property.longhand_id() {
             if !self.longhands.contains(id) {
                 return false
             }
@@ -1092,12 +1093,14 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
     type Declaration = Importance;
     type Error = StyleParseErrorKind<'i>;
 
-    fn parse_value<'t>(&mut self, name: CowRcStr<'i>, input: &mut Parser<'i, 't>)
-                       -> Result<Importance, ParseError<'i>> {
-        let prop_context = PropertyParserContext::new(self.context);
-        let id = match PropertyId::parse(&name, Some(&prop_context)) {
+    fn parse_value<'t>(
+        &mut self,
+        name: CowRcStr<'i>,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Importance, ParseError<'i>> {
+        let id = match PropertyId::parse(&name) {
             Ok(id) => id,
-            Err(()) => {
+            Err(..) => {
                 return Err(input.new_custom_error(if is_non_mozilla_vendor_identifier(&name) {
                     StyleParseErrorKind::UnknownVendorProperty
                 } else {
@@ -1107,7 +1110,6 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
         };
         input.parse_until_before(Delimiter::Bang, |input| {
             PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input)
-                .map_err(|e| e.into())
         })?;
         let importance = match input.try(parse_important) {
             Ok(()) => Importance::Important,

components/style/properties/gecko.mako.rs

@@ -3560,7 +3560,7 @@ fn static_assert() {
                         Gecko_AppendWillChange(&mut self.gecko, feature.0.as_ptr());
                     }
 
-                    if let Ok(prop_id) = PropertyId::parse(&feature.0.to_string(), None) {
+                    if let Ok(prop_id) = PropertyId::parse(&feature.0.to_string()) {
                         match prop_id.as_shorthand() {
                             Ok(shorthand) => {
                                 for longhand in shorthand.longhands() {