Stop using UnsafeNode in the StyleSharingCandidateCache.

components/layout/context.rs

@@ -25,13 +25,14 @@ use std::hash::BuildHasherDefault;
 use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use style::context::{SharedStyleContext, ThreadLocalStyleContext};
+use style::dom::TElement;
 
 /// TLS data scoped to the traversal.
-pub struct ScopedThreadLocalLayoutContext {
-    pub style_context: ThreadLocalStyleContext,
+pub struct ScopedThreadLocalLayoutContext<E: TElement> {
+    pub style_context: ThreadLocalStyleContext<E>,
 }
 
-impl ScopedThreadLocalLayoutContext {
+impl<E: TElement> ScopedThreadLocalLayoutContext<E> {
     pub fn new(shared: &SharedLayoutContext) -> Self {
         ScopedThreadLocalLayoutContext {
             style_context: ThreadLocalStyleContext::new(&shared.style_context),

components/layout/traversal.rs

@@ -54,7 +54,7 @@ impl<N> DomTraversal<N> for RecalcStyleAndConstructFlows
     where N: LayoutNode + TNode,
           N::ConcreteElement: TElement
 {
-    type ThreadLocalContext = ScopedThreadLocalLayoutContext;
+    type ThreadLocalContext = ScopedThreadLocalLayoutContext<N::ConcreteElement>;
 
     fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData,
                         thread_local: &mut Self::ThreadLocalContext, node: N) {
@@ -115,7 +115,7 @@ pub trait PostorderNodeMutTraversal<ConcreteThreadSafeLayoutNode: ThreadSafeLayo
 #[inline]
 #[allow(unsafe_code)]
 fn construct_flows_at<'a, N>(context: &LayoutContext<'a>,
-                             _thread_local: &ScopedThreadLocalLayoutContext,
+                             _thread_local: &ScopedThreadLocalLayoutContext<N::ConcreteElement>,
                              root: OpaqueNode, node: N)
     where N: LayoutNode,
 {

components/style/context.rs

@@ -6,12 +6,11 @@
 
 use animation::Animation;
 use app_units::Au;
-use dom::OpaqueNode;
+use dom::{OpaqueNode, TElement};
 use error_reporting::ParseErrorReporter;
 use euclid::Size2D;
 use matching::StyleSharingCandidateCache;
 use parking_lot::RwLock;
-use std::cell::RefCell;
 use std::collections::HashMap;
 use std::sync::{Arc, Mutex};
 use std::sync::mpsc::Sender;
@@ -76,25 +75,25 @@ pub struct SharedStyleContext {
     pub quirks_mode: QuirksMode,
 }
 
-pub struct ThreadLocalStyleContext {
-    pub style_sharing_candidate_cache: RefCell<StyleSharingCandidateCache>,
+pub struct ThreadLocalStyleContext<E: TElement> {
+    pub style_sharing_candidate_cache: StyleSharingCandidateCache<E>,
     /// A channel on which new animations that have been triggered by style
     /// recalculation can be sent.
     pub new_animations_sender: Sender<Animation>,
 }
 
-impl ThreadLocalStyleContext {
+impl<E: TElement> ThreadLocalStyleContext<E> {
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
-            style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()),
+            style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
             new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(),
         }
     }
 }
 
-pub struct StyleContext<'a> {
+pub struct StyleContext<'a, E: TElement + 'a> {
     pub shared: &'a SharedStyleContext,
-    pub thread_local: &'a ThreadLocalStyleContext,
+    pub thread_local: &'a mut ThreadLocalStyleContext<E>,
 }
 
 /// Why we're doing reflow.

components/style/gecko/traversal.rs

@@ -22,10 +22,10 @@ impl RecalcStyleOnly {
 }
 
 impl<'ln> DomTraversal<GeckoNode<'ln>> for RecalcStyleOnly {
-    type ThreadLocalContext = ThreadLocalStyleContext;
+    type ThreadLocalContext = ThreadLocalStyleContext<GeckoElement<'ln>>;
 
     fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData,
-                        thread_local: &mut ThreadLocalStyleContext,
+                        thread_local: &mut Self::ThreadLocalContext,
                         node: GeckoNode<'ln>)
     {
         if node.is_element() {
@@ -39,7 +39,7 @@ impl<'ln> DomTraversal<GeckoNode<'ln>> for RecalcStyleOnly {
         }
     }
 
-    fn process_postorder(&self, _: &mut ThreadLocalStyleContext, _: GeckoNode<'ln>) {
+    fn process_postorder(&self, _: &mut Self::ThreadLocalContext, _: GeckoNode<'ln>) {
         unreachable!();
     }
 
@@ -58,7 +58,7 @@ impl<'ln> DomTraversal<GeckoNode<'ln>> for RecalcStyleOnly {
         &self.shared
     }
 
-    fn create_thread_local_context(&self) -> ThreadLocalStyleContext {
+    fn create_thread_local_context(&self) -> Self::ThreadLocalContext {
         ThreadLocalStyleContext::new(&self.shared)
     }
 }

components/style/matching.rs

@@ -13,7 +13,7 @@ use cache::LRUCache;
 use cascade_info::CascadeInfo;
 use context::{SharedStyleContext, StyleContext};
 use data::{ComputedStyle, ElementData, ElementStyles, PseudoStyles};
-use dom::{TElement, TNode, UnsafeNode};
+use dom::{TElement, TNode};
 use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
 use properties::longhands::display::computed_value as display;
 use rule_tree::StrongRuleNode;
@@ -65,22 +65,29 @@ impl MatchResults {
     }
 }
 
+// TElement isn't Send because we want to be careful and explicit about our
+// parallel traversal, but we need the candidates to be Send so that we can stick
+// them in ScopedTLS.
+#[derive(Debug, PartialEq)]
+struct SendElement<E: TElement>(pub E);
+unsafe impl<E: TElement> Send for SendElement<E> {}
+
 /// Information regarding a candidate.
 ///
 /// TODO: We can stick a lot more info here.
 #[derive(Debug)]
-struct StyleSharingCandidate {
-    /// The node, guaranteed to be an element.
-    node: UnsafeNode,
+struct StyleSharingCandidate<E: TElement> {
+    /// The element.
+    element: SendElement<E>,
     /// The cached common style affecting attribute info.
     common_style_affecting_attributes: Option<CommonStyleAffectingAttributes>,
     /// the cached class names.
     class_attributes: Option<Vec<Atom>>,
 }
 
-impl PartialEq<StyleSharingCandidate> for StyleSharingCandidate {
+impl<E: TElement> PartialEq<StyleSharingCandidate<E>> for StyleSharingCandidate<E> {
     fn eq(&self, other: &Self) -> bool {
-        self.node == other.node &&
+        self.element == other.element &&
             self.common_style_affecting_attributes == other.common_style_affecting_attributes
     }
 }
@@ -90,13 +97,8 @@ impl PartialEq<StyleSharingCandidate> for StyleSharingCandidate {
 ///
 /// Note that this cache is flushed every time we steal work from the queue, so
 /// storing nodes here temporarily is safe.
-///
-/// NB: We store UnsafeNode's, but this is not unsafe. It's a shame being
-/// generic over elements is unfeasible (you can make compile style without much
-/// difficulty, but good luck with layout and all the types with assoc.
-/// lifetimes).
-pub struct StyleSharingCandidateCache {
-    cache: LRUCache<StyleSharingCandidate, ()>,
+pub struct StyleSharingCandidateCache<E: TElement> {
+    cache: LRUCache<StyleSharingCandidate<E>, ()>,
 }
 
 #[derive(Clone, Debug)]
@@ -117,7 +119,7 @@ pub enum CacheMiss {
 }
 
 fn element_matches_candidate<E: TElement>(element: &E,
-                                          candidate: &mut StyleSharingCandidate,
+                                          candidate: &mut StyleSharingCandidate<E>,
                                           candidate_element: &E,
                                           shared_context: &SharedStyleContext)
                                           -> Result<ComputedStyle, CacheMiss> {
@@ -193,7 +195,7 @@ fn element_matches_candidate<E: TElement>(element: &E,
 }
 
 fn have_same_common_style_affecting_attributes<E: TElement>(element: &E,
-                                                            candidate: &mut StyleSharingCandidate,
+                                                            candidate: &mut StyleSharingCandidate<E>,
                                                             candidate_element: &E) -> bool {
     if candidate.common_style_affecting_attributes.is_none() {
         candidate.common_style_affecting_attributes =
@@ -272,7 +274,7 @@ pub fn rare_style_affecting_attributes() -> [LocalName; 3] {
 }
 
 fn have_same_class<E: TElement>(element: &E,
-                                candidate: &mut StyleSharingCandidate,
+                                candidate: &mut StyleSharingCandidate<E>,
                                 candidate_element: &E) -> bool {
     // XXX Efficiency here, I'm only validating ideas.
     let mut element_class_attributes = vec![];
@@ -304,21 +306,21 @@ fn match_same_sibling_affecting_rules<E: TElement>(element: &E,
 
 static STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 8;
 
-impl StyleSharingCandidateCache {
+impl<E: TElement> StyleSharingCandidateCache<E> {
     pub fn new() -> Self {
         StyleSharingCandidateCache {
             cache: LRUCache::new(STYLE_SHARING_CANDIDATE_CACHE_SIZE),
         }
     }
 
-    fn iter_mut(&mut self) -> IterMut<(StyleSharingCandidate, ())> {
+    fn iter_mut(&mut self) -> IterMut<(StyleSharingCandidate<E>, ())> {
         self.cache.iter_mut()
     }
 
-    pub fn insert_if_possible<E: TElement>(&mut self,
-                                           element: &E,
-                                           style: &Arc<ComputedValues>,
-                                           relations: StyleRelations) {
+    pub fn insert_if_possible(&mut self,
+                              element: &E,
+                              style: &Arc<ComputedValues>,
+                              relations: StyleRelations) {
         use traversal::relations_are_shareable;
 
         let parent = match element.parent_element() {
@@ -348,10 +350,10 @@ impl StyleSharingCandidateCache {
         }
 
         debug!("Inserting into cache: {:?} with parent {:?}",
-               element.as_node().to_unsafe(), parent.as_node().to_unsafe());
+               element, parent);
 
         self.cache.insert(StyleSharingCandidate {
-            node: element.as_node().to_unsafe(),
+            element: SendElement(*element),
             common_style_affecting_attributes: None,
             class_attributes: None,
         }, ());
@@ -392,7 +394,7 @@ trait PrivateMatchMethods: TElement {
     /// Note that animations only apply to nodes or ::before or ::after
     /// pseudo-elements.
     fn cascade_node_pseudo_element<'a>(&self,
-                                       context: &StyleContext,
+                                       context: &StyleContext<Self>,
                                        parent_style: Option<&Arc<ComputedValues>>,
                                        old_style: Option<&Arc<ComputedValues>>,
                                        rule_node: &StrongRuleNode,
@@ -497,20 +499,17 @@ trait PrivateMatchMethods: TElement {
 
     fn share_style_with_candidate_if_possible(&self,
                                               shared_context: &SharedStyleContext,
-                                              candidate: &mut StyleSharingCandidate)
+                                              candidate: &mut StyleSharingCandidate<Self>)
                                               -> Result<ComputedStyle, CacheMiss> {
-        let candidate_element = unsafe {
-            Self::ConcreteNode::from_unsafe(&candidate.node).as_element().unwrap()
-        };
-
+        let candidate_element = candidate.element.0;
         element_matches_candidate(self, candidate, &candidate_element,
                                   shared_context)
     }
 }
 
-fn compute_rule_node(context: &StyleContext,
-                     applicable_declarations: &mut Vec<ApplicableDeclarationBlock>)
-                     -> StrongRuleNode
+fn compute_rule_node<E: TElement>(context: &StyleContext<E>,
+                                  applicable_declarations: &mut Vec<ApplicableDeclarationBlock>)
+                                  -> StrongRuleNode
 {
     let rules = applicable_declarations.drain(..).map(|d| (d.source, d.importance));
     let rule_node = context.shared.stylist.rule_tree.insert_ordered_rules(rules);
@@ -520,7 +519,7 @@ fn compute_rule_node(context: &StyleContext,
 impl<E: TElement> PrivateMatchMethods for E {}
 
 pub trait MatchMethods : TElement {
-    fn match_element(&self, context: &StyleContext, parent_bf: Option<&BloomFilter>)
+    fn match_element(&self, context: &StyleContext<Self>, parent_bf: Option<&BloomFilter>)
                      -> MatchResults
     {
         let mut applicable_declarations: Vec<ApplicableDeclarationBlock> = Vec::with_capacity(16);
@@ -569,7 +568,7 @@ pub trait MatchMethods : TElement {
     /// guarantee that at the type system level yet.
     unsafe fn share_style_if_possible(&self,
                                       style_sharing_candidate_cache:
-                                        &mut StyleSharingCandidateCache,
+                                        &mut StyleSharingCandidateCache<Self>,
                                       shared_context: &SharedStyleContext,
                                       data: &mut AtomicRefMut<ElementData>)
                                       -> StyleSharingResult {
@@ -718,7 +717,7 @@ pub trait MatchMethods : TElement {
     }
 
     unsafe fn cascade_node(&self,
-                           context: &StyleContext,
+                           context: &StyleContext<Self>,
                            mut data: &mut AtomicRefMut<ElementData>,
                            parent: Option<Self>,
                            primary_rule_node: StrongRuleNode,
@@ -795,7 +794,7 @@ pub trait MatchMethods : TElement {
             mut old_pseudos: Option<&mut PseudoStyles>,
             new_primary: &Arc<ComputedValues>,
             new_pseudos: &mut PseudoStyles,
-            context: &StyleContext,
+            context: &StyleContext<Self>,
             mut pseudo_rule_nodes: PseudoRuleNodes,
             possibly_expired_animations: &mut Vec<PropertyAnimation>)
             -> RestyleDamage

components/style/traversal.rs

@@ -336,7 +336,7 @@ pub fn relations_are_shareable(relations: &StyleRelations) -> bool {
 
 /// Handles lazy resolution of style in display:none subtrees. See the comment
 /// at the callsite in query.rs.
-pub fn style_element_in_display_none_subtree<E, F>(context: &StyleContext,
+pub fn style_element_in_display_none_subtree<E, F>(context: &StyleContext<E>,
                                                    element: E, init_data: &F) -> E
     where E: TElement,
           F: Fn(E),
@@ -375,7 +375,7 @@ pub fn style_element_in_display_none_subtree<E, F>(context: &StyleContext,
 #[allow(unsafe_code)]
 pub fn recalc_style_at<E, D>(traversal: &D,
                              traversal_data: &mut PerLevelTraversalData,
-                             context: &StyleContext,
+                             context: &mut StyleContext<E>,
                              element: E,
                              mut data: &mut AtomicRefMut<ElementData>)
     where E: TElement,
@@ -422,7 +422,7 @@ pub fn recalc_style_at<E, D>(traversal: &D,
 // since we have separate bits for each now.
 fn compute_style<E, D>(_traversal: &D,
                        traversal_data: &mut PerLevelTraversalData,
-                       context: &StyleContext,
+                       context: &mut StyleContext<E>,
                        element: E,
                        mut data: &mut AtomicRefMut<ElementData>) -> bool
     where E: TElement,
@@ -444,13 +444,10 @@ fn compute_style<E, D>(_traversal: &D,
     bf.assert_complete(element);
 
     // Check to see whether we can share a style with someone.
-    let mut style_sharing_candidate_cache =
-        context.thread_local.style_sharing_candidate_cache.borrow_mut();
-
     let sharing_result = if element.parent_element().is_none() {
         StyleSharingResult::CannotShare
     } else {
-        unsafe { element.share_style_if_possible(&mut style_sharing_candidate_cache,
+        unsafe { element.share_style_if_possible(&mut context.thread_local.style_sharing_candidate_cache,
                                                  shared_context, &mut data) }
     };
 
@@ -485,16 +482,16 @@ fn compute_style<E, D>(_traversal: &D,
 
             // Add ourselves to the LRU cache.
             if let Some(element) = shareable_element {
-                style_sharing_candidate_cache.insert_if_possible(&element,
-                                                                 &data.styles().primary.values,
-                                                                 relations);
+                context.thread_local
+                       .style_sharing_candidate_cache
+                       .insert_if_possible(&element, &data.styles().primary.values, relations);
             }
         }
         StyleSharingResult::StyleWasShared(index) => {
             if opts::get().style_sharing_stats {
                 STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed);
             }
-            style_sharing_candidate_cache.touch(index);
+            context.thread_local.style_sharing_candidate_cache.touch(index);
         }
     }
 

ports/geckolib/glue.rs

@@ -875,12 +875,12 @@ pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
             };
 
             let mut tlc = ThreadLocalStyleContext::new(traversal.shared_context());
-            let context = StyleContext {
+            let mut context = StyleContext {
                 shared: traversal.shared_context(),
                 thread_local: &mut tlc,
             };
 
-            recalc_style_at(&traversal, &mut traversal_data, &context, element, &mut data);
+            recalc_style_at(&traversal, &mut traversal_data, &mut context, element, &mut data);
 
             // The element was either unstyled or needed restyle. If it was unstyled, it may have
             // additional unstyled children that subsequent traversals won't find now that the style