Auto merge of #20138 - emilio:longhand-iterator, r=nox

style: Only expose longhands to rust via iterators.

The key here is that we only filter longhands if the shorthand is accessible to
content and vice-versa. This prevents the bug that prevented me to land this
patch before, which was us not expanding properly chrome-only shorthands.

<!-- 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/20138)
<!-- Reviewable:end -->

components/script/dom/webidls/CSSStyleDeclaration.webidl

@@ -387,19 +387,19 @@ partial interface CSSStyleDeclaration {
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString imageRendering;
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString image-rendering;
 
-  [Pref="layout.column-count.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString columnCount;
-  [Pref="layout.column-count.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString column-count;
-  [Pref="layout.column-width.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString columnWidth;
-  [Pref="layout.column-width.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString column-width;
   [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString columns;
-  [Pref="layout.column-gap.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString columnGap;
-  [Pref="layout.column-gap.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
+  [Pref="layout.columns.enabled", CEReactions, SetterThrows, TreatNullAs=EmptyString]
   attribute DOMString column-gap;
 
   [CEReactions, SetterThrows, TreatNullAs=EmptyString] attribute DOMString transition;

components/style/animation.rs

@@ -285,9 +285,9 @@ impl PropertyAnimation {
         match transition_property {
             TransitionProperty::Unsupported(_) => result,
             TransitionProperty::Shorthand(ref shorthand_id) => {
-                shorthand_id.longhands().iter().filter_map(|longhand| {
+                shorthand_id.longhands().filter_map(|longhand| {
                     PropertyAnimation::from_longhand(
-                        &longhand,
+                        longhand,
                         timing_function,
                         duration,
                         old_style,
@@ -295,7 +295,7 @@ impl PropertyAnimation {
                     )
                 }).collect()
             }
-            TransitionProperty::Longhand(ref longhand_id) => {
+            TransitionProperty::Longhand(longhand_id) => {
                 let animation = PropertyAnimation::from_longhand(
                     longhand_id,
                     timing_function,
@@ -313,7 +313,7 @@ impl PropertyAnimation {
     }
 
     fn from_longhand(
-        longhand: &LonghandId,
+        longhand: LonghandId,
         timing_function: TimingFunction,
         duration: Time,
         old_style: &ComputedValues,
@@ -751,7 +751,7 @@ where
                 debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"",
                        property, name);
                 let animation = PropertyAnimation::from_longhand(
-                    &property,
+                    property,
                     timing_function,
                     Time::from_seconds(relative_duration as f32),
                     &from_style,

components/style/gecko/wrapper.rs

@@ -773,7 +773,7 @@ impl<'le> GeckoElement<'le> {
 
     fn needs_transitions_update_per_property(
         &self,
-        longhand_id: &LonghandId,
+        longhand_id: LonghandId,
         combined_duration: f32,
         before_change_style: &ComputedValues,
         after_change_style: &ComputedValues,
@@ -787,7 +787,7 @@ impl<'le> GeckoElement<'le> {
         // If the end value has not changed, we should leave the currently
         // running transition as-is since we don't want to interrupt its timing
         // function.
-        if let Some(ref existing) = existing_transitions.get(longhand_id) {
+        if let Some(ref existing) = existing_transitions.get(&longhand_id) {
             let after_value =
                 AnimationValue::from_computed_values(
                     longhand_id,
@@ -798,11 +798,11 @@ impl<'le> GeckoElement<'le> {
         }
 
         let from = AnimationValue::from_computed_values(
-            &longhand_id,
+            longhand_id,
             before_change_style,
         );
         let to = AnimationValue::from_computed_values(
-            &longhand_id,
+            longhand_id,
             after_change_style,
         );
 
@@ -1531,8 +1531,8 @@ impl<'le> TElement for GeckoElement<'le> {
 
             let transition_property: TransitionProperty = property.into();
 
-            let mut property_check_helper = |property: &LonghandId| -> bool {
-                transitions_to_keep.insert(*property);
+            let mut property_check_helper = |property: LonghandId| -> bool {
+                transitions_to_keep.insert(property);
                 self.needs_transitions_update_per_property(
                     property,
                     combined_duration,
@@ -1545,11 +1545,11 @@ impl<'le> TElement for GeckoElement<'le> {
             match transition_property {
                 TransitionProperty::Unsupported(..) => {},
                 TransitionProperty::Shorthand(ref shorthand) => {
-                    if shorthand.longhands().iter().any(property_check_helper) {
+                    if shorthand.longhands().any(property_check_helper) {
                         return true;
                     }
                 },
-                TransitionProperty::Longhand(ref longhand_id) => {
+                TransitionProperty::Longhand(longhand_id) => {
                     if property_check_helper(longhand_id) {
                         return true;
                     }

components/style/properties/data.py

@@ -229,6 +229,11 @@ class Longhand(object):
     def enabled_in_content(self):
         return self.enabled_in == "content"
 
+    def may_be_disabled_in(self, shorthand, product):
+        if product == "gecko":
+            return self.gecko_pref and self.gecko_pref != shorthand.gecko_pref
+        return self.servo_pref and self.servo_pref != shorthand.servo_pref
+
     def base_type(self):
         if self.predefined_type and not self.is_vector:
             return "::values::specified::{}".format(self.predefined_type)

components/style/properties/declaration_block.rs

@@ -333,7 +333,7 @@ impl PropertyDeclarationBlock {
         let mut important_count = 0;
 
         // Step 1.2.2
-        for &longhand in shorthand.longhands() {
+        for longhand in shorthand.longhands() {
             // Step 1.2.2.1
             let declaration = self.get(PropertyDeclarationId::Longhand(longhand));
 
@@ -395,7 +395,7 @@ impl PropertyDeclarationBlock {
         match property.as_shorthand() {
             Ok(shorthand) => {
                 // Step 2.1 & 2.2 & 2.3
-                if shorthand.longhands().iter().all(|&l| {
+                if shorthand.longhands().all(|l| {
                     self.get(PropertyDeclarationId::Longhand(l))
                         .map_or(false, |(_, importance)| importance.important())
                 }) {
@@ -455,7 +455,7 @@ impl PropertyDeclarationBlock {
         match drain.all_shorthand {
             AllShorthand::NotSet => {}
             AllShorthand::CSSWideKeyword(keyword) => {
-                for &id in ShorthandId::All.longhands() {
+                for id in ShorthandId::All.longhands() {
                     let decl = PropertyDeclaration::CSSWideKeyword(
                         WideKeywordDeclaration { id, keyword },
                     );
@@ -467,7 +467,7 @@ impl PropertyDeclarationBlock {
                 }
             }
             AllShorthand::WithVariables(unparsed) => {
-                for &id in ShorthandId::All.longhands() {
+                for id in ShorthandId::All.longhands() {
                     let decl = PropertyDeclaration::WithVariables(
                         VariableDeclaration { id, value: unparsed.clone() },
                     );
@@ -809,7 +809,7 @@ impl PropertyDeclarationBlock {
             // iterating below.
 
             // Step 3.3.2
-            for &shorthand in longhand_id.shorthands() {
+            for shorthand in longhand_id.shorthands() {
                 // We already attempted to serialize this shorthand before.
                 if already_serialized.contains(shorthand.into()) {
                     continue;
@@ -853,7 +853,7 @@ impl PropertyDeclarationBlock {
                     }
                 } else {
                     let mut contains_all_longhands = true;
-                    for &longhand in shorthand.longhands() {
+                    for longhand in shorthand.longhands() {
                         match self.get(PropertyDeclarationId::Longhand(longhand)) {
                             Some((declaration, importance)) => {
                                 current_longhands.push(declaration);

components/style/properties/gecko.mako.rs

@@ -3472,7 +3472,7 @@ fn static_assert() {
         use properties::PropertyId;
         use properties::longhands::will_change::computed_value::T;
 
-        fn will_change_bitfield_from_prop_flags(prop: &LonghandId) -> u8 {
+        fn will_change_bitfield_from_prop_flags(prop: LonghandId) -> u8 {
             use properties::PropertyFlags;
             use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_ABSPOS_CB;
             use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_FIXPOS_CB;
@@ -3526,7 +3526,7 @@ fn static_assert() {
                                 if let PropertyDeclarationId::Longhand(longhand)
                                     = longhand_or_custom {
                                     self.gecko.mWillChangeBitField |=
-                                        will_change_bitfield_from_prop_flags(&longhand);
+                                        will_change_bitfield_from_prop_flags(longhand);
                                 }
                             },
                         }

components/style/properties/helpers.mako.rs

@@ -35,9 +35,10 @@
         % endif
         #[allow(unused_variables)]
         #[inline]
-        pub fn parse<'i, 't>(context: &ParserContext,
-                             input: &mut Parser<'i, 't>)
-                             -> Result<SpecifiedValue, ParseError<'i>> {
+        pub fn parse<'i, 't>(
+            context: &ParserContext,
+            input: &mut Parser<'i, 't>,
+        ) -> Result<SpecifiedValue, ParseError<'i>> {
             % if allow_quirks:
             specified::${type}::${parse_method}_quirky(context, input, AllowQuirks::Yes)
             % elif needs_context:
@@ -687,7 +688,13 @@
         pub struct LonghandsToSerialize<'a> {
             % for sub_property in shorthand.sub_properties:
                 pub ${sub_property.ident}:
+                % if sub_property.may_be_disabled_in(shorthand, product):
+                    Option<
+                % endif
                     &'a longhands::${sub_property.ident}::SpecifiedValue,
+                % if sub_property.may_be_disabled_in(shorthand, product):
+                    >,
+                % endif
             % endfor
         }
 
@@ -695,7 +702,8 @@
             /// Tries to get a serializable set of longhands given a set of
             /// property declarations.
             pub fn from_iter<I>(iter: I) -> Result<Self, ()>
-                where I: Iterator<Item=&'a PropertyDeclaration>,
+            where
+                I: Iterator<Item=&'a PropertyDeclaration>,
             {
                 // Define all of the expected variables that correspond to the shorthand
                 % for sub_property in shorthand.sub_properties:
@@ -723,12 +731,16 @@
 
                     (
                     % for sub_property in shorthand.sub_properties:
+                        % if sub_property.may_be_disabled_in(shorthand, product):
+                        ${sub_property.ident},
+                        % else:
                         Some(${sub_property.ident}),
+                        % endif
                     % endfor
                     ) =>
                     Ok(LonghandsToSerialize {
                         % for sub_property in shorthand.sub_properties:
-                            ${sub_property.ident}: ${sub_property.ident},
+                            ${sub_property.ident},
                         % endfor
                     }),
                     _ => Err(())
@@ -738,14 +750,24 @@
 
         /// Parse the given shorthand and fill the result into the
         /// `declarations` vector.
-        pub fn parse_into<'i, 't>(declarations: &mut SourcePropertyDeclaration,
-                                  context: &ParserContext, input: &mut Parser<'i, 't>)
-                                  -> Result<(), ParseError<'i>> {
+        pub fn parse_into<'i, 't>(
+            declarations: &mut SourcePropertyDeclaration,
+            context: &ParserContext,
+            input: &mut Parser<'i, 't>,
+        ) -> Result<(), ParseError<'i>> {
+            #[allow(unused_imports)]
+            use properties::{NonCustomPropertyId, LonghandId};
             input.parse_entirely(|input| parse_value(context, input)).map(|longhands| {
                 % for sub_property in shorthand.sub_properties:
+                % if sub_property.may_be_disabled_in(shorthand, product):
+                if NonCustomPropertyId::from(LonghandId::${sub_property.camel_case}).allowed_in(context) {
+                % endif
                     declarations.push(PropertyDeclaration::${sub_property.camel_case}(
                         longhands.${sub_property.ident}
                     ));
+                % if sub_property.may_be_disabled_in(shorthand, product):
+                }
+                % endif
                 % endfor
             })
         }

components/style/properties/helpers/animated_properties.mako.rs

@@ -21,7 +21,6 @@ use properties::longhands;
 use properties::longhands::font_weight::computed_value::T as FontWeight;
 use properties::longhands::font_stretch::computed_value::T as FontStretch;
 use properties::longhands::visibility::computed_value::T as Visibility;
-#[cfg(feature = "gecko")]
 use properties::PropertyId;
 use properties::{LonghandId, ShorthandId};
 use servo_arc::Arc;
@@ -113,15 +112,6 @@ impl TransitionProperty {
         TransitionProperty::Shorthand(ShorthandId::All)
     }
 
-    /// Iterates over each longhand property.
-    pub fn each<F: FnMut(&LonghandId) -> ()>(mut cb: F) {
-        % for prop in data.longhands:
-            % if prop.transitionable:
-                cb(&LonghandId::${prop.camel_case});
-            % endif
-        % endfor
-    }
-
     /// Parse a transition-property value.
     pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         // FIXME(https://github.com/rust-lang/rust/issues/33156): remove this
@@ -300,11 +290,11 @@ impl AnimatedProperty {
     /// Get an animatable value from a transition-property, an old style, and a
     /// new style.
     pub fn from_longhand(
-        property: &LonghandId,
+        property: LonghandId,
         old_style: &ComputedValues,
         new_style: &ComputedValues,
     ) -> Option<AnimatedProperty> {
-        Some(match *property {
+        Some(match property {
             % for prop in data.longhands:
             % if prop.animatable:
                 LonghandId::${prop.camel_case} => {
@@ -646,10 +636,10 @@ impl AnimationValue {
 
     /// Get an AnimationValue for an AnimatableLonghand from a given computed values.
     pub fn from_computed_values(
-        property: &LonghandId,
+        property: LonghandId,
         computed_values: &ComputedValues
     ) -> Option<Self> {
-        Some(match *property {
+        Some(match property {
             % for prop in data.longhands:
             % if prop.animatable:
             LonghandId::${prop.camel_case} => {
@@ -3088,15 +3078,17 @@ impl ComputeSquaredDistance for AnimatedFilterList {
 ///   border-top-color, border-color, border-top, border
 ///
 /// [property-order] https://drafts.csswg.org/web-animations/#calculating-computed-keyframes
-#[cfg(feature = "gecko")]
 pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Ordering {
     match (a.as_shorthand(), b.as_shorthand()) {
         // Within shorthands, sort by the number of subproperties, then by IDL name.
         (Ok(a), Ok(b)) => {
-            let subprop_count_a = a.longhands().len();
-            let subprop_count_b = b.longhands().len();
-            subprop_count_a.cmp(&subprop_count_b).then_with(
-                || get_idl_name_sort_order(&a).cmp(&get_idl_name_sort_order(&b)))
+            let subprop_count_a = a.longhands().count();
+            let subprop_count_b = b.longhands().count();
+            subprop_count_a
+                .cmp(&subprop_count_b)
+                .then_with(|| {
+                    get_idl_name_sort_order(a).cmp(&get_idl_name_sort_order(b))
+                })
         },
 
         // Longhands go before shorthands.
@@ -3109,8 +3101,7 @@ pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Orderin
     }
 }
 
-#[cfg(feature = "gecko")]
-fn get_idl_name_sort_order(shorthand: &ShorthandId) -> u32 {
+fn get_idl_name_sort_order(shorthand: ShorthandId) -> u32 {
 <%
 # Sort by IDL name.
 sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident))
@@ -3118,7 +3109,7 @@ sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident))
 # Annotate with sorted position
 sorted_shorthands = [(p, position) for position, p in enumerate(sorted_shorthands)]
 %>
-    match *shorthand {
+    match shorthand {
         % for property, position in sorted_shorthands:
             ShorthandId::${property.camel_case} => ${position},
         % endfor

components/style/properties/longhand/box.mako.rs

@@ -193,7 +193,7 @@ ${helpers.single_keyword("-servo-overflow-clip-box", "padding-box content-box",
         enabled_in="ua",
         needs_context=False,
         flags="APPLIES_TO_PLACEHOLDER",
-        gecko_pref="layout.css.overscroll-behavior.enabled",
+        gecko_pref="layout.css.overflow-clip-box.enabled",
         animation_value_type="discrete",
         spec="Internal, may be standardized in the future: \
               https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-clip-box",

components/style/properties/longhand/column.mako.rs

@@ -12,7 +12,7 @@ ${helpers.predefined_type("column-width",
                           initial_specified_value="Either::Second(Auto)",
                           extra_prefixes="moz",
                           animation_value_type="NonNegativeLengthOrAuto",
-                          servo_pref="layout.column-width.enabled",
+                          servo_pref="layout.columns.enabled",
                           spec="https://drafts.csswg.org/css-multicol/#propdef-column-width",
                           servo_restyle_damage="rebuild_and_reflow")}
 
@@ -22,7 +22,7 @@ ${helpers.predefined_type(
     "ColumnCount",
     "computed::ColumnCount::auto()",
     initial_specified_value="specified::ColumnCount::auto()",
-    servo_pref="layout.column-count.enabled",
+    servo_pref="layout.columns.enabled",
     animation_value_type="AnimatedColumnCount",
     extra_prefixes="moz",
     spec="https://drafts.csswg.org/css-multicol/#propdef-column-count",
@@ -33,7 +33,7 @@ ${helpers.predefined_type("column-gap",
                           "length::NonNegativeLengthOrNormal",
                           "Either::Second(Normal)",
                           extra_prefixes="moz",
-                          servo_pref="layout.column-gap.enabled",
+                          servo_pref="layout.columns.enabled",
                           animation_value_type="NonNegativeLengthOrNormal",
                           spec="https://drafts.csswg.org/css-multicol/#propdef-column-gap",
                           servo_restyle_damage = "reflow")}

components/style/properties/longhand/position.mako.rs

@@ -352,5 +352,4 @@ ${helpers.predefined_type("grid-template-areas",
                           initial_value="computed::GridTemplateAreas::none()",
                           products="gecko",
                           animation_value_type="discrete",
-                          gecko_pref="layout.css.grid.enabled",
                           spec="https://drafts.csswg.org/css-grid/#propdef-grid-template-areas")}

components/style/properties/properties.mako.rs

@@ -821,7 +821,7 @@ impl LonghandId {
         INHERITED.contains(*self)
     }
 
-    fn shorthands(&self) -> &'static [ShorthandId] {
+    fn shorthands(&self) -> NonCustomPropertyIterator<ShorthandId> {
         // first generate longhand to shorthands lookup map
         //
         // NOTE(emilio): This currently doesn't exclude the "all" shorthand. It
@@ -862,15 +862,21 @@ impl LonghandId {
             ];
         % endfor
 
-        match *self {
-            % for property in data.longhands:
-                LonghandId::${property.camel_case} => ${property.ident.upper()},
-            % endfor
+        NonCustomPropertyIterator {
+            filter: NonCustomPropertyId::from(*self).enabled_for_all_content(),
+            iter: match *self {
+                % for property in data.longhands:
+                    LonghandId::${property.camel_case} => ${property.ident.upper()},
+                % endfor
+            }.iter(),
         }
     }
 
-    fn parse_value<'i, 't>(&self, context: &ParserContext, input: &mut Parser<'i, 't>)
-                           -> Result<PropertyDeclaration, ParseError<'i>> {
+    fn parse_value<'i, 't>(
+        &self,
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<PropertyDeclaration, ParseError<'i>> {
         match *self {
             % for property in data.longhands:
                 LonghandId::${property.camel_case} => {
@@ -1100,6 +1106,29 @@ impl LonghandId {
     }
 }
 
+/// An iterator over all the property ids that are enabled for a given
+/// shorthand, if that shorthand is enabled for all content too.
+pub struct NonCustomPropertyIterator<Item: 'static> {
+    filter: bool,
+    iter: ::std::slice::Iter<'static, Item>,
+}
+
+impl<Item> Iterator for NonCustomPropertyIterator<Item>
+where
+    Item: 'static + Copy + Into<NonCustomPropertyId>,
+{
+    type Item = Item;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        loop {
+            let id = *self.iter.next()?;
+            if !self.filter || id.into().enabled_for_all_content() {
+                return Some(id)
+            }
+        }
+    }
+}
+
 /// An identifier for a given shorthand property.
 #[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToCss)]
 pub enum ShorthandId {
@@ -1127,7 +1156,7 @@ impl ShorthandId {
     }
 
     /// Get the longhand ids that form this shorthand.
-    pub fn longhands(&self) -> &'static [LonghandId] {
+    pub fn longhands(&self) -> NonCustomPropertyIterator<LonghandId> {
         % for property in data.shorthands:
             static ${property.ident.upper()}: &'static [LonghandId] = &[
                 % for sub in property.sub_properties:
@@ -1135,10 +1164,13 @@ impl ShorthandId {
                 % endfor
             ];
         % endfor
-        match *self {
-            % for property in data.shorthands:
-                ShorthandId::${property.camel_case} => ${property.ident.upper()},
-            % endfor
+        NonCustomPropertyIterator {
+            filter: NonCustomPropertyId::from(*self).enabled_for_all_content(),
+            iter: match *self {
+                % for property in data.shorthands:
+                    ShorthandId::${property.camel_case} => ${property.ident.upper()},
+                % endfor
+            }.iter()
         }
     }
 
@@ -1441,7 +1473,7 @@ impl<'a> PropertyDeclarationId<'a> {
     /// shorthand.
     pub fn is_longhand_of(&self, shorthand: ShorthandId) -> bool {
         match *self {
-            PropertyDeclarationId::Longhand(ref id) => id.shorthands().contains(&shorthand),
+            PropertyDeclarationId::Longhand(ref id) => id.shorthands().any(|s| s == shorthand),
             _ => false,
         }
     }
@@ -1952,7 +1984,7 @@ impl PropertyDeclaration {
                     if id == ShorthandId::All {
                         declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword)
                     } else {
-                        for &longhand in id.longhands() {
+                        for longhand in id.longhands() {
                             declarations.push(PropertyDeclaration::CSSWideKeyword(
                                 WideKeywordDeclaration {
                                     id: longhand,
@@ -1983,7 +2015,7 @@ impl PropertyDeclaration {
                             if id == ShorthandId::All {
                                 declarations.all_shorthand = AllShorthand::WithVariables(unparsed)
                             } else {
-                                for &id in id.longhands() {
+                                for id in id.longhands() {
                                     declarations.push(
                                         PropertyDeclaration::WithVariables(VariableDeclaration {
                                             id,