Bug 1458192: Make ShadowCascadeOrder a newtype. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 01 May 2018 09:15:21 +0200
changeset 797680 b0c7f3b26f2433671cafbb6d3eee0188d40b6913
parent 797378 e1fafe357b36270b835acf86f17aef855ce76fa5
child 797681 f9adf3127e2aebadcbedd665ed184363815b8517
push id110536
push userbmo:emilio@crisal.io
push dateMon, 21 May 2018 11:43:32 +0000
bugs1458192
milestone62.0a1
Bug 1458192: Make ShadowCascadeOrder a newtype. MozReview-Commit-ID: EhCwMMNGM6S
servo/components/style/applicable_declarations.rs
servo/components/style/rule_tree/mod.rs
servo/components/style/stylist.rs
--- a/servo/components/style/applicable_declarations.rs
+++ b/servo/components/style/applicable_declarations.rs
@@ -56,27 +56,27 @@ impl ApplicableDeclarationBits {
         cascade_level: CascadeLevel,
         shadow_cascade_order: ShadowCascadeOrder,
     ) -> Self {
         debug_assert!(
             cascade_level as u8 <= CASCADE_LEVEL_MAX,
             "Gotta find more bits!"
         );
         let mut bits = ::std::cmp::min(source_order, SOURCE_ORDER_MAX);
-        bits |= ((shadow_cascade_order & SHADOW_CASCADE_ORDER_MAX) as u32) << SHADOW_CASCADE_ORDER_SHIFT;
+        bits |= ((shadow_cascade_order.unwrap() & SHADOW_CASCADE_ORDER_MAX) as u32) << SHADOW_CASCADE_ORDER_SHIFT;
         bits |= (cascade_level as u8 as u32) << CASCADE_LEVEL_SHIFT;
         ApplicableDeclarationBits(bits)
     }
 
     fn source_order(&self) -> u32 {
         (self.0 & SOURCE_ORDER_MASK) >> SOURCE_ORDER_SHIFT
     }
 
     fn shadow_cascade_order(&self) -> ShadowCascadeOrder {
-        ((self.0 & SHADOW_CASCADE_ORDER_MASK) >> SHADOW_CASCADE_ORDER_SHIFT) as ShadowCascadeOrder
+        ShadowCascadeOrder::wrap(((self.0 & SHADOW_CASCADE_ORDER_MASK) >> SHADOW_CASCADE_ORDER_SHIFT) as u8)
     }
 
     fn level(&self) -> CascadeLevel {
         let byte = ((self.0 & CASCADE_LEVEL_MASK) >> CASCADE_LEVEL_SHIFT) as u8;
         unsafe { CascadeLevel::from_byte(byte) }
     }
 }
 
@@ -112,17 +112,17 @@ impl ApplicableDeclarationBlock {
     /// declaration block and importance.
     #[inline]
     pub fn from_declarations(
         declarations: Arc<Locked<PropertyDeclarationBlock>>,
         level: CascadeLevel,
     ) -> Self {
         ApplicableDeclarationBlock {
             source: StyleSource::from_declarations(declarations),
-            bits: ApplicableDeclarationBits::new(0, level, 0),
+            bits: ApplicableDeclarationBits::new(0, level, ShadowCascadeOrder::same_tree()),
             specificity: 0,
         }
     }
 
     /// Constructs an applicable declaration block from the given components
     #[inline]
     pub fn new(
         source: StyleSource,
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -155,25 +155,74 @@ impl StyleSource {
 ///
 /// The root node doesn't have a null pointer in the free list, but this value.
 const FREE_LIST_SENTINEL: *mut RuleNode = 0x01 as *mut RuleNode;
 
 /// A second sentinel value for the free list, indicating that it's locked (i.e.
 /// another thread is currently adding an entry). We spin if we find this value.
 const FREE_LIST_LOCKED: *mut RuleNode = 0x02 as *mut RuleNode;
 
-/// A counter to track how many inner shadow roots rules deep we are.
+/// A counter to track how many inner shadow roots rules deep we are, or whether
+/// we're in the same tree and such.
 ///
 /// This is used to handle:
 ///
 /// https://drafts.csswg.org/css-scoping/#shadow-cascading
 ///
-/// In particular, it'd be `0` for the innermost shadow host, `1` for the next,
-/// and so on.
-pub type ShadowCascadeOrder = u8;
+/// This needs a couple special values:
+///
+///  * One for rules coming from the same tree the element is in.
+///  * One for rules coming from the element's shadow root (effectively `:host`
+///    rules).
+///
+/// The other values are for `::slotted(..)` rules, and they increment as you go
+/// up the slot chain.
+#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)]
+pub struct ShadowCascadeOrder(u8);
+
+impl ShadowCascadeOrder {
+    /// A value that represents that these rules come from the same tree the
+    /// element is in.
+    #[inline]
+    pub fn same_tree() -> Self {
+        ShadowCascadeOrder(0)
+    }
+
+    /// A value that represents that these rules come from :host rules.
+    #[inline]
+    pub fn host() -> Self {
+        ShadowCascadeOrder(1)
+    }
+
+    /// A value that represents that these rules come from the first assigned
+    /// slot's shadow root.
+    #[inline]
+    pub fn first_slot() -> Self {
+        ShadowCascadeOrder(2)
+    }
+
+    /// Increments the counter for the next slot.
+    #[inline]
+    pub fn next_slot(self) -> Self {
+        debug_assert!(self.0 >= 2, "Need to be for an slot in the first place");
+        ShadowCascadeOrder(self.0 + 1)
+    }
+
+    /// Unwraps the value, for FFI and packing in ApplicableDeclarationBlock.
+    #[inline]
+    pub fn unwrap(self) -> u8 {
+        self.0
+    }
+
+    /// Wraps the value, for FFI and packing in ApplicableDeclarationBlock.
+    #[inline]
+    pub fn wrap(raw: u8) -> Self {
+        ShadowCascadeOrder(raw)
+    }
+}
 
 impl RuleTree {
     /// Construct a new rule tree.
     pub fn new() -> Self {
         RuleTree {
             root: StrongRuleNode::new(Box::new(RuleNode::root())),
         }
     }
@@ -218,17 +267,17 @@ impl RuleTree {
         let mut important_same_tree = SmallVec::<[StyleSource; 4]>::new();
         let mut important_inner_shadow = SmallVec::<[SmallVec<[StyleSource; 4]>; 4]>::new();
         important_inner_shadow.push(SmallVec::new());
 
         let mut important_user = SmallVec::<[StyleSource; 4]>::new();
         let mut important_ua = SmallVec::<[StyleSource; 4]>::new();
         let mut transition = None;
 
-        let mut last_cascade_order = 0;
+        let mut last_cascade_order = ShadowCascadeOrder::same_tree();
         for (source, level, shadow_cascade_order) in iter {
             debug_assert!(level >= last_level, "Not really ordered");
             debug_assert!(!level.is_important(), "Important levels handled internally");
             let any_important = {
                 let pdb = source.read(level.guard(guards));
                 pdb.any_important()
             };
 
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -1182,17 +1182,17 @@ impl Stylist {
         {
             map.get_all_matching_rules(
                 element,
                 rule_hash_target,
                 applicable_declarations,
                 context,
                 flags_setter,
                 CascadeLevel::UANormal,
-                0,
+                ShadowCascadeOrder::same_tree(),
             );
         }
 
         // NB: the following condition, although it may look somewhat
         // inaccurate, would be equivalent to something like:
         //
         //     element.matches_user_and_author_rules() ||
         //     (is_implemented_pseudo &&
@@ -1204,17 +1204,17 @@ impl Stylist {
             if let Some(map) = self.cascade_data.user.normal_rules(pseudo_element) {
                 map.get_all_matching_rules(
                     element,
                     rule_hash_target,
                     applicable_declarations,
                     context,
                     flags_setter,
                     CascadeLevel::UserNormal,
-                    0,
+                    ShadowCascadeOrder::same_tree(),
                 );
             }
         }
 
         if pseudo_element.is_none() && !only_default_rules {
             // Presentational hints.
             //
             // These go before author rules, but after user rules, see:
@@ -1229,17 +1229,16 @@ impl Stylist {
                     for declaration in &applicable_declarations[length_before_preshints..] {
                         assert_eq!(declaration.level(), CascadeLevel::PresHints);
                     }
                 }
             }
         }
 
         let mut match_document_author_rules = matches_author_rules;
-        let mut shadow_cascade_order = 0;
 
         // XBL / Shadow DOM rules, which are author rules too.
         //
         // TODO(emilio): Cascade order here is wrong for Shadow DOM. In
         // particular, normally document rules override ::slotted() rules, but
         // for !important it should be the other way around. So probably we need
         // to add some sort of AuthorScoped cascade level or something.
         if matches_author_rules && !only_default_rules {
@@ -1248,66 +1247,65 @@ impl Stylist {
                     context.with_shadow_host(Some(rule_hash_target), |context| {
                         map.get_all_matching_rules(
                             element,
                             rule_hash_target,
                             applicable_declarations,
                             context,
                             flags_setter,
                             CascadeLevel::InnerShadowNormal,
-                            shadow_cascade_order,
+                            ShadowCascadeOrder::host(),
                         );
                     });
-                    shadow_cascade_order += 1;
                 }
             }
 
             // Match slotted rules in reverse order, so that the outer slotted
             // rules come before the inner rules (and thus have less priority).
             let mut slots = SmallVec::<[_; 3]>::new();
             let mut current = rule_hash_target.assigned_slot();
             while let Some(slot) = current {
                 slots.push(slot);
                 current = slot.assigned_slot();
             }
 
+            let mut order = ShadowCascadeOrder::first_slot();
             for slot in slots.iter().rev() {
                 let shadow = slot.containing_shadow().unwrap();
                 let styles = shadow.style_data();
                 if let Some(map) = styles.slotted_rules(pseudo_element) {
                     context.with_shadow_host(Some(shadow.host()), |context| {
                         map.get_all_matching_rules(
                             element,
                             rule_hash_target,
                             applicable_declarations,
                             context,
                             flags_setter,
                             CascadeLevel::InnerShadowNormal,
-                            shadow_cascade_order,
+                            order,
                         );
                     });
-                    shadow_cascade_order += 1;
                 }
+                order = order.next_slot();
             }
 
             if let Some(containing_shadow) = rule_hash_target.containing_shadow() {
                 let cascade_data = containing_shadow.style_data();
                 if let Some(map) = cascade_data.normal_rules(pseudo_element) {
                     context.with_shadow_host(Some(containing_shadow.host()), |context| {
                         map.get_all_matching_rules(
                             element,
                             rule_hash_target,
                             applicable_declarations,
                             context,
                             flags_setter,
                             CascadeLevel::SameTreeAuthorNormal,
-                            shadow_cascade_order,
+                            ShadowCascadeOrder::same_tree(),
                         );
                     });
-                    shadow_cascade_order += 1;
                 }
 
                 match_document_author_rules = false;
             }
         }
 
         // FIXME(emilio): It looks very wrong to match XBL rules even for
         // getDefaultComputedStyle!
@@ -1331,34 +1329,34 @@ impl Stylist {
                     // preserve behavior, though that's kinda fishy...
                     map.get_all_matching_rules(
                         element,
                         rule_hash_target,
                         applicable_declarations,
                         &mut matching_context,
                         flags_setter,
                         CascadeLevel::SameTreeAuthorNormal,
-                        shadow_cascade_order,
+                        ShadowCascadeOrder::same_tree(),
                     );
                 }
             });
 
         match_document_author_rules &= !cut_xbl_binding_inheritance;
 
         if match_document_author_rules && !only_default_rules {
             // Author normal rules.
             if let Some(map) = self.cascade_data.author.normal_rules(pseudo_element) {
                 map.get_all_matching_rules(
                     element,
                     rule_hash_target,
                     applicable_declarations,
                     context,
                     flags_setter,
                     CascadeLevel::SameTreeAuthorNormal,
-                    shadow_cascade_order,
+                    ShadowCascadeOrder::same_tree(),
                 );
             }
         }
 
         if !only_default_rules {
             // Style attribute ("Normal override declarations").
             if let Some(sa) = style_attribute {
                 applicable_declarations.push(ApplicableDeclarationBlock::from_declarations(
@@ -2213,17 +2211,17 @@ impl CascadeData {
                                     .as_mut()
                                     .expect("Expected precomputed declarations for the UA level")
                                     .get_or_insert_with(&pseudo.canonical(), Vec::new)
                                     .push(ApplicableDeclarationBlock::new(
                                         StyleSource::from_rule(locked.clone()),
                                         self.rules_source_order,
                                         CascadeLevel::UANormal,
                                         selector.specificity(),
-                                        0,
+                                        ShadowCascadeOrder::same_tree(),
                                     ));
                                 continue;
                             }
                         }
 
                         let hashes = AncestorHashes::new(&selector, quirks_mode);
 
                         let rule = Rule::new(