servo: Merge #19769 - layout: Simplify the pseudo-element setup (from emilio:simplify-layout-construct); r=nox

It's kind of a mess. See individual commits for details.

Source-Repo: https://github.com/servo/servo
Source-Revision: 032fd388e4efdd9ecc682877d017b9d2f78e29e2

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : f365989a4d7c713caf92ed0ef34a228a6243644e

servo/components/layout/construct.rs

@@ -109,7 +109,10 @@ pub enum ConstructionItem {
     /// Inline fragments and associated {ib} splits that have not yet found flows.
     InlineFragments(InlineFragmentsConstructionResult),
     /// Potentially ignorable whitespace.
-    Whitespace(OpaqueNode, PseudoElementType<()>, ServoArc<ComputedValues>, RestyleDamage),
+    ///
+    /// FIXME(emilio): How could whitespace have any PseudoElementType other
+    /// than Normal?
+    Whitespace(OpaqueNode, PseudoElementType, ServoArc<ComputedValues>, RestyleDamage),
     /// TableColumn Fragment
     TableColumnFragment(Fragment),
 }
@@ -257,7 +260,7 @@ impl InlineFragmentsAccumulator {
             fragments: IntermediateInlineFragments::new(),
             enclosing_node: Some(InlineFragmentNodeInfo {
                 address: node.opaque(),
-                pseudo: node.get_pseudo_element_type().strip(),
+                pseudo: node.get_pseudo_element_type(),
                 style: node.style(style_context),
                 selected_style: node.selected_style(),
                 flags: InlineFragmentNodeFlags::FIRST_FRAGMENT_OF_ELEMENT |
@@ -695,7 +698,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                 let specific_fragment_info = SpecificFragmentInfo::UnscannedText(info);
                 fragments.fragments.push_back(Fragment::from_opaque_node_and_style(
                         node.opaque(),
-                        node.get_pseudo_element_type().strip(),
+                        node.get_pseudo_element_type(),
                         style,
                         selected_style,
                         node.restyle_damage(),
@@ -715,7 +718,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                     };
                     fragments.fragments.push_back(Fragment::from_opaque_node_and_style(
                             node.opaque(),
-                            node.get_pseudo_element_type().strip(),
+                            node.get_pseudo_element_type(),
                             style.clone(),
                             selected_style.clone(),
                             node.restyle_damage(),
@@ -851,7 +854,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                 Box::new(UnscannedTextFragmentInfo::new(String::new(), None))
             );
             let fragment = Fragment::from_opaque_node_and_style(node.opaque(),
-                                                                node.get_pseudo_element_type().strip(),
+                                                                node.get_pseudo_element_type(),
                                                                 node_style.clone(),
                                                                 node.selected_style(),
                                                                 node.restyle_damage(),
@@ -898,7 +901,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
         if node.is_ignorable_whitespace(context) {
             return ConstructionResult::ConstructionItem(ConstructionItem::Whitespace(
                 node.opaque(),
-                node.get_pseudo_element_type().strip(),
+                node.get_pseudo_element_type(),
                 context.stylist.style_for_anonymous(
                     &context.guards, &PseudoElement::ServoText, &style),
                 node.restyle_damage()))
@@ -951,7 +954,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
         let fragment_info = SpecificFragmentInfo::InlineBlock(InlineBlockFragmentInfo::new(
                 block_flow));
         let fragment = Fragment::from_opaque_node_and_style(node.opaque(),
-                                                            node.get_pseudo_element_type().strip(),
+                                                            node.get_pseudo_element_type(),
                                                             style,
                                                             node.selected_style(),
                                                             node.restyle_damage(),
@@ -1392,7 +1395,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                         if fragment.node != node.opaque() {
                             continue
                         }
-                        if fragment.pseudo != node.get_pseudo_element_type().strip() {
+                        if fragment.pseudo != node.get_pseudo_element_type() {
                             continue
                         }
 
@@ -1462,8 +1465,10 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS
     fn process(&mut self, node: &ConcreteThreadSafeLayoutNode) {
         node.insert_flags(LayoutDataFlags::HAS_NEWLY_CONSTRUCTED_FLOW);
 
+        let style = node.style(self.style_context());
+
         // Bail out if this node has an ancestor with display: none.
-        if node.style(self.style_context()).is_in_display_none_subtree() {
+        if style.is_in_display_none_subtree() {
             self.set_flow_construction_result(node, ConstructionResult::None);
             return;
         }
@@ -1472,20 +1477,13 @@ impl<'a, ConcreteThreadSafeLayoutNode> PostorderNodeMutTraversal<ConcreteThreadS
         let (display, float, positioning) = match node.type_id() {
             None => {
                 // Pseudo-element.
-                let style = node.style(self.style_context());
-                let display = match node.get_pseudo_element_type() {
-                    PseudoElementType::Normal => Display::Inline,
-                    PseudoElementType::Before(maybe_display) |
-                    PseudoElementType::After(maybe_display) |
-                    PseudoElementType::DetailsContent(maybe_display) |
-                    PseudoElementType::DetailsSummary(maybe_display)
-                        => maybe_display.unwrap_or(style.get_box().display),
-                };
-                (display, style.get_box().float, style.get_box().position)
+                (style.get_box().display, style.get_box().float, style.get_box().position)
             }
             Some(LayoutNodeType::Element(_)) => {
-                let style = node.style(self.style_context());
                 let original_display = style.get_box().original_display;
+                // FIXME(emilio, #19771): This munged_display business is pretty
+                // wrong. After we fix this we should be able to unify the
+                // pseudo-element path too.
                 let munged_display = match original_display {
                     Display::Inline | Display::InlineBlock => original_display,
                     _ => style.get_box().display,
@@ -1649,10 +1647,10 @@ impl<ConcreteThreadSafeLayoutNode> NodeUtils for ConcreteThreadSafeLayoutNode
 
     fn construction_result_mut(self, data: &mut LayoutData) -> &mut ConstructionResult {
         match self.get_pseudo_element_type() {
-            PseudoElementType::Before(_) => &mut data.before_flow_construction_result,
-            PseudoElementType::After(_) => &mut data.after_flow_construction_result,
-            PseudoElementType::DetailsSummary(_) => &mut data.details_summary_flow_construction_result,
-            PseudoElementType::DetailsContent(_) => &mut data.details_content_flow_construction_result,
+            PseudoElementType::Before => &mut data.before_flow_construction_result,
+            PseudoElementType::After => &mut data.after_flow_construction_result,
+            PseudoElementType::DetailsSummary => &mut data.details_summary_flow_construction_result,
+            PseudoElementType::DetailsContent => &mut data.details_content_flow_construction_result,
             PseudoElementType::Normal    => &mut data.flow_construction_result,
         }
     }

servo/components/layout/display_list/builder.rs

@@ -45,7 +45,6 @@ use msg::constellation_msg::{BrowsingContextId, PipelineId};
 use net_traits::image::base::PixelFormat;
 use net_traits::image_cache::UsePlaceholder;
 use range::Range;
-use script_layout_interface::wrapper_traits::PseudoElementType;
 use servo_config::opts;
 use servo_geometry::max_rect;
 use std::{cmp, f32};
@@ -2276,13 +2275,7 @@ impl FragmentDisplayListBuilding for Fragment {
     }
 
     fn fragment_type(&self) -> FragmentType {
-        match self.pseudo {
-            PseudoElementType::Normal => FragmentType::FragmentBody,
-            PseudoElementType::Before(_) => FragmentType::BeforePseudoContent,
-            PseudoElementType::After(_) => FragmentType::AfterPseudoContent,
-            PseudoElementType::DetailsSummary(_) => FragmentType::FragmentBody,
-            PseudoElementType::DetailsContent(_) => FragmentType::FragmentBody,
-        }
+        self.pseudo.fragment_type()
     }
 }
 

servo/components/layout/fragment.rs

@@ -138,7 +138,7 @@ pub struct Fragment {
     pub restyle_damage: RestyleDamage,
 
     /// The pseudo-element that this fragment represents.
-    pub pseudo: PseudoElementType<()>,
+    pub pseudo: PseudoElementType,
 
     /// Various flags for this fragment.
     pub flags: FragmentFlags,
@@ -622,7 +622,7 @@ impl Fragment {
             margin: LogicalMargin::zero(writing_mode),
             specific: specific,
             inline_context: None,
-            pseudo: node.get_pseudo_element_type().strip(),
+            pseudo: node.get_pseudo_element_type(),
             flags: FragmentFlags::empty(),
             debug_id: DebugId::new(),
             stacking_context_id: StackingContextId::root(),
@@ -631,7 +631,7 @@ impl Fragment {
 
     /// Constructs a new `Fragment` instance from an opaque node.
     pub fn from_opaque_node_and_style(node: OpaqueNode,
-                                      pseudo: PseudoElementType<()>,
+                                      pseudo: PseudoElementType,
                                       style: ServoArc<ComputedValues>,
                                       selected_style: ServoArc<ComputedValues>,
                                       mut restyle_damage: RestyleDamage,

servo/components/layout/generated_content.rs

@@ -368,7 +368,7 @@ impl Counter {
     fn render(&self,
               layout_context: &LayoutContext,
               node: OpaqueNode,
-              pseudo: PseudoElementType<()>,
+              pseudo: PseudoElementType,
               style: ::ServoArc<ComputedValues>,
               list_style_type: ListStyleType,
               mode: RenderingMode)
@@ -431,7 +431,7 @@ struct CounterValue {
 /// Creates fragment info for a literal string.
 fn render_text(layout_context: &LayoutContext,
                node: OpaqueNode,
-               pseudo: PseudoElementType<()>,
+               pseudo: PseudoElementType,
                style: ::ServoArc<ComputedValues>,
                string: String)
                -> Option<SpecificFragmentInfo> {

servo/components/layout/inline.rs

@@ -1766,7 +1766,7 @@ pub struct InlineFragmentNodeInfo {
     pub address: OpaqueNode,
     pub style: ServoArc<ComputedValues>,
     pub selected_style: ServoArc<ComputedValues>,
-    pub pseudo: PseudoElementType<()>,
+    pub pseudo: PseudoElementType,
     pub flags: InlineFragmentNodeFlags,
 }
 

servo/components/layout_thread/dom_wrapper.rs

@@ -63,7 +63,6 @@ use std::sync::atomic::Ordering;
 use style::CaseSensitivityExt;
 use style::applicable_declarations::ApplicableDeclarationBlock;
 use style::attr::AttrValue;
-use style::computed_values::display;
 use style::context::SharedStyleContext;
 use style::data::ElementData;
 use style::dom::{DomChildren, LayoutIterator, NodeInfo, OpaqueNode};
@@ -804,7 +803,7 @@ pub struct ServoThreadSafeLayoutNode<'ln> {
 
     /// The pseudo-element type, with (optionally)
     /// a specified display value to override the stylesheet.
-    pseudo: PseudoElementType<Option<display::T>>,
+    pseudo: PseudoElementType,
 }
 
 impl<'a> PartialEq for ServoThreadSafeLayoutNode<'a> {
@@ -994,7 +993,7 @@ impl<ConcreteNode> ThreadSafeLayoutNodeChildrenIterator<ConcreteNode>
                     unsafe { parent.dangerous_first_child() }
                 })
             },
-            PseudoElementType::DetailsContent(_) | PseudoElementType::DetailsSummary(_) => {
+            PseudoElementType::DetailsContent | PseudoElementType::DetailsSummary => {
                 unsafe { parent.dangerous_first_child() }
             },
             _ => None,
@@ -1012,9 +1011,9 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
     fn next(&mut self) -> Option<ConcreteNode> {
         use ::selectors::Element;
         match self.parent_node.get_pseudo_element_type() {
-            PseudoElementType::Before(_) | PseudoElementType::After(_) => None,
+            PseudoElementType::Before | PseudoElementType::After => None,
 
-            PseudoElementType::DetailsSummary(_) => {
+            PseudoElementType::DetailsSummary => {
                 let mut current_node = self.current_node.clone();
                 loop {
                     let next_node = if let Some(ref node) = current_node {
@@ -1034,7 +1033,7 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
                 }
             }
 
-            PseudoElementType::DetailsContent(_) => {
+            PseudoElementType::DetailsContent => {
                 let node = self.current_node.clone();
                 let node = node.and_then(|node| {
                     if node.is_element() &&
@@ -1053,7 +1052,7 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
                 let node = self.current_node.clone();
                 if let Some(ref node) = node {
                     self.current_node = match node.get_pseudo_element_type() {
-                        PseudoElementType::Before(_) => {
+                        PseudoElementType::Before => {
                             self.parent_node.get_details_summary_pseudo()
                                 .or_else(|| unsafe { self.parent_node.dangerous_first_child() })
                                 .or_else(|| self.parent_node.get_after_pseudo())
@@ -1061,11 +1060,9 @@ impl<ConcreteNode> Iterator for ThreadSafeLayoutNodeChildrenIterator<ConcreteNod
                         PseudoElementType::Normal => {
                             unsafe { node.dangerous_next_sibling() }.or_else(|| self.parent_node.get_after_pseudo())
                         },
-                        PseudoElementType::DetailsSummary(_) => self.parent_node.get_details_content_pseudo(),
-                        PseudoElementType::DetailsContent(_) => self.parent_node.get_after_pseudo(),
-                        PseudoElementType::After(_) => {
-                            None
-                        },
+                        PseudoElementType::DetailsSummary => self.parent_node.get_details_content_pseudo(),
+                        PseudoElementType::DetailsContent => self.parent_node.get_after_pseudo(),
+                        PseudoElementType::After => None,
                     };
                 }
                 node
@@ -1083,7 +1080,7 @@ pub struct ServoThreadSafeLayoutElement<'le> {
 
     /// The pseudo-element type, with (optionally)
     /// a specified display value to override the stylesheet.
-    pseudo: PseudoElementType<Option<display::T>>,
+    pseudo: PseudoElementType,
 }
 
 impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
@@ -1096,15 +1093,14 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
         }
     }
 
-    fn get_pseudo_element_type(&self) -> PseudoElementType<Option<display::T>> {
+    fn get_pseudo_element_type(&self) -> PseudoElementType {
         self.pseudo
     }
 
-    fn with_pseudo(&self,
-                   pseudo: PseudoElementType<Option<display::T>>) -> Self {
+    fn with_pseudo(&self, pseudo: PseudoElementType) -> Self {
         ServoThreadSafeLayoutElement {
             element: self.element.clone(),
-            pseudo: pseudo,
+            pseudo,
         }
     }
 

servo/components/script_layout_interface/wrapper_traits.rs

@@ -17,7 +17,6 @@ use servo_arc::Arc;
 use servo_url::ServoUrl;
 use std::fmt::Debug;
 use style::attr::AttrValue;
-use style::computed_values::display::T as Display;
 use style::context::SharedStyleContext;
 use style::data::ElementData;
 use style::dom::{LayoutIterator, NodeInfo, TNode};
@@ -29,46 +28,46 @@ use style::stylist::RuleInclusion;
 use webrender_api::ClipId;
 
 #[derive(Clone, Copy, Debug, PartialEq)]
-pub enum PseudoElementType<T> {
+pub enum PseudoElementType {
     Normal,
-    Before(T),
-    After(T),
-    DetailsSummary(T),
-    DetailsContent(T),
+    Before,
+    After,
+    DetailsSummary,
+    DetailsContent,
 }
 
-impl<T> PseudoElementType<T> {
+impl PseudoElementType {
+    pub fn fragment_type(&self) -> FragmentType {
+        match *self {
+            PseudoElementType::Normal => FragmentType::FragmentBody,
+            PseudoElementType::Before => FragmentType::BeforePseudoContent,
+            PseudoElementType::After => FragmentType::AfterPseudoContent,
+            PseudoElementType::DetailsSummary => FragmentType::FragmentBody,
+            PseudoElementType::DetailsContent => FragmentType::FragmentBody,
+        }
+    }
+
     pub fn is_before(&self) -> bool {
         match *self {
-            PseudoElementType::Before(_) => true,
+            PseudoElementType::Before => true,
             _ => false,
         }
     }
 
     pub fn is_replaced_content(&self) -> bool {
         match *self {
-            PseudoElementType::Before(_) | PseudoElementType::After(_) => true,
+            PseudoElementType::Before | PseudoElementType::After => true,
             _ => false,
         }
     }
 
-    pub fn strip(&self) -> PseudoElementType<()> {
-        match *self {
-            PseudoElementType::Normal => PseudoElementType::Normal,
-            PseudoElementType::Before(_) => PseudoElementType::Before(()),
-            PseudoElementType::After(_) => PseudoElementType::After(()),
-            PseudoElementType::DetailsSummary(_) => PseudoElementType::DetailsSummary(()),
-            PseudoElementType::DetailsContent(_) => PseudoElementType::DetailsContent(()),
-        }
-    }
-
     pub fn style_pseudo_element(&self) -> PseudoElement {
         match *self {
             PseudoElementType::Normal => unreachable!("style_pseudo_element called with PseudoElementType::Normal"),
-            PseudoElementType::Before(_) => PseudoElement::Before,
-            PseudoElementType::After(_) => PseudoElement::After,
-            PseudoElementType::DetailsSummary(_) => PseudoElement::DetailsSummary,
-            PseudoElementType::DetailsContent(_) => PseudoElement::DetailsContent,
+            PseudoElementType::Before => PseudoElement::Before,
+            PseudoElementType::After => PseudoElement::After,
+            PseudoElementType::DetailsSummary => PseudoElement::DetailsSummary,
+            PseudoElementType::DetailsContent => PseudoElement::DetailsContent,
         }
     }
 }
@@ -197,7 +196,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
     fn as_element(&self) -> Option<Self::ConcreteThreadSafeLayoutElement>;
 
     #[inline]
-    fn get_pseudo_element_type(&self) -> PseudoElementType<Option<Display>> {
+    fn get_pseudo_element_type(&self) -> PseudoElementType {
         self.as_element().map_or(PseudoElementType::Normal, |el| el.get_pseudo_element_type())
     }
 
@@ -265,13 +264,7 @@ pub trait ThreadSafeLayoutNode: Clone + Copy + Debug + GetLayoutData + NodeInfo
     fn get_rowspan(&self) -> u32;
 
     fn fragment_type(&self) -> FragmentType {
-        match self.get_pseudo_element_type() {
-            PseudoElementType::Normal => FragmentType::FragmentBody,
-            PseudoElementType::Before(_) => FragmentType::BeforePseudoContent,
-            PseudoElementType::After(_) => FragmentType::AfterPseudoContent,
-            PseudoElementType::DetailsSummary(_) => FragmentType::FragmentBody,
-            PseudoElementType::DetailsContent(_) => FragmentType::FragmentBody,
-        }
+        self.get_pseudo_element_type().fragment_type()
     }
 
     fn generate_scroll_root_id(&self, pipeline_id: PipelineId) -> ClipId {
@@ -302,7 +295,7 @@ pub trait ThreadSafeLayoutElement
 
     /// Creates a new `ThreadSafeLayoutElement` for the same `LayoutElement`
     /// with a different pseudo-element type.
-    fn with_pseudo(&self, pseudo: PseudoElementType<Option<Display>>) -> Self;
+    fn with_pseudo(&self, pseudo: PseudoElementType) -> Self;
 
     /// Returns the type ID of this node.
     /// Returns `None` if this is a pseudo-element; otherwise, returns `Some`.
@@ -325,12 +318,12 @@ pub trait ThreadSafeLayoutElement
     fn style_data(&self) -> AtomicRef<ElementData>;
 
     #[inline]
-    fn get_pseudo_element_type(&self) -> PseudoElementType<Option<Display>>;
+    fn get_pseudo_element_type(&self) -> PseudoElementType;
 
     #[inline]
     fn get_before_pseudo(&self) -> Option<Self> {
         if self.style_data().styles.pseudos.get(&PseudoElement::Before).is_some() {
-            Some(self.with_pseudo(PseudoElementType::Before(None)))
+            Some(self.with_pseudo(PseudoElementType::Before))
         } else {
             None
         }
@@ -339,7 +332,7 @@ pub trait ThreadSafeLayoutElement
     #[inline]
     fn get_after_pseudo(&self) -> Option<Self> {
         if self.style_data().styles.pseudos.get(&PseudoElement::After).is_some() {
-            Some(self.with_pseudo(PseudoElementType::After(None)))
+            Some(self.with_pseudo(PseudoElementType::After))
         } else {
             None
         }
@@ -349,7 +342,7 @@ pub trait ThreadSafeLayoutElement
     fn get_details_summary_pseudo(&self) -> Option<Self> {
         if self.get_local_name() == &local_name!("details") &&
            self.get_namespace() == &ns!(html) {
-            Some(self.with_pseudo(PseudoElementType::DetailsSummary(None)))
+            Some(self.with_pseudo(PseudoElementType::DetailsSummary))
         } else {
             None
         }
@@ -358,13 +351,9 @@ pub trait ThreadSafeLayoutElement
     #[inline]
     fn get_details_content_pseudo(&self) -> Option<Self> {
         if self.get_local_name() == &local_name!("details") &&
-           self.get_namespace() == &ns!(html) {
-            let display = if self.get_attr(&ns!(), &local_name!("open")).is_some() {
-                None // Specified by the stylesheet
-            } else {
-                Some(Display::None)
-            };
-            Some(self.with_pseudo(PseudoElementType::DetailsContent(display)))
+           self.get_namespace() == &ns!(html) &&
+           self.get_attr(&ns!(), &local_name!("open")).is_some() {
+            Some(self.with_pseudo(PseudoElementType::DetailsContent))
         } else {
             None
         }