Auto merge of #17528 - bzbarsky:first-letter-better, r=emilio

Fix stylo support for first-letter

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1324618

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17528)
<!-- Reviewable:end -->

components/style/dom.rs

@@ -426,9 +426,19 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
     /// TODO(emilio, bz): actually implement the logic for it.
     fn may_generate_pseudo(
         &self,
-        _pseudo: &PseudoElement,
+        pseudo: &PseudoElement,
         _primary_style: &ComputedValues,
     ) -> bool {
+        // ::before/::after are always supported for now, though we could try to
+        // optimize out leaf elements.
+
+        // ::first-letter and ::first-line are only supported for block-inside
+        // things, and only in Gecko, not Servo.  Unfortunately, Gecko has
+        // block-inside things that might have any computed display value due to
+        // things like fieldsets, legends, etc.  Need to figure out how this
+        // should work.
+        debug_assert!(pseudo.is_eager(),
+                      "Someone called may_generate_pseudo with a non-eager pseudo.");
         true
     }
 

components/style/gecko/generated/bindings.rs

@@ -2654,6 +2654,8 @@ extern "C" {
     pub fn Servo_ResolvePseudoStyle(element: RawGeckoElementBorrowed,
                                     pseudo_type: CSSPseudoElementType,
                                     is_probe: bool,
+                                    inherited_style:
+                                        ServoComputedValuesBorrowedOrNull,
                                     set: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong;
 }

components/style/gecko/pseudo_element.rs

@@ -77,6 +77,12 @@ impl PseudoElement {
         matches!(*self, PseudoElement::Before | PseudoElement::After)
     }
 
+    /// Whether this pseudo-element is ::first-letter.
+    #[inline]
+    pub fn is_first_letter(&self) -> bool {
+        *self == PseudoElement::FirstLetter
+    }
+
     /// Whether this pseudo-element is lazily-cascaded.
     #[inline]
     pub fn is_lazy(&self) -> bool {

components/style/matching.rs

@@ -1202,6 +1202,14 @@ pub trait MatchMethods : TElement {
         // Compute rule nodes for eagerly-cascaded pseudo-elements.
         let mut matches_different_pseudos = false;
         SelectorImpl::each_eagerly_cascaded_pseudo_element(|pseudo| {
+            // For pseudo-elements, we only try to match visited rules if there
+            // are also unvisited rules.  (This matches Gecko's behavior.)
+            if visited_handling == VisitedHandlingMode::RelevantLinkVisited &&
+               !context.cascade_inputs().pseudos.has(&pseudo) {
+                return
+            }
+
+            if self.may_generate_pseudo(&pseudo, data.styles.primary()) {
                 let bloom_filter = context.thread_local.bloom_filter.filter();
 
                 let mut matching_context =
@@ -1210,18 +1218,6 @@ pub trait MatchMethods : TElement {
                                                      visited_handling,
                                                      context.shared.quirks_mode);
 
-            // For pseudo-elements, we only try to match visited rules if there
-            // are also unvisited rules.  (This matches Gecko's behavior.)
-            if visited_handling == VisitedHandlingMode::RelevantLinkVisited &&
-               !context.cascade_inputs().pseudos.has(&pseudo) {
-                return
-            }
-
-            if !self.may_generate_pseudo(&pseudo, data.styles.primary()) {
-                return;
-            }
-
-            {
                 let map = &mut context.thread_local.selector_flags;
                 let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
                     self.apply_selector_flags(map, element, flags);
@@ -1482,6 +1478,7 @@ pub trait MatchMethods : TElement {
                                 pseudo: Option<&PseudoElement>)
                                 -> StyleDifference
     {
+        debug_assert!(pseudo.map_or(true, |p| p.is_eager()));
         if let Some(source) = self.existing_style_for_restyle_damage(old_values, pseudo) {
             return RestyleDamage::compute_style_difference(source, new_values)
         }
@@ -1510,9 +1507,25 @@ pub trait MatchMethods : TElement {
                 // triggering any change.
                 return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
             }
+            // FIXME(bz): This will keep reframing replaced elements.  Do we
+            // need this at all?  Seems like if we add/remove ::before or
+            // ::after styles we would get reframed over in match_pseudos and if
+            // that part didn't change and we had no frame for the
+            // ::before/::after then we don't care.  Need to double-check that
+            // we handle the "content" and "display" properties changing
+            // correctly, though.
+            // https://bugzilla.mozilla.org/show_bug.cgi?id=1376352
             return StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
         }
 
+        if pseudo.map_or(false, |p| p.is_first_letter()) {
+            // No one cares about this pseudo, and we've checked above that
+            // we're not switching from a "cares" to a "doesn't care" state
+            // or vice versa.
+            return StyleDifference::new(RestyleDamage::empty(),
+                                        StyleChange::Unchanged)
+        }
+
         // Something else. Be conservative for now.
         warn!("Reframing due to lack of old style source: {:?}, pseudo: {:?}",
                self, pseudo);

components/style/servo/selector_parser.rs

@@ -38,6 +38,7 @@ pub enum PseudoElement {
     After = 0,
     Before,
     Selection,
+    // If/when :first-letter is added, update is_first_letter accordingly.
     // Non-eager pseudos.
     DetailsSummary,
     DetailsContent,
@@ -110,6 +111,12 @@ impl PseudoElement {
         matches!(*self, PseudoElement::After | PseudoElement::Before)
     }
 
+    /// Whether the current pseudo element is :first-letter
+    #[inline]
+    pub fn is_first_letter(&self) -> bool {
+        false
+    }
+
     /// Whether this pseudo-element is eagerly-cascaded.
     #[inline]
     pub fn is_eager(&self) -> bool {

components/style/stylist.rs

@@ -685,7 +685,25 @@ impl Stylist {
         where E: TElement,
     {
         let rule_node =
-            match self.lazy_pseudo_rules(guards, element, pseudo, rule_inclusion) {
+            self.lazy_pseudo_rules(guards, element, pseudo, rule_inclusion);
+        self.compute_pseudo_element_style_with_rulenode(rule_node.as_ref(),
+                                                        guards,
+                                                        parent_style,
+                                                        font_metrics)
+    }
+
+    /// Computes a pseudo-element style lazily using the given rulenode.  This
+    /// can be used for truly lazy pseudo-elements or to avoid redoing selector
+    /// matching for eager pseudo-elements when we need to recompute their style
+    /// with a new parent style.
+    pub fn compute_pseudo_element_style_with_rulenode(&self,
+                                                      rule_node: Option<&StrongRuleNode>,
+                                                      guards: &StylesheetGuards,
+                                                      parent_style: &ComputedValues,
+                                                      font_metrics: &FontMetricsProvider)
+                                                      -> Option<Arc<ComputedValues>>
+    {
+        let rule_node = match rule_node {
             Some(rule_node) => rule_node,
             None => return None
         };
@@ -697,7 +715,7 @@ impl Stylist {
         // Bug 1364242: We need to add visited support for lazy pseudos
         let computed =
             properties::cascade(&self.device,
-                                &rule_node,
+                                rule_node,
                                 guards,
                                 Some(parent_style),
                                 Some(parent_style),

ports/geckolib/glue.rs

@@ -1447,6 +1447,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null:
 pub extern "C" fn Servo_ResolvePseudoStyle(element: RawGeckoElementBorrowed,
                                            pseudo_type: CSSPseudoElementType,
                                            is_probe: bool,
+                                           inherited_style: ServoComputedValuesBorrowedOrNull,
                                            raw_data: RawServoStyleSetBorrowed)
      -> ServoComputedValuesStrong
 {
@@ -1478,6 +1479,7 @@ pub extern "C" fn Servo_ResolvePseudoStyle(element: RawGeckoElementBorrowed,
         &pseudo,
         RuleInclusion::All,
         &data.styles,
+        ComputedValues::arc_from_borrowed(&inherited_style).map(|v| v.as_ref()),
         &*doc_data,
         is_probe
     );
@@ -1517,13 +1519,47 @@ fn get_pseudo_style(
     pseudo: &PseudoElement,
     rule_inclusion: RuleInclusion,
     styles: &ElementStyles,
+    inherited_styles: Option<&ComputedValues>,
     doc_data: &PerDocumentStyleDataImpl,
     is_probe: bool,
 ) -> Option<Arc<ComputedValues>> {
     let style = match pseudo.cascade_type() {
-        PseudoElementCascadeType::Eager => styles.pseudos.get(&pseudo).map(|s| s.clone()),
+        PseudoElementCascadeType::Eager => {
+            match *pseudo {
+                PseudoElement::FirstLetter => {
+                    // inherited_styles can be None when doing lazy resolution
+                    // (e.g. for computed style) or when probing.  In that case
+                    // we just inherit from our element, which is what Gecko
+                    // does in that situation.  What should actually happen in
+                    // the computed style case is a bit unclear.
+                    let inherited_styles =
+                        inherited_styles.unwrap_or(styles.primary());
+                    let guards = StylesheetGuards::same(guard);
+                    let metrics = get_metrics_provider_for_product();
+                    let rule_node = match styles.pseudos.get(&pseudo) {
+                        Some(styles) => styles.rules.as_ref(),
+                        None => None,
+                    };
+                    doc_data.stylist
+                        .compute_pseudo_element_style_with_rulenode(
+                            rule_node,
+                            &guards,
+                            inherited_styles,
+                            &metrics)
+                },
+                _ => {
+                    debug_assert!(inherited_styles.is_none() ||
+                                  ptr::eq(inherited_styles.unwrap(),
+                                          &**styles.primary()));
+                    styles.pseudos.get(&pseudo).cloned()
+                },
+            }
+        }
         PseudoElementCascadeType::Precomputed => unreachable!("No anonymous boxes"),
         PseudoElementCascadeType::Lazy => {
+            debug_assert!(inherited_styles.is_none() ||
+                          ptr::eq(inherited_styles.unwrap(),
+                                  &**styles.primary()));
             let base = if pseudo.inherits_from_default_values() {
                 doc_data.default_computed_values()
             } else {
@@ -1539,7 +1575,6 @@ fn get_pseudo_style(
                     rule_inclusion,
                     base,
                     &metrics)
-                .map(|s| s.clone())
         },
     };
 
@@ -2633,6 +2668,7 @@ pub extern "C" fn Servo_ResolveStyleLazily(element: RawGeckoElementBorrowed,
                     pseudo,
                     rule_inclusion,
                     styles,
+                    /* inherited_styles = */ None,
                     &*data,
                     /* is_probe = */ false,
                 ).expect("We're not probing, so we should always get a style \