style: Fix style attribute important and revert-layer behavior

By modeling it as a separate layer that behaves somewhat specially.

See https://github.com/w3c/csswg-drafts/issues/6872.

The remaining revert-layer tests that we fail are because either we
don't implement a feature (like @property) or because it's used in
keyframes (where revert is a bit unspecified and we have existing
issues with it).

Differential Revision: https://phabricator.services.mozilla.com/D133373

components/style/applicable_declarations.rs

@@ -45,7 +45,9 @@ pub struct CascadePriority {
 #[allow(dead_code)]
 fn size_assert() {
     #[allow(unsafe_code)]
-    unsafe { std::mem::transmute::<u32, CascadePriority>(0u32) };
+    unsafe {
+        std::mem::transmute::<u32, CascadePriority>(0u32)
+    };
 }
 
 impl PartialOrd for CascadePriority {
@@ -57,34 +59,39 @@ impl PartialOrd for CascadePriority {
 
 impl Ord for CascadePriority {
     fn cmp(&self, other: &Self) -> std::cmp::Ordering {
-        self.cascade_level
+        self.cascade_level.cmp(&other.cascade_level).then_with(|| {
-            .cmp(&other.cascade_level)
+            let ordering = self.layer_order.cmp(&other.layer_order);
-            .then_with(|| {
+            if ordering == std::cmp::Ordering::Equal {
-                let ordering = self.layer_order.cmp(&other.layer_order);
+                return ordering;
-                // https://drafts.csswg.org/css-cascade-5/#cascade-layering
+            }
-                //
+            // https://drafts.csswg.org/css-cascade-5/#cascade-layering
-                //     Cascade layers (like declarations) are ordered by order
+            //
-                //     of appearance. When comparing declarations that belong to
+            //     Cascade layers (like declarations) are ordered by order
-                //     different layers, then for normal rules the declaration
+            //     of appearance. When comparing declarations that belong to
-                //     whose cascade layer is last wins, and for important rules
+            //     different layers, then for normal rules the declaration
-                //     the declaration whose cascade layer is first wins.
+            //     whose cascade layer is last wins, and for important rules
-                //
+            //     the declaration whose cascade layer is first wins.
-                // FIXME: This creates somewhat surprising behavior for the
+            //
-                // style attribute, see
+            // But the style attribute layer for some reason is special.
-                // https://github.com/w3c/csswg-drafts/issues/6872
+            if self.cascade_level.is_important() &&
-                if self.cascade_level.is_important() {
+                !self.layer_order.is_style_attribute_layer() &&
-                    ordering.reverse()
+                !other.layer_order.is_style_attribute_layer()
-                } else {
+            {
-                    ordering
+                ordering.reverse()
-                }
+            } else {
-            })
+                ordering
+            }
+        })
     }
 }
 
 impl CascadePriority {
     /// Construct a new CascadePriority for a given (level, order) pair.
     pub fn new(cascade_level: CascadeLevel, layer_order: LayerOrder) -> Self {
-        Self { cascade_level, layer_order }
+        Self {
+            cascade_level,
+            layer_order,
+        }
     }
 
     /// Returns the layer order.
@@ -149,12 +156,13 @@ impl ApplicableDeclarationBlock {
     pub fn from_declarations(
         declarations: Arc<Locked<PropertyDeclarationBlock>>,
         level: CascadeLevel,
+        layer_order: LayerOrder,
     ) -> Self {
         ApplicableDeclarationBlock {
             source: StyleSource::from_declarations(declarations),
             source_order: 0,
             specificity: 0,
-            cascade_priority: CascadePriority::new(level, LayerOrder::root()),
+            cascade_priority: CascadePriority::new(level, layer_order),
         }
     }
 

components/style/gecko/wrapper.rs

@@ -1579,6 +1579,7 @@ impl<'le> TElement for GeckoElement<'le> {
         use crate::properties::longhands::color::SpecifiedValue as SpecifiedColor;
         use crate::properties::longhands::text_align::SpecifiedValue as SpecifiedTextAlign;
         use crate::values::specified::color::Color;
+        use crate::stylesheets::layer_rule::LayerOrder;
         lazy_static! {
             static ref TH_RULE: ApplicableDeclarationBlock = {
                 let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -1587,7 +1588,7 @@ impl<'le> TElement for GeckoElement<'le> {
                     Importance::Normal,
                 );
                 let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb));
-                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints)
+                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root())
             };
             static ref TABLE_COLOR_RULE: ApplicableDeclarationBlock = {
                 let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -1596,7 +1597,7 @@ impl<'le> TElement for GeckoElement<'le> {
                     Importance::Normal,
                 );
                 let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb));
-                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints)
+                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root())
             };
             static ref MATHML_LANG_RULE: ApplicableDeclarationBlock = {
                 let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -1605,7 +1606,7 @@ impl<'le> TElement for GeckoElement<'le> {
                     Importance::Normal,
                 );
                 let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb));
-                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints)
+                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root())
             };
             static ref SVG_TEXT_DISABLE_ZOOM_RULE: ApplicableDeclarationBlock = {
                 let global_style_data = &*GLOBAL_STYLE_DATA;
@@ -1614,7 +1615,7 @@ impl<'le> TElement for GeckoElement<'le> {
                     Importance::Normal,
                 );
                 let arc = Arc::new_leaked(global_style_data.shared_lock.wrap(pdb));
-                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints)
+                ApplicableDeclarationBlock::from_declarations(arc, ServoCascadeLevel::PresHints, LayerOrder::root())
             };
         };
 
@@ -1642,6 +1643,7 @@ impl<'le> TElement for GeckoElement<'le> {
             hints.push(ApplicableDeclarationBlock::from_declarations(
                 decl.clone_arc(),
                 ServoCascadeLevel::PresHints,
+                LayerOrder::root(),
             ));
         }
         let declarations = unsafe { Gecko_GetExtraContentStyleDeclarations(self.0).as_ref() };
@@ -1651,6 +1653,7 @@ impl<'le> TElement for GeckoElement<'le> {
             hints.push(ApplicableDeclarationBlock::from_declarations(
                 decl.clone_arc(),
                 ServoCascadeLevel::PresHints,
+                LayerOrder::root(),
             ));
         }
 
@@ -1678,6 +1681,7 @@ impl<'le> TElement for GeckoElement<'le> {
                 hints.push(ApplicableDeclarationBlock::from_declarations(
                     decl.clone_arc(),
                     ServoCascadeLevel::PresHints,
+                    LayerOrder::root(),
                 ));
             }
 
@@ -1693,6 +1697,7 @@ impl<'le> TElement for GeckoElement<'le> {
                     hints.push(ApplicableDeclarationBlock::from_declarations(
                         decl.clone_arc(),
                         ServoCascadeLevel::PresHints,
+                        LayerOrder::root(),
                     ));
                 }
             }
@@ -1714,6 +1719,7 @@ impl<'le> TElement for GeckoElement<'le> {
             hints.push(ApplicableDeclarationBlock::from_declarations(
                 arc,
                 ServoCascadeLevel::PresHints,
+                LayerOrder::root(),
             ))
         }
         // MathML's default lang has precedence over both `lang` and `xml:lang`

components/style/rule_collector.rs

@@ -11,7 +11,7 @@ use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder};
 use crate::selector_map::SelectorMap;
 use crate::selector_parser::PseudoElement;
 use crate::shared_lock::Locked;
-use crate::stylesheets::Origin;
+use crate::stylesheets::{layer_rule::LayerOrder, Origin};
 use crate::stylist::{AuthorStylesEnabled, CascadeData, Rule, RuleInclusion, Stylist};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
 use servo_arc::ArcBorrow;
@@ -450,6 +450,7 @@ where
                 .push(ApplicableDeclarationBlock::from_declarations(
                     sa.clone_arc(),
                     CascadeLevel::same_tree_author_normal(),
+                    LayerOrder::style_attribute(),
                 ));
         }
     }
@@ -460,6 +461,7 @@ where
                 .push(ApplicableDeclarationBlock::from_declarations(
                     so.clone_arc(),
                     CascadeLevel::SMILOverride,
+                    LayerOrder::root(),
                 ));
         }
 
@@ -471,6 +473,7 @@ where
                 .push(ApplicableDeclarationBlock::from_declarations(
                     anim,
                     CascadeLevel::Animations,
+                    LayerOrder::root(),
                 ));
         }
 
@@ -481,6 +484,7 @@ where
                 .push(ApplicableDeclarationBlock::from_declarations(
                     anim,
                     CascadeLevel::Transitions,
+                    LayerOrder::root(),
                 ));
         }
     }

components/style/rule_tree/mod.rs

@@ -72,7 +72,9 @@ impl RuleTree {
             if any_important {
                 found_important = true;
                 match level {
-                    AuthorNormal { .. } => important_author.push((source.clone(), priority.important())),
+                    AuthorNormal { .. } => {
+                        important_author.push((source.clone(), priority.important()))
+                    },
                     UANormal => important_ua.push((source.clone(), priority.important())),
                     UserNormal => important_user.push((source.clone(), priority.important())),
                     _ => {},

components/style/stylesheets/layer_rule.rs

@@ -28,9 +28,25 @@ pub struct LayerOrder(u16);
 impl LayerOrder {
     /// The order of the root layer.
     pub const fn root() -> Self {
+        Self(std::u16::MAX - 1)
+    }
+
+    /// The order of the style attribute layer.
+    pub const fn style_attribute() -> Self {
         Self(std::u16::MAX)
     }
 
+    /// Returns whether this layer is for the style attribute, which behaves
+    /// differently in terms of !important, see
+    /// https://github.com/w3c/csswg-drafts/issues/6872
+    ///
+    /// (This is a bit silly, mind-you, but it's needed so that revert-layer
+    /// behaves correctly).
+    #[inline]
+    pub fn is_style_attribute_layer(&self) -> bool {
+        *self == Self::style_attribute()
+    }
+
     /// The first cascade layer order.
     pub const fn first() -> Self {
         Self(0)
@@ -39,7 +55,7 @@ impl LayerOrder {
     /// Increment the cascade layer order.
     #[inline]
     pub fn inc(&mut self) {
-        if self.0 != std::u16::MAX {
+        if self.0 != std::u16::MAX - 1 {
             self.0 += 1;
         }
     }