style: Store in Gecko PseudoElements whether they skip parent display-based style fixups. r=emilio draft
authorCameron McCormack <cam@mcc.id.au>
Mon, 24 Apr 2017 16:28:36 +0800
changeset 567515 a19d334d84b14969d85a4b9d12f11d8e61235f80
parent 567514 fbfc6f9ea565d203c7b618dd7494ecb88a341073
child 567516 6dd8a680b92b885bac43d48b55d5f347556a3282
push id55600
push userbmo:cam@mcc.id.au
push dateTue, 25 Apr 2017 04:55:45 +0000
reviewersemilio
milestone55.0a1
style: Store in Gecko PseudoElements whether they skip parent display-based style fixups. r=emilio MozReview-Commit-ID: 20FAhb3cydp
servo/components/style/gecko/selector_parser.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/gecko/selector_parser.rs
+++ b/servo/components/style/gecko/selector_parser.rs
@@ -15,36 +15,47 @@ use std::borrow::Cow;
 use std::fmt;
 use std::ptr;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 
 /// A representation of a CSS pseudo-element.
 ///
 /// In Gecko, we represent pseudo-elements as plain `Atom`s.
 ///
-/// The boolean field represents whether this element is an anonymous box. This
-/// is just for convenience, instead of recomputing it.
-///
 /// Also, note that the `Atom` member is always a static atom, so if space is a
 /// concern, we can use the raw pointer and use the lower bit to represent it
 /// without space overhead.
 ///
 /// FIXME(emilio): we know all these atoms are static. Patches are starting to
 /// pile up, but a further potential optimisation is generating bindings without
 /// `-no-gen-bitfield-methods` (that was removed to compile on stable, but it no
 /// longer depends on it), and using the raw *mut nsIAtom (properly asserting
 /// we're a static atom).
 ///
 /// This should allow us to avoid random FFI overhead when cloning/dropping
 /// pseudos.
 ///
 /// Also, we can further optimize PartialEq and hash comparing/hashing only the
 /// atoms.
+///
+/// The two boolean fields are stored just for convenience, to avoid recomputing
+/// them from the gecko_pseudo_element_helper.rs data.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
-pub struct PseudoElement(Atom, bool);
+pub struct PseudoElement {
+    /// The atom representing the pseudo-element, with only a single leading
+    /// colon.
+    atom: Atom,
+
+    /// Whether this pseudo-element is an anonymous box.
+    is_anon_box: bool,
+
+    /// Whether this pseudo-element skips parent display-based style fixups.
+    /// Must always be false if is_anon_box is false.
+    skips_display_fixup: bool,
+}
 
 /// List of eager pseudos. Keep this in sync with the count below.
 macro_rules! each_eager_pseudo {
     ($macro_name:ident, $atom_macro:ident) => {
         $macro_name!($atom_macro!(":after"), 0);
         $macro_name!($atom_macro!(":before"), 1);
     }
 }
@@ -63,32 +74,46 @@ impl PseudoElement {
         each_eager_pseudo!(case, atom);
         panic!("Not eager")
     }
 
     /// Creates a pseudo-element from an eager index.
     #[inline]
     pub fn from_eager_index(i: usize) -> Self {
         macro_rules! case {
-            ($atom:expr, $idx:expr) => { if i == $idx { return PseudoElement($atom, false); } }
+            ($atom:expr, $idx:expr) => {
+                if i == $idx {
+                    return PseudoElement {
+                        atom: $atom,
+                        is_anon_box: false,
+                        skips_display_fixup: false
+                    };
+                }
+            }
         }
         each_eager_pseudo!(case, atom);
         panic!("Not eager")
     }
 
     /// Get the pseudo-element as an atom.
     #[inline]
     pub fn as_atom(&self) -> &Atom {
-        &self.0
+        &self.atom
     }
 
     /// Whether this pseudo-element is an anonymous box.
     #[inline]
     fn is_anon_box(&self) -> bool {
-        self.1
+        self.is_anon_box
+    }
+
+    /// Whether this pseudo-element skips parent display-based style fixups.
+    #[inline]
+    pub fn skips_display_fixup(&self) -> bool {
+        self.skips_display_fixup
     }
 
     /// Whether this pseudo-element is ::before or ::after.
     #[inline]
     pub fn is_before_or_after(&self) -> bool {
         *self.as_atom() == atom!(":before") ||
         *self.as_atom() == atom!(":after")
     }
@@ -115,37 +140,46 @@ impl PseudoElement {
         self.is_anon_box()
     }
 
     /// Construct a pseudo-element from an `Atom`, receiving whether it is also
     /// an anonymous box, and don't check it on release builds.
     ///
     /// On debug builds we assert it's the result we expect.
     #[inline]
-    pub fn from_atom_unchecked(atom: Atom, is_anon_box: bool) -> Self {
+    pub fn from_atom_unchecked(atom: Atom, is_anon_box: bool, skips_display_fixup: bool) -> Self {
         if cfg!(debug_assertions) {
             // Do the check on debug regardless.
             match Self::from_atom(&*atom, true) {
                 Some(pseudo) => {
                     assert_eq!(pseudo.is_anon_box(), is_anon_box);
+                    assert_eq!(pseudo.skips_display_fixup(), skips_display_fixup);
                     return pseudo;
                 }
                 None => panic!("Unknown pseudo: {:?}", atom),
             }
         }
 
-        PseudoElement(atom, is_anon_box)
+        PseudoElement {
+            atom: atom,
+            is_anon_box: is_anon_box,
+            skips_display_fixup: skips_display_fixup,
+        }
     }
 
     #[inline]
     fn from_atom(atom: &WeakAtom, _in_ua: bool) -> Option<Self> {
         macro_rules! pseudo_element {
-            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr) => {{
+            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr, $skips_display_fixup:expr) => {{
                 if atom == &*$atom {
-                    return Some(PseudoElement($atom, $is_anon_box));
+                    return Some(PseudoElement {
+                        atom: $atom,
+                        is_anon_box: $is_anon_box,
+                        skips_display_fixup: $skips_display_fixup
+                    });
                 }
             }}
         }
 
         include!("generated/gecko_pseudo_element_helper.rs");
 
         None
     }
@@ -156,20 +190,24 @@ impl PseudoElement {
     /// If we're not in a user-agent stylesheet, we will never parse anonymous
     /// box pseudo-elements.
     ///
     /// Returns `None` if the pseudo-element is not recognised.
     #[inline]
     fn from_slice(s: &str, in_ua_stylesheet: bool) -> Option<Self> {
         use std::ascii::AsciiExt;
         macro_rules! pseudo_element {
-            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr) => {{
+            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr, $skips_display_fixup:expr) => {{
                 if !$is_anon_box || in_ua_stylesheet {
                     if s.eq_ignore_ascii_case(&$pseudo_str_with_colon[1..]) {
-                        return Some(PseudoElement($atom, $is_anon_box))
+                        return Some(PseudoElement {
+                            atom: $atom,
+                            is_anon_box: $is_anon_box,
+                            skips_display_fixup: $skips_display_fixup
+                        });
                     }
                 }
             }}
         }
 
         include!("generated/gecko_pseudo_element_helper.rs");
 
         None
@@ -180,20 +218,20 @@ impl PseudoElement {
     pub fn ns_atom_or_null_from_opt(pseudo: Option<&PseudoElement>) -> *mut nsIAtom {
         pseudo.map(|p| p.as_atom().as_ptr()).unwrap_or(ptr::null_mut())
     }
 }
 
 impl ToCss for PseudoElement {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         // FIXME: why does the atom contain one colon? Pseudo-element has two
-        debug_assert!(self.0.as_slice().starts_with(&[b':' as u16]) &&
-                      !self.0.as_slice().starts_with(&[b':' as u16, b':' as u16]));
+        debug_assert!(self.atom.as_slice().starts_with(&[b':' as u16]) &&
+                      !self.atom.as_slice().starts_with(&[b':' as u16, b':' as u16]));
         try!(dest.write_char(':'));
-        write!(dest, "{}", self.0)
+        write!(dest, "{}", self.atom)
     }
 }
 
 bitflags! {
     flags NonTSPseudoClassFlag: u8 {
         // See NonTSPseudoClass::is_internal()
         const PSEUDO_CLASS_INTERNAL = 0x01,
     }
@@ -459,30 +497,40 @@ impl SelectorImpl {
 
     /// A helper to traverse each eagerly cascaded pseudo-element, executing
     /// `fun` on it.
     #[inline]
     pub fn each_eagerly_cascaded_pseudo_element<F>(mut fun: F)
         where F: FnMut(PseudoElement),
     {
         macro_rules! case {
-            ($atom:expr, $idx:expr) => { fun(PseudoElement($atom, false)); }
+            ($atom:expr, $idx:expr) => {
+                fun(PseudoElement {
+                    atom: $atom,
+                    is_anon_box: false,
+                    skips_display_fixup: false
+                });
+            }
         }
         each_eager_pseudo!(case, atom);
     }
 
 
     #[inline]
     /// Executes a function for each pseudo-element.
     pub fn each_pseudo_element<F>(mut fun: F)
         where F: FnMut(PseudoElement),
     {
         macro_rules! pseudo_element {
-            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr) => {{
-                fun(PseudoElement($atom, $is_anon_box));
+            ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr, $skips_display_fixup:expr) => {{
+                fun(PseudoElement {
+                    atom: $atom,
+                    is_anon_box: $is_anon_box,
+                    skips_display_fixup: $skips_display_fixup,
+                });
             }}
         }
 
         include!("generated/gecko_pseudo_element_helper.rs")
     }
 
     #[inline]
     /// Returns the relevant state flag for a given non-tree-structural
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -446,17 +446,19 @@ pub extern "C" fn Servo_StyleSet_GetBase
     let element = GeckoElement(element);
     let element_data = element.borrow_data().unwrap();
     let styles = element_data.styles();
 
     let pseudo = if pseudo_tag.is_null() {
         None
     } else {
         let atom = Atom::from(pseudo_tag);
-        Some(PseudoElement::from_atom_unchecked(atom, /* anon_box = */ false))
+        Some(PseudoElement::from_atom_unchecked(atom,
+                                                /* anon_box = */ false,
+                                                /* skip_display_fixup = */ false))
     };
     let pseudos = &styles.pseudos;
     let pseudo_style = match pseudo {
         Some(ref p) => {
             let style = pseudos.get(p);
             debug_assert!(style.is_some());
             style
         }
@@ -886,18 +888,17 @@ pub extern "C" fn Servo_ComputedValues_G
                                                           skip_display_fixup: bool,
                                                           raw_data: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong {
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
     let guards = StylesheetGuards::same(&guard);
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
     let atom = Atom::from(pseudo_tag);
-    let pseudo = PseudoElement::from_atom_unchecked(atom, /* anon_box = */ true);
-
+    let pseudo = PseudoElement::from_atom_unchecked(atom, /* anon_box = */ true, skip_display_fixup);
 
     let maybe_parent = ComputedValues::arc_from_borrowed(&parent_style_or_null);
     let mut cascade_flags = CascadeFlags::empty();
     if skip_display_fixup {
         cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP);
     }
     let metrics = get_metrics_provider_for_product();
     data.stylist.precomputed_values_for_pseudo(&guards, &pseudo, maybe_parent,
@@ -934,17 +935,19 @@ pub extern "C" fn Servo_ResolvePseudoSty
         None => Strong::null(),
     }
 }
 
 fn get_pseudo_style(guard: &SharedRwLockReadGuard, element: GeckoElement, pseudo_tag: *mut nsIAtom,
                     styles: &ElementStyles, doc_data: &PerDocumentStyleData)
                     -> Option<Arc<ComputedValues>>
 {
-    let pseudo = PseudoElement::from_atom_unchecked(Atom::from(pseudo_tag), false);
+    let pseudo = PseudoElement::from_atom_unchecked(Atom::from(pseudo_tag),
+                                                    /* anon_box = */ false,
+                                                    /* skip_display_fixup = */ false);
     match SelectorImpl::pseudo_element_cascade_type(&pseudo) {
         PseudoElementCascadeType::Eager => styles.pseudos.get(&pseudo).map(|s| s.values().clone()),
         PseudoElementCascadeType::Precomputed => unreachable!("No anonymous boxes"),
         PseudoElementCascadeType::Lazy => {
             let d = doc_data.borrow_mut();
             let base = styles.primary.values();
             let guards = StylesheetGuards::same(guard);
             let metrics = get_metrics_provider_for_product();