style: Use precomputation for the -servo-details-content pseudo-element

components/layout/wrapper.rs

@@ -707,10 +707,6 @@ pub trait ThreadSafeLayoutNode : Clone + Copy + Sized + PartialEq {
             })
     }
 
-    // TODO(emilio): Since the ::-details-* pseudos are internal, just affecting
-    // one element, and only changing `display` property when the element `open`
-    // attribute changes, this should be eligible for not being cascaded
-    // eagerly, reading the display property from layout instead.
     #[inline]
     fn get_details_summary_pseudo(&self) -> Option<Self> {
         if self.is_element() &&
@@ -729,18 +725,29 @@ pub trait ThreadSafeLayoutNode : Clone + Copy + Sized + PartialEq {
 
     #[inline]
     fn get_details_content_pseudo(&self) -> Option<Self> {
-        if self.is_element() &&
+        if !self.is_element() {
-           self.as_element().get_local_name() == &atom!("details") &&
+            return None;
-           self.as_element().get_namespace() == &ns!(html) {
-            self.borrow_layout_data().unwrap()
-                .style_data.per_pseudo
-                .get(&PseudoElement::DetailsContent)
-                .map(|style| {
-                    self.with_pseudo(PseudoElementType::DetailsContent(style.get_box().display))
-                })
-        } else {
-            None
         }
+
+        let element = self.as_element();
+        if element.get_local_name() != &atom!("details") ||
+           element.get_namespace() != &ns!(html) {
+            return None;
+        }
+
+        self.borrow_layout_data().unwrap()
+            .style_data
+            .precomputed
+            .non_eagerly_cascaded_pseudo_elements
+            .get(&PseudoElement::DetailsContent)
+            .map(|style| {
+                let display = if element.get_attr(&ns!(), &atom!("open")).is_some() {
+                    style.get_box().display
+                } else {
+                    display::T::none
+                };
+                self.with_pseudo(PseudoElementType::DetailsContent(display))
+            })
     }
 
     /// Borrows the layout data immutably. Fails on a conflicting borrow.
@@ -763,11 +770,19 @@ pub trait ThreadSafeLayoutNode : Clone + Copy + Sized + PartialEq {
     fn style(&self) -> Ref<Arc<ServoComputedValues>> {
         Ref::map(self.borrow_layout_data().unwrap(), |data| {
             let style = match self.get_pseudo_element_type() {
-                PseudoElementType::Before(_) => data.style_data.per_pseudo.get(&PseudoElement::Before),
+                PseudoElementType::Before(_)
-                PseudoElementType::After(_) => data.style_data.per_pseudo.get(&PseudoElement::After),
+                    => data.style_data.per_pseudo.get(&PseudoElement::Before),
-                PseudoElementType::DetailsSummary(_) => data.style_data.per_pseudo.get(&PseudoElement::DetailsSummary),
+                PseudoElementType::After(_)
-                PseudoElementType::DetailsContent(_) => data.style_data.per_pseudo.get(&PseudoElement::DetailsContent),
+                    => data.style_data.per_pseudo.get(&PseudoElement::After),
-                PseudoElementType::Normal => data.style_data.style.as_ref(),
+                PseudoElementType::DetailsSummary(_)
+                    => data.style_data.per_pseudo.get(&PseudoElement::DetailsSummary),
+                PseudoElementType::DetailsContent(_)
+                    => data.style_data
+                           .precomputed
+                           .non_eagerly_cascaded_pseudo_elements
+                           .get(&PseudoElement::DetailsContent),
+                PseudoElementType::Normal
+                    => data.style_data.style.as_ref(),
             };
             style.unwrap()
         })
@@ -1165,8 +1180,8 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
                 loop {
                     let next_node = if let Some(ref node) = current_node {
                         if node.is_element() &&
-                                node.as_element().get_local_name() == &atom!("summary") &&
+                           node.as_element().get_local_name() == &atom!("summary") &&
-                                node.as_element().get_namespace() == &ns!(html) {
+                           node.as_element().get_namespace() == &ns!(html) {
                             self.current_node = None;
                             return Some(node.clone());
                         }

components/style/selector_impl.rs

@@ -159,15 +159,14 @@ impl SelectorImpl for ServoSelectorImpl {
 impl SelectorImplExt for ServoSelectorImpl {
     type ComputedValues = ServoComputedValues;
 
-    // TODO: Making details-summary not eagerly cascaded shouldn't be difficult
     #[inline]
     fn is_eagerly_cascaded_pseudo_element(pseudo: &PseudoElement) -> bool {
         match *pseudo {
             PseudoElement::Before |
             PseudoElement::After |
             PseudoElement::Selection |
-            PseudoElement::DetailsContent |
             PseudoElement::DetailsSummary => true,
+            PseudoElement::DetailsContent => false,
         }
     }
 

components/style/selector_matching.rs

@@ -90,7 +90,7 @@ pub struct PrecomputedStyleData<Impl: SelectorImpl, Computed: ComputedValues> {
     /// are eagerly computed once, and then just looked up in the table,
     /// since they only appear in rules of the form *|*::pseudo-element
     pub non_eagerly_cascaded_pseudo_elements: HashMap<Impl::PseudoElement,
-                                                      Computed,
+                                                      Arc<Computed>,
                                                       BuildHasherDefault<::fnv::FnvHasher>>,
 }
 
@@ -277,7 +277,7 @@ impl<Impl: SelectorImplExt> Stylist<Impl> {
                                                                 &declarations, false,
                                                                 None, None,
                                                                 box StdoutErrorReporter);
-                precomputed.non_eagerly_cascaded_pseudo_elements.insert(pseudo, computed);
+                precomputed.non_eagerly_cascaded_pseudo_elements.insert(pseudo, Arc::new(computed));
             }
         })
     }
@@ -287,7 +287,7 @@ impl<Impl: SelectorImplExt> Stylist<Impl> {
     }
 
     pub fn get_non_eagerly_cascaded_pseudo_element_style(&self,
-                                                         pseudo: &Impl::PseudoElement) -> Option<Impl::ComputedValues> {
+                                                         pseudo: &Impl::PseudoElement) -> Option<Arc<Impl::ComputedValues>> {
         debug_assert!(!Impl::is_eagerly_cascaded_pseudo_element(pseudo));
         self.precomputed
             .non_eagerly_cascaded_pseudo_elements

resources/servo.css

@@ -51,15 +51,11 @@ details::-servo-details-summary {
 details[open]::-servo-details-summary {
   list-style: disclosure-open;
 }
-details::-servo-details-content {
+*|*::-servo-details-content {
   margin-left: 40px;
   overflow: hidden;
-  display: none;
-}
-details[open]::-servo-details-content {
   display: block;
 }
-
 /*
  * Until servo supports svg properly, make sure to at least prevent svg
  * children from being layed out and rendered like usual html.