style: Update StyleSource to use ArcUnion.

Bug: 1455784
Reviewed-by: Manishearth
MozReview-Commit-ID: AT4sud9goGV

components/style/applicable_declarations.rs

@@ -116,7 +116,7 @@ impl ApplicableDeclarationBlock {
         level: CascadeLevel,
     ) -> Self {
         ApplicableDeclarationBlock {
-            source: StyleSource::Declarations(declarations),
+            source: StyleSource::from_declarations(declarations),
             bits: ApplicableDeclarationBits::new(0, level, 0),
             specificity: 0,
         }

components/style/properties/properties.mako.rs

@@ -3424,7 +3424,7 @@ where
             let source = node.style_source();
 
             let declarations = if source.is_some() {
-                source.read(cascade_level.guard(guards)).declaration_importance_iter()
+                source.as_ref().unwrap().read(cascade_level.guard(guards)).declaration_importance_iter()
             } else {
                 // The root node has no style source.
                 DeclarationImportanceIterator::new(&[], &empty)

components/style/rule_cache.rs

@@ -8,7 +8,7 @@
 use fnv::FnvHashMap;
 use logical_geometry::WritingMode;
 use properties::{ComputedValues, StyleBuilder};
-use rule_tree::{StrongRuleNode, StyleSource};
+use rule_tree::StrongRuleNode;
 use selector_parser::PseudoElement;
 use servo_arc::Arc;
 use shared_lock::StylesheetGuards;
@@ -97,16 +97,18 @@ impl RuleCache {
         mut rule_node: Option<&'r StrongRuleNode>,
     ) -> Option<&'r StrongRuleNode> {
         while let Some(node) = rule_node {
-            match *node.style_source() {
-                StyleSource::Declarations(ref decls) => {
-                    let cascade_level = node.cascade_level();
-                    let decls = decls.read_with(cascade_level.guard(guards));
-                    if decls.contains_any_reset() {
-                        break;
-                    }
+            match node.style_source() {
+                Some(s) => match s.as_declarations() {
+                    Some(decls) => {
+                        let cascade_level = node.cascade_level();
+                        let decls = decls.read_with(cascade_level.guard(guards));
+                        if decls.contains_any_reset() {
+                            break;
+                        }
+                    },
+                    None => break,
                 },
-                StyleSource::None => {},
-                StyleSource::Style(_) => break,
+                None => {},
             }
             rule_node = node.parent();
         }

components/style/rule_tree/mod.rs

@@ -12,7 +12,7 @@ use gecko::selector_parser::PseudoElement;
 #[cfg(feature = "gecko")]
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
-use servo_arc::{Arc, ArcBorrow, NonZeroPtrMut};
+use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow, NonZeroPtrMut};
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use smallvec::SmallVec;
 use std::io::{self, Write};
@@ -89,40 +89,27 @@ impl MallocSizeOf for RuleTree {
 /// more debuggability, and also the ability of show those selectors to
 /// devtools.
 #[derive(Clone, Debug)]
-pub enum StyleSource {
-    /// A style rule stable pointer.
-    Style(Arc<Locked<StyleRule>>),
-    /// A declaration block stable pointer.
-    Declarations(Arc<Locked<PropertyDeclarationBlock>>),
-    /// Indicates no style source. Used to save an Option wrapper around the stylesource in
-    /// RuleNode
-    None,
-}
+pub struct StyleSource(ArcUnion<Locked<StyleRule>, Locked<PropertyDeclarationBlock>>);
 
 impl PartialEq for StyleSource {
     fn eq(&self, other: &Self) -> bool {
-        self.ptr_equals(other)
+        ArcUnion::ptr_eq(&self.0, &other.0)
     }
 }
 
 impl StyleSource {
-    #[inline]
-    fn ptr_equals(&self, other: &Self) -> bool {
-        use self::StyleSource::*;
-        match (self, other) {
-            (&Style(ref one), &Style(ref other)) => Arc::ptr_eq(one, other),
-            (&Declarations(ref one), &Declarations(ref other)) => Arc::ptr_eq(one, other),
-            (&None, _) | (_, &None) => {
-                panic!("Should not check for equality between null StyleSource objects")
-            },
-            _ => false,
-        }
+    /// Creates a StyleSource from a StyleRule.
+    pub fn from_rule(rule: Arc<Locked<StyleRule>>) -> Self {
+        StyleSource(ArcUnion::from_first(rule))
+    }
+
+    /// Creates a StyleSource from a PropertyDeclarationBlock.
+    pub fn from_declarations(decls: Arc<Locked<PropertyDeclarationBlock>>) -> Self {
+        StyleSource(ArcUnion::from_second(decls))
     }
 
     fn dump<W: Write>(&self, guard: &SharedRwLockReadGuard, writer: &mut W) {
-        use self::StyleSource::*;
-
-        if let Style(ref rule) = *self {
+        if let Some(ref rule) = self.0.as_first() {
             let rule = rule.read_with(guard);
             let _ = write!(writer, "{:?}", rule.selectors);
         }
@@ -134,20 +121,31 @@ impl StyleSource {
     /// underlying property declaration block.
     #[inline]
     pub fn read<'a>(&'a self, guard: &'a SharedRwLockReadGuard) -> &'a PropertyDeclarationBlock {
-        let block = match *self {
-            StyleSource::Style(ref rule) => &rule.read_with(guard).block,
-            StyleSource::Declarations(ref block) => block,
-            StyleSource::None => panic!("Cannot call read on StyleSource::None"),
+        let block: &Locked<PropertyDeclarationBlock> = match self.0.borrow() {
+            ArcUnionBorrow::First(ref rule) => &rule.get().read_with(guard).block,
+            ArcUnionBorrow::Second(ref block) => block.get(),
         };
         block.read_with(guard)
     }
 
-    /// Indicates if this StyleSource has a value
-    pub fn is_some(&self) -> bool {
-        match *self {
-            StyleSource::None => false,
-            _ => true,
-        }
+    /// Indicates if this StyleSource is a style rule.
+    pub fn is_rule(&self) -> bool {
+        self.0.is_first()
+    }
+
+    /// Indicates if this StyleSource is a PropertyDeclarationBlock.
+    pub fn is_declarations(&self) -> bool {
+        self.0.is_second()
+    }
+
+    /// Returns the style rule if applicable, otherwise None.
+    pub fn as_rule(&self) -> Option<ArcBorrow<Locked<StyleRule>>> {
+        self.0.as_first()
+    }
+
+    /// Returns the declaration block if applicable, otherwise None.
+    pub fn as_declarations(&self) -> Option<ArcBorrow<Locked<PropertyDeclarationBlock>>> {
+        self.0.as_second()
     }
 }
 
@@ -248,11 +246,12 @@ impl RuleTree {
                             last_cascade_order = shadow_cascade_order;
                             important_inner_shadow.push(SmallVec::new());
                         }
-                        important_inner_shadow.last_mut().unwrap().push(source.clone())
-                    }
-                    SameTreeAuthorNormal => {
-                        important_same_tree.push(source.clone())
+                        important_inner_shadow
+                            .last_mut()
+                            .unwrap()
+                            .push(source.clone())
                     },
+                    SameTreeAuthorNormal => important_same_tree.push(source.clone()),
                     UANormal => important_ua.push(source.clone()),
                     UserNormal => important_user.push(source.clone()),
                     StyleAttributeNormal => {
@@ -391,7 +390,10 @@ impl RuleTree {
         // First walk up until the first less-or-equally specific rule.
         let mut children = SmallVec::<[_; 10]>::new();
         while current.get().level > level {
-            children.push((current.get().source.clone(), current.get().level));
+            children.push((
+                current.get().source.as_ref().unwrap().clone(),
+                current.get().level,
+            ));
             current = current.parent().unwrap().clone();
         }
 
@@ -418,13 +420,14 @@ impl RuleTree {
                 // also equally valid. This is less likely, and would require an
                 // in-place mutation of the source, which is, at best, fiddly,
                 // so let's skip it for now.
-                let is_here_already = match current.get().source {
-                    StyleSource::Declarations(ref already_here) => {
-                        pdb.with_arc(|arc| Arc::ptr_eq(arc, already_here))
-                    },
-                    _ => unreachable!("Replacing non-declarations style?"),
-                };
-
+                let current_decls = current
+                    .get()
+                    .source
+                    .as_ref()
+                    .unwrap()
+                    .as_declarations()
+                    .expect("Replacing non-declarations style?");
+                let is_here_already = ArcBorrow::ptr_eq(&pdb, &current_decls);
                 if is_here_already {
                     debug!("Picking the fast path in rule replacement");
                     return None;
@@ -447,7 +450,7 @@ impl RuleTree {
                 if pdb.read_with(level.guard(guards)).any_important() {
                     current = current.ensure_child(
                         self.root.downgrade(),
-                        StyleSource::Declarations(pdb.clone_arc()),
+                        StyleSource::from_declarations(pdb.clone_arc()),
                         level,
                     );
                 }
@@ -455,7 +458,7 @@ impl RuleTree {
                 if pdb.read_with(level.guard(guards)).any_normal() {
                     current = current.ensure_child(
                         self.root.downgrade(),
-                        StyleSource::Declarations(pdb.clone_arc()),
+                        StyleSource::from_declarations(pdb.clone_arc()),
                         level,
                     );
                 }
@@ -491,7 +494,10 @@ impl RuleTree {
         let mut children = SmallVec::<[_; 10]>::new();
         for node in iter {
             if !node.cascade_level().is_animation() {
-                children.push((node.get().source.clone(), node.cascade_level()));
+                children.push((
+                    node.get().source.as_ref().unwrap().clone(),
+                    node.cascade_level(),
+                ));
             }
             last = node;
         }
@@ -689,7 +695,9 @@ pub struct RuleNode {
 
     /// The actual style source, either coming from a selector in a StyleRule,
     /// or a raw property declaration block (like the style attribute).
-    source: StyleSource,
+    ///
+    /// None for the root node.
+    source: Option<StyleSource>,
 
     /// The cascade level this rule is positioned at.
     level: CascadeLevel,
@@ -775,7 +783,7 @@ impl RuleNode {
         RuleNode {
             root: Some(root),
             parent: Some(parent),
-            source: source,
+            source: Some(source),
             level: level,
             refcount: AtomicUsize::new(1),
             first_child: AtomicPtr::new(ptr::null_mut()),
@@ -789,7 +797,7 @@ impl RuleNode {
         RuleNode {
             root: None,
             parent: None,
-            source: StyleSource::None,
+            source: None,
             level: CascadeLevel::UANormal,
             refcount: AtomicUsize::new(1),
             first_child: AtomicPtr::new(ptr::null_mut()),
@@ -884,7 +892,10 @@ impl RuleNode {
         }
 
         if self.source.is_some() {
-            self.source.dump(self.level.guard(guards), writer);
+            self.source
+                .as_ref()
+                .unwrap()
+                .dump(self.level.guard(guards), writer);
         } else {
             if indent != 0 {
                 warn!("How has this happened?");
@@ -986,7 +997,7 @@ impl StrongRuleNode {
         // WeakRuleNode, and implementing this on WeakRuleNode itself...
         for child in self.get().iter_children() {
             let child_node = unsafe { &*child.ptr() };
-            if child_node.level == level && child_node.source.ptr_equals(&source) {
+            if child_node.level == level && child_node.source.as_ref().unwrap() == &source {
                 return child.upgrade();
             }
             last = Some(child);
@@ -1026,7 +1037,7 @@ impl StrongRuleNode {
                 // we accessed `last`.
                 next = WeakRuleNode::from_ptr(existing);
 
-                if unsafe { &*next.ptr() }.source.ptr_equals(&source) {
+                if unsafe { &*next.ptr() }.source.as_ref().unwrap() == &source {
                     // That node happens to be for the same style source, use
                     // that, and let node fall out of scope.
                     return next.upgrade();
@@ -1054,8 +1065,8 @@ impl StrongRuleNode {
     /// Get the style source corresponding to this rule node. May return `None`
     /// if it's the root node, which means that the node hasn't matched any
     /// rules.
-    pub fn style_source(&self) -> &StyleSource {
-        &self.get().source
+    pub fn style_source(&self) -> Option<&StyleSource> {
+        self.get().source.as_ref()
     }
 
     /// The cascade level for this node
@@ -1317,6 +1328,8 @@ impl StrongRuleNode {
                 let source = node.style_source();
                 let declarations = if source.is_some() {
                     source
+                        .as_ref()
+                        .unwrap()
                         .read(node.cascade_level().guard(guards))
                         .declaration_importance_iter()
                 } else {
@@ -1444,7 +1457,7 @@ impl StrongRuleNode {
             .take_while(|node| node.cascade_level() > CascadeLevel::Animations);
         let mut result = (LonghandIdSet::new(), false);
         for node in iter {
-            let style = node.style_source();
+            let style = node.style_source().unwrap();
             for (decl, important) in style
                 .read(node.cascade_level().guard(guards))
                 .declaration_importance_iter()
@@ -1464,33 +1477,6 @@ impl StrongRuleNode {
         }
         result
     }
-
-    /// Returns PropertyDeclarationBlock for this node.
-    /// This function must be called only for animation level node.
-    fn get_animation_style(&self) -> &Arc<Locked<PropertyDeclarationBlock>> {
-        debug_assert!(
-            self.cascade_level().is_animation(),
-            "The cascade level should be an animation level"
-        );
-        match *self.style_source() {
-            StyleSource::Declarations(ref block) => block,
-            StyleSource::Style(_) => unreachable!("animating style should not be a style rule"),
-            StyleSource::None => unreachable!("animating style should not be none"),
-        }
-    }
-
-    /// Returns SMIL override declaration block if exists.
-    pub fn get_smil_animation_rule(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
-        if cfg!(feature = "servo") {
-            // Servo has no knowledge of a SMIL rule, so just avoid looking for it.
-            return None;
-        }
-
-        self.self_and_ancestors()
-            .take_while(|node| node.cascade_level() >= CascadeLevel::SMILOverride)
-            .find(|node| node.cascade_level() == CascadeLevel::SMILOverride)
-            .map(|node| node.get_animation_style())
-    }
 }
 
 /// An iterator over a rule node and its ancestors.

components/style/stylist.rs

@@ -1523,7 +1523,7 @@ impl Stylist {
         // just avoid allocating it and calling `apply_declarations` directly,
         // maybe...
         let rule_node = self.rule_tree.insert_ordered_rules(iter::once((
-            StyleSource::Declarations(declarations),
+            StyleSource::from_declarations(declarations),
             CascadeLevel::StyleAttributeNormal,
         )));
 
@@ -2177,7 +2177,7 @@ impl CascadeData {
                                     .expect("Expected precomputed declarations for the UA level")
                                     .get_or_insert_with(&pseudo.canonical(), Vec::new)
                                     .push(ApplicableDeclarationBlock::new(
-                                        StyleSource::Style(locked.clone()),
+                                        StyleSource::from_rule(locked.clone()),
                                         self.rules_source_order,
                                         CascadeLevel::UANormal,
                                         selector.specificity(),
@@ -2480,7 +2480,7 @@ impl Rule {
         level: CascadeLevel,
         shadow_cascade_order: ShadowCascadeOrder,
     ) -> ApplicableDeclarationBlock {
-        let source = StyleSource::Style(self.style_rule.clone());
+        let source = StyleSource::from_rule(self.style_rule.clone());
         ApplicableDeclarationBlock::new(source, self.source_order, level, self.specificity(), shadow_cascade_order)
     }
 

ports/geckolib/glue.rs

@@ -137,7 +137,7 @@ use style::properties::{parse_one_declaration_into, parse_style_attribute};
 use style::properties::animated_properties::AnimationValue;
 use style::properties::animated_properties::compare_property_priority;
 use style::rule_cache::RuleCacheConditions;
-use style::rule_tree::{CascadeLevel, StrongRuleNode, StyleSource};
+use style::rule_tree::{CascadeLevel, StrongRuleNode};
 use style::selector_parser::{PseudoElementCascadeType, SelectorImpl};
 use style::shared_lock::{SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::{Atom, WeakAtom};
@@ -3123,8 +3123,8 @@ pub extern "C" fn Servo_ComputedValues_GetStyleRuleList(
 
     let mut result = SmallVec::<[_; 10]>::new();
     for node in rule_node.self_and_ancestors() {
-        let style_rule = match *node.style_source() {
-            StyleSource::Style(ref rule) => rule,
+        let style_rule = match node.style_source().and_then(|x| x.as_rule()) {
+            Some(rule) => rule,
             _ => continue,
         };
 
@@ -3141,9 +3141,11 @@ pub extern "C" fn Servo_ComputedValues_GetStyleRuleList(
 
     unsafe { rules.set_len(result.len() as u32) };
     for (ref src, ref mut dest) in result.into_iter().zip(rules.iter_mut()) {
-        src.with_raw_offset_arc(|arc| {
-            **dest = *Locked::<StyleRule>::arc_as_borrowed(arc);
-        })
+        src.with_arc(|a| {
+            a.with_raw_offset_arc(|arc| {
+                **dest = *Locked::<StyleRule>::arc_as_borrowed(arc);
+            })
+        });
     }
 }
 

ports/geckolib/tests/size_of.rs

@@ -35,8 +35,8 @@ size_of_test!(test_size_of_element_data, ElementData, 24);
 
 size_of_test!(test_size_of_property_declaration, style::properties::PropertyDeclaration, 32);
 
-size_of_test!(test_size_of_application_declaration_block, ApplicableDeclarationBlock, 24);
-size_of_test!(test_size_of_rule_node, RuleNode, 80);
+size_of_test!(test_size_of_application_declaration_block, ApplicableDeclarationBlock, 16);
+size_of_test!(test_size_of_rule_node, RuleNode, 72);
 
 // This is huge, but we allocate it on the stack and then never move it,
 // we only pass `&mut SourcePropertyDeclaration` references around.