script: Eliminate PseudoElementType (#36146)

Servo has a `PseudoElementType` which more or less duplicate's Stylo's
`PseudoElement` with the addition of a non-pseudo element variant. This
type needs to be converted into `PseudoElement` anyway when asking for
the style of an element from Stylo, so eliminate Servo's version and
simply use `Option<PseudoElement>` with the `None` variant meaning the
non-pseudo.

This is preparation for adding support for the `::marker` pseudo
element.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/layout_2020/dom_traversal.rs

@@ -407,8 +407,8 @@ where
     Node: NodeExt<'dom>,
 {
     match which {
-        WhichPseudoElement::Before => element.to_threadsafe().get_before_pseudo(),
+        WhichPseudoElement::After => element.to_threadsafe().get_pseudo(PseudoElement::After),
-        WhichPseudoElement::After => element.to_threadsafe().get_after_pseudo(),
+        WhichPseudoElement::Before => element.to_threadsafe().get_pseudo(PseudoElement::Before),
     }
     .and_then(|pseudo_element| {
         let style = pseudo_element.style(context.shared_context());

components/layout_2020/fragment_tree/base_fragment.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use bitflags::bitflags;
-use script_layout_interface::{FragmentType, combine_id_with_fragment_type};
+use script_layout_interface::combine_id_with_fragment_type;
 use style::dom::OpaqueNode;
 use style::selector_parser::PseudoElement;
 
@@ -132,11 +132,6 @@ impl Tag {
     }
 
     pub(crate) fn to_display_list_fragment_id(self) -> u64 {
-        let fragment_type = match self.pseudo {
+        combine_id_with_fragment_type(self.node.id(), self.pseudo.into())
-            Some(PseudoElement::Before) => FragmentType::BeforePseudoContent,
-            Some(PseudoElement::After) => FragmentType::AfterPseudoContent,
-            _ => FragmentType::FragmentBody,
-        };
-        combine_id_with_fragment_type(self.node.id(), fragment_type)
     }
 }

components/layout_2020/query.rs

@@ -9,7 +9,6 @@ use app_units::Au;
 use euclid::default::{Point2D, Rect};
 use euclid::{SideOffsets2D, Size2D, Vector2D};
 use itertools::Itertools;
-use log::warn;
 use script_layout_interface::wrapper_traits::{
     LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode,
 };
@@ -117,15 +116,10 @@ pub fn process_resolved_style_request<'dom>(
     // We call process_resolved_style_request after performing a whole-document
     // traversal, so in the common case, the element is styled.
     let layout_element = node.to_threadsafe().as_element().unwrap();
-    let layout_element = match *pseudo {
+    let layout_element = pseudo.map_or_else(
-        None => Some(layout_element),
+        || Some(layout_element),
-        Some(PseudoElement::Before) => layout_element.get_before_pseudo(),
+        |pseudo_element| layout_element.get_pseudo(pseudo_element),
-        Some(PseudoElement::After) => layout_element.get_after_pseudo(),
+    );
-        Some(_) => {
-            warn!("Got unexpected pseudo element type!");
-            None
-        },
-    };
 
     let layout_element = match layout_element {
         None => {

components/script/layout_dom/element.rs

@@ -11,7 +11,7 @@ use constellation_traits::UntrustedNodeAddress;
 use html5ever::{LocalName, Namespace, local_name, namespace_url, ns};
 use js::jsapi::JSObject;
 use script_layout_interface::wrapper_traits::{
-    LayoutNode, PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode,
+    LayoutNode, ThreadSafeLayoutElement, ThreadSafeLayoutNode,
 };
 use script_layout_interface::{LayoutNodeType, StyleData};
 use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
@@ -785,9 +785,9 @@ impl<'dom> ::selectors::Element for ServoLayoutElement<'dom> {
 pub struct ServoThreadSafeLayoutElement<'dom> {
     pub(super) element: ServoLayoutElement<'dom>,
 
-    /// The pseudo-element type, with (optionally)
+    /// The pseudo-element type for this element, or `None` if it is the non-pseudo
-    /// a specified display value to override the stylesheet.
+    /// version of the element.
-    pub(super) pseudo: PseudoElementType,
+    pub(super) pseudo: Option<PseudoElement>,
 }
 
 impl<'dom> ThreadSafeLayoutElement<'dom> for ServoThreadSafeLayoutElement<'dom> {
@@ -801,14 +801,14 @@ impl<'dom> ThreadSafeLayoutElement<'dom> for ServoThreadSafeLayoutElement<'dom>
         }
     }
 
-    fn get_pseudo_element_type(&self) -> PseudoElementType {
+    fn pseudo_element(&self) -> Option<PseudoElement> {
         self.pseudo
     }
 
-    fn with_pseudo(&self, pseudo: PseudoElementType) -> Self {
+    fn with_pseudo(&self, pseudo: PseudoElement) -> Self {
         ServoThreadSafeLayoutElement {
             element: self.element,
-            pseudo,
+            pseudo: Some(pseudo),
         }
     }
 

components/script/layout_dom/node.rs

@@ -13,9 +13,7 @@ use fonts_traits::ByteIndex;
 use html5ever::{local_name, namespace_url, ns};
 use pixels::{Image, ImageMetadata};
 use range::Range;
-use script_layout_interface::wrapper_traits::{
+use script_layout_interface::wrapper_traits::{LayoutDataTrait, LayoutNode, ThreadSafeLayoutNode};
-    LayoutDataTrait, LayoutNode, PseudoElementType, ThreadSafeLayoutNode,
-};
 use script_layout_interface::{
     GenericLayoutData, HTMLCanvasData, HTMLMediaData, LayoutNodeType, SVGSVGData, StyleData,
     TrustedNodeAddress,
@@ -25,6 +23,7 @@ use servo_url::ServoUrl;
 use style;
 use style::dom::{NodeInfo, TElement, TNode, TShadowRoot};
 use style::properties::ComputedValues;
+use style::selector_parser::PseudoElement;
 
 use super::{
     ServoLayoutDocument, ServoLayoutElement, ServoShadowRoot, ServoThreadSafeLayoutElement,
@@ -230,18 +229,15 @@ pub struct ServoThreadSafeLayoutNode<'dom> {
     /// The wrapped `ServoLayoutNode`.
     pub(super) node: ServoLayoutNode<'dom>,
 
-    /// The pseudo-element type, with (optionally)
+    /// The pseudo-element type for this node, or `None` if it is the non-pseudo
-    /// a specified display value to override the stylesheet.
+    /// version of the element.
-    pub(super) pseudo: PseudoElementType,
+    pub(super) pseudo: Option<PseudoElement>,
 }
 
 impl<'dom> ServoThreadSafeLayoutNode<'dom> {
     /// Creates a new `ServoThreadSafeLayoutNode` from the given `ServoLayoutNode`.
     pub fn new(node: ServoLayoutNode<'dom>) -> Self {
-        ServoThreadSafeLayoutNode {
+        ServoThreadSafeLayoutNode { node, pseudo: None }
-            node,
-            pseudo: PseudoElementType::Normal,
-        }
     }
 
     /// Returns the interior of this node as a `LayoutDom`. This is highly unsafe for layout to
@@ -289,8 +285,12 @@ impl<'dom> ThreadSafeLayoutNode<'dom> for ServoThreadSafeLayoutNode<'dom> {
         unsafe { self.get_jsmanaged().opaque() }
     }
 
+    fn pseudo_element(&self) -> Option<PseudoElement> {
+        self.pseudo
+    }
+
     fn type_id(&self) -> Option<LayoutNodeType> {
-        if self.pseudo == PseudoElementType::Normal {
+        if self.pseudo.is_none() {
             Some(self.node.type_id())
         } else {
             None
@@ -434,12 +434,12 @@ pub struct ServoThreadSafeLayoutNodeChildrenIterator<'dom> {
 
 impl<'dom> ServoThreadSafeLayoutNodeChildrenIterator<'dom> {
     pub fn new(parent: ServoThreadSafeLayoutNode<'dom>) -> Self {
-        let first_child = match parent.get_pseudo_element_type() {
+        let first_child = match parent.pseudo_element() {
-            PseudoElementType::Normal => parent
+            None => parent
-                .get_before_pseudo()
+                .get_pseudo(PseudoElement::Before)
                 .or_else(|| parent.get_details_summary_pseudo())
                 .or_else(|| unsafe { parent.dangerous_first_child() }),
-            PseudoElementType::DetailsContent | PseudoElementType::DetailsSummary => unsafe {
+            Some(PseudoElement::DetailsContent) | Some(PseudoElement::DetailsSummary) => unsafe {
                 parent.dangerous_first_child()
             },
             _ => None,
@@ -455,10 +455,10 @@ impl<'dom> Iterator for ServoThreadSafeLayoutNodeChildrenIterator<'dom> {
     type Item = ServoThreadSafeLayoutNode<'dom>;
     fn next(&mut self) -> Option<ServoThreadSafeLayoutNode<'dom>> {
         use selectors::Element;
-        match self.parent_node.get_pseudo_element_type() {
+        match self.parent_node.pseudo_element() {
-            PseudoElementType::Before | PseudoElementType::After => None,
+            Some(PseudoElement::Before) | Some(PseudoElement::After) => None,
 
-            PseudoElementType::DetailsSummary => {
+            Some(PseudoElement::DetailsSummary) => {
                 let mut current_node = self.current_node;
                 loop {
                     let next_node = if let Some(ref node) = current_node {
@@ -479,7 +479,7 @@ impl<'dom> Iterator for ServoThreadSafeLayoutNodeChildrenIterator<'dom> {
                 }
             },
 
-            PseudoElementType::DetailsContent => {
+            Some(PseudoElement::DetailsContent) => {
                 let node = self.current_node;
                 let node = node.and_then(|node| {
                     if node.is_element() &&
@@ -497,22 +497,24 @@ impl<'dom> Iterator for ServoThreadSafeLayoutNodeChildrenIterator<'dom> {
                 node
             },
 
-            PseudoElementType::Normal => {
+            None | Some(_) => {
                 let node = self.current_node;
                 if let Some(ref node) = node {
-                    self.current_node = match node.get_pseudo_element_type() {
+                    self.current_node = match node.pseudo_element() {
-                        PseudoElementType::Before => self
+                        Some(PseudoElement::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()),
+                            .or_else(|| self.parent_node.get_pseudo(PseudoElement::After)),
-                        PseudoElementType::Normal => unsafe { node.dangerous_next_sibling() }
+                        Some(PseudoElement::DetailsSummary) => {
-                            .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(),
+                        Some(PseudoElement::DetailsContent) => {
-                        PseudoElementType::After => None,
+                            self.parent_node.get_pseudo(PseudoElement::After)
+                        },
+                        Some(PseudoElement::After) => None,
+                        None | Some(_) => unsafe { node.dangerous_next_sibling() }
+                            .or_else(|| self.parent_node.get_pseudo(PseudoElement::After)),
                     };
                 }
                 node

components/shared/script_layout/lib.rs

@@ -460,6 +460,16 @@ pub enum FragmentType {
     AfterPseudoContent,
 }
 
+impl From<Option<PseudoElement>> for FragmentType {
+    fn from(value: Option<PseudoElement>) -> Self {
+        match value {
+            Some(PseudoElement::After) => FragmentType::AfterPseudoContent,
+            Some(PseudoElement::Before) => FragmentType::BeforePseudoContent,
+            _ => FragmentType::FragmentBody,
+        }
+    }
+}
+
 /// The next ID that will be used for a special scroll root id.
 ///
 /// A special scroll root is a scroll root that is created for generated content.

components/shared/script_layout/wrapper_traits.rs

@@ -31,47 +31,6 @@ use crate::{
 
 pub trait LayoutDataTrait: Default + Send + Sync + 'static {}
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub enum PseudoElementType {
-    Normal,
-    Before,
-    After,
-    DetailsSummary,
-    DetailsContent,
-}
-
-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 {
-        matches!(*self, PseudoElementType::Before)
-    }
-
-    pub fn is_replaced_content(&self) -> bool {
-        matches!(*self, PseudoElementType::Before | PseudoElementType::After)
-    }
-
-    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,
-        }
-    }
-}
-
 /// A wrapper so that layout can access only the methods that it should have access to. Layout must
 /// only ever see these and must never see instances of `LayoutDom`.
 /// FIXME(mrobinson): `Send + Sync` is required here for Layout 2020, but eventually it
@@ -198,16 +157,11 @@ pub trait ThreadSafeLayoutNode<'dom>: Clone + Copy + Debug + NodeInfo + PartialE
     /// the parent until all the children have been processed.
     fn parent_style(&self) -> Arc<ComputedValues>;
 
-    fn get_before_pseudo(&self) -> Option<Self> {
+    fn get_pseudo(&self, pseudo_element: PseudoElement) -> Option<Self> {
         self.as_element()
-            .and_then(|el| el.get_before_pseudo())
+            .and_then(|element| element.get_pseudo(pseudo_element))
-            .map(|el| el.as_node())
+            .as_ref()
-    }
+            .map(ThreadSafeLayoutElement::as_node)
-
-    fn get_after_pseudo(&self) -> Option<Self> {
-        self.as_element()
-            .and_then(|el| el.get_after_pseudo())
-            .map(|el| el.as_node())
     }
 
     fn get_details_summary_pseudo(&self) -> Option<Self> {
@@ -233,12 +187,6 @@ pub trait ThreadSafeLayoutNode<'dom>: Clone + Copy + Debug + NodeInfo + PartialE
     /// Returns a ThreadSafeLayoutElement if this is an element in an HTML namespace, None otherwise.
     fn as_html_element(&self) -> Option<Self::ConcreteThreadSafeLayoutElement>;
 
-    #[inline]
-    fn get_pseudo_element_type(&self) -> PseudoElementType {
-        self.as_element()
-            .map_or(PseudoElementType::Normal, |el| el.get_pseudo_element_type())
-    }
-
     /// Get the [`StyleData`] for this node. Returns None if the node is unstyled.
     fn style_data(&self) -> Option<&'dom StyleData>;
 
@@ -313,8 +261,10 @@ pub trait ThreadSafeLayoutNode<'dom>: Clone + Copy + Debug + NodeInfo + PartialE
     fn get_colspan(&self) -> Option<u32>;
     fn get_rowspan(&self) -> Option<u32>;
 
+    fn pseudo_element(&self) -> Option<PseudoElement>;
+
     fn fragment_type(&self) -> FragmentType {
-        self.get_pseudo_element_type().fragment_type()
+        self.pseudo_element().into()
     }
 }
 
@@ -333,7 +283,7 @@ pub trait ThreadSafeLayoutElement<'dom>:
 
     /// Creates a new `ThreadSafeLayoutElement` for the same `LayoutElement`
     /// with a different pseudo-element type.
-    fn with_pseudo(&self, pseudo: PseudoElementType) -> Self;
+    fn with_pseudo(&self, pseudo: PseudoElement) -> Self;
 
     /// Returns the type ID of this node.
     /// Returns `None` if this is a pseudo-element; otherwise, returns `Some`.
@@ -357,33 +307,18 @@ pub trait ThreadSafeLayoutElement<'dom>:
 
     fn style_data(&self) -> AtomicRef<ElementData>;
 
-    fn get_pseudo_element_type(&self) -> PseudoElementType;
+    fn pseudo_element(&self) -> Option<PseudoElement>;
 
     #[inline]
-    fn get_before_pseudo(&self) -> Option<Self> {
+    fn get_pseudo(&self, pseudo_element: PseudoElement) -> Option<Self> {
         if self
             .style_data()
             .styles
             .pseudos
-            .get(&PseudoElement::Before)
+            .get(&pseudo_element)
             .is_some()
         {
-            Some(self.with_pseudo(PseudoElementType::Before))
+            Some(self.with_pseudo(pseudo_element))
-        } else {
-            None
-        }
-    }
-
-    #[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))
         } else {
             None
         }
@@ -392,7 +327,7 @@ pub trait ThreadSafeLayoutElement<'dom>:
     #[inline]
     fn get_details_summary_pseudo(&self) -> Option<Self> {
         if self.has_local_name(&local_name!("details")) && self.has_namespace(&ns!(html)) {
-            Some(self.with_pseudo(PseudoElementType::DetailsSummary))
+            Some(self.with_pseudo(PseudoElement::DetailsSummary))
         } else {
             None
         }
@@ -404,7 +339,7 @@ pub trait ThreadSafeLayoutElement<'dom>:
             self.has_namespace(&ns!(html)) &&
             self.get_attr(&ns!(), &local_name!("open")).is_some()
         {
-            Some(self.with_pseudo(PseudoElementType::DetailsContent))
+            Some(self.with_pseudo(PseudoElement::DetailsContent))
         } else {
             None
         }
@@ -417,12 +352,11 @@ pub trait ThreadSafeLayoutElement<'dom>:
     #[inline]
     fn style(&self, context: &SharedStyleContext) -> Arc<ComputedValues> {
         let data = self.style_data();
-        match self.get_pseudo_element_type() {
+        match self.pseudo_element() {
-            PseudoElementType::Normal => data.styles.primary().clone(),
+            None => data.styles.primary().clone(),
-            other => {
+            Some(style_pseudo) => {
                 // Precompute non-eagerly-cascaded pseudo-element styles if not
                 // cached before.
-                let style_pseudo = other.style_pseudo_element();
                 match style_pseudo.cascade_type() {
                     // Already computed during the cascade.
                     PseudoElementCascadeType::Eager => self
@@ -478,14 +412,9 @@ pub trait ThreadSafeLayoutElement<'dom>:
     #[inline]
     fn resolved_style(&self) -> Arc<ComputedValues> {
         let data = self.style_data();
-        match self.get_pseudo_element_type() {
+        match self.pseudo_element() {
-            PseudoElementType::Normal => data.styles.primary().clone(),
+            None => data.styles.primary().clone(),
-            other => data
+            Some(pseudo_element) => data.styles.pseudos.get(&pseudo_element).unwrap().clone(),
-                .styles
-                .pseudos
-                .get(&other.style_pseudo_element())
-                .unwrap()
-                .clone(),
         }
     }