Auto merge of #16648 - emilio:combine, r=jryans

Bug 1360508: Adjust text-combine properly. r=jryans

Text style is always resolved via ServoStyleSet::ResolveStyleForText, either
from the frame constructor initially, or from
ServoRestyleManager::ProcessPostTraversalForText.

So text-only adjustments should go there instead. Since that doesn't call into
cascade(), all the code that passes `pseudo` there is dead code we can remove.

MozReview-Commit-ID: jpbBYpLlUL
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

<!-- 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/16648)
<!-- Reviewable:end -->

components/style/animation.rs

@@ -461,7 +461,6 @@ fn compute_style_for_animation_step(context: &SharedStyleContext,
             let computed =
                 properties::apply_declarations(&context.stylist.device,
                                                /* is_root = */ false,
-                                               /* pseudo = */ None,
                                                iter,
                                                previous_style,
                                                previous_style,

components/style/build_gecko.rs

@@ -468,6 +468,7 @@ mod bindings {
             "mozilla::binding_danger::AssertAndSuppressCleanupPolicy",
             "RawServoAnimationValueMapBorrowed",
             "mozilla::LengthParsingMode",
+            "mozilla::InheritTarget",
         ];
         let opaque_types = [
             "std::pair__PCCP",
@@ -725,6 +726,7 @@ mod bindings {
             "EffectCompositor_CascadeLevel",
             "UpdateAnimationsTasks",
             "LengthParsingMode",
+            "InheritTarget",
         ];
         struct ArrayType {
             cpp_type: &'static str,

components/style/gecko_bindings/bindings.rs

@@ -191,6 +191,7 @@ use gecko_bindings::structs::ServoStyleSheet;
 use gecko_bindings::structs::EffectCompositor_CascadeLevel;
 use gecko_bindings::structs::UpdateAnimationsTasks;
 use gecko_bindings::structs::LengthParsingMode;
+use gecko_bindings::structs::InheritTarget;
 pub type nsTArrayBorrowed_uintptr_t<'a> = &'a mut ::gecko_bindings::structs::nsTArray<usize>;
 pub type ServoCssRulesStrong = ::gecko_bindings::sugar::ownership::Strong<ServoCssRules>;
 pub type ServoCssRulesBorrowed<'a> = &'a ServoCssRules;
@@ -2107,7 +2108,8 @@ extern "C" {
 extern "C" {
     pub fn Servo_ComputedValues_Inherit(set: RawServoStyleSetBorrowed,
                                         parent_style:
-                                            ServoComputedValuesBorrowedOrNull)
+                                            ServoComputedValuesBorrowedOrNull,
+                                        target: InheritTarget)
      -> ServoComputedValuesStrong;
 }
 extern "C" {

components/style/gecko_bindings/structs_debug.rs

@@ -6682,6 +6682,13 @@ pub mod root {
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
         pub enum LengthParsingMode { Default = 0, SVG = 1, }
+        #[repr(i32)]
+        #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+        pub enum InheritTarget {
+            Text = 0,
+            FirstLetterContinuation = 1,
+            PlaceholderFrame = 2,
+        }
         pub type CSSPseudoElementTypeBase = u8;
         pub const CSSPseudoElementType_InheritingAnonBox:
                   root::mozilla::CSSPseudoElementType =

components/style/gecko_bindings/structs_release.rs

@@ -6585,6 +6585,13 @@ pub mod root {
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
         pub enum LengthParsingMode { Default = 0, SVG = 1, }
+        #[repr(i32)]
+        #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+        pub enum InheritTarget {
+            Text = 0,
+            FirstLetterContinuation = 1,
+            PlaceholderFrame = 2,
+        }
         pub type CSSPseudoElementTypeBase = u8;
         pub const CSSPseudoElementType_InheritingAnonBox:
                   root::mozilla::CSSPseudoElementType =

components/style/matching.rs

@@ -410,8 +410,7 @@ trait PrivateMatchMethods: TElement {
                           font_metrics_provider: &FontMetricsProvider,
                           rule_node: &StrongRuleNode,
                           primary_style: &ComputedStyle,
-                          inherit_mode: InheritMode,
+                          inherit_mode: InheritMode)
-                          pseudo: Option<&PseudoElement>)
                           -> Arc<ComputedValues> {
         let mut cascade_info = CascadeInfo::new();
         let mut cascade_flags = CascadeFlags::empty();
@@ -476,7 +475,6 @@ trait PrivateMatchMethods: TElement {
         let values =
             Arc::new(cascade(&shared_context.stylist.device,
                              rule_node,
-                             pseudo,
                              &shared_context.guards,
                              style_to_inherit_from,
                              layout_parent_style,
@@ -493,7 +491,6 @@ trait PrivateMatchMethods: TElement {
     fn cascade_internal(&self,
                         context: &StyleContext<Self>,
                         primary_style: &ComputedStyle,
-                        pseudo: Option<&PseudoElement>,
                         eager_pseudo_style: Option<&ComputedStyle>)
                         -> Arc<ComputedValues> {
         // Grab the rule node.
@@ -508,8 +505,7 @@ trait PrivateMatchMethods: TElement {
                                 &context.thread_local.font_metrics_provider,
                                 rule_node,
                                 primary_style,
-                                inherit_mode,
+                                inherit_mode)
-                                pseudo)
     }
 
     /// Computes values and damage for the primary or pseudo style of an element,
@@ -561,7 +557,6 @@ trait PrivateMatchMethods: TElement {
                 } else {
                     self.cascade_internal(context,
                                           primary_style,
-                                          None,
                                           None)
                 }
             }
@@ -570,7 +565,6 @@ trait PrivateMatchMethods: TElement {
                 // work.
                 self.cascade_internal(context,
                                       primary_style,
-                                      pseudo,
                                       pseudo_style.as_ref().map(|s| &**s))
             }
         };
@@ -612,8 +606,7 @@ trait PrivateMatchMethods: TElement {
     #[cfg(feature = "gecko")]
     fn get_after_change_style(&self,
                               context: &mut StyleContext<Self>,
-                              primary_style: &ComputedStyle,
+                              primary_style: &ComputedStyle)
-                              pseudo: Option<&PseudoElement>)
                               -> Option<Arc<ComputedValues>> {
         let rule_node = &primary_style.rules;
         let without_transition_rules =
@@ -628,8 +621,7 @@ trait PrivateMatchMethods: TElement {
                                      &context.thread_local.font_metrics_provider,
                                      &without_transition_rules,
                                      primary_style,
-                                     InheritMode::FromParentElement,
+                                     InheritMode::FromParentElement))
-                                     pseudo))
     }
 
     #[cfg(feature = "gecko")]
@@ -674,7 +666,7 @@ trait PrivateMatchMethods: TElement {
         let before_change_style = if self.might_need_transitions_update(old_values.as_ref().map(|s| &**s),
                                                                         new_values) {
             let after_change_style = if self.has_css_transitions() {
-                self.get_after_change_style(context, primary_style, None)
+                self.get_after_change_style(context, primary_style)
             } else {
                 None
             };
@@ -1435,7 +1427,6 @@ pub trait MatchMethods : TElement {
                       shared_context: &SharedStyleContext,
                       font_metrics_provider: &FontMetricsProvider,
                       primary_style: &ComputedStyle,
-                      pseudo: Option<&PseudoElement>,
                       pseudo_style: Option<&ComputedStyle>)
                       -> Arc<ComputedValues> {
         let relevant_style = pseudo_style.unwrap_or(primary_style);
@@ -1452,8 +1443,7 @@ pub trait MatchMethods : TElement {
                                 font_metrics_provider,
                                 &without_animation_rules,
                                 primary_style,
-                                InheritMode::FromParentElement,
+                                InheritMode::FromParentElement)
-                                pseudo)
     }
 
 }

components/style/properties/gecko.mako.rs

@@ -102,8 +102,10 @@ pub struct ComputedValues {
 }
 
 impl ComputedValues {
-    pub fn inherit_from(parent: &Self, default: &Self) -> Arc<Self> {
+    /// Inherits style from the parent element, accounting for the default
-        Arc::new(ComputedValues {
+    /// computed values that need to be provided as well.
+    pub fn inherit_from(parent: &Self, default: &Self) -> Self {
+        ComputedValues {
             custom_properties: parent.custom_properties.clone(),
             writing_mode: parent.writing_mode,
             root_font_size: parent.root_font_size,
@@ -116,7 +118,7 @@ impl ComputedValues {
             ${style_struct.ident}: default.${style_struct.ident}.clone(),
             % endif
             % endfor
-        })
+        }
     }
 
     pub fn new(custom_properties: Option<Arc<ComputedValuesMap>>,

components/style/properties/properties.mako.rs

@@ -30,7 +30,6 @@ use logical_geometry::WritingMode;
 use media_queries::Device;
 use parser::{LengthParsingMode, Parse, ParserContext};
 use properties::animated_properties::TransitionProperty;
-use selector_parser::PseudoElement;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
 use shared_lock::StylesheetGuards;
 use style_traits::ToCss;
@@ -2114,7 +2113,6 @@ bitflags! {
 ///
 pub fn cascade(device: &Device,
                rule_node: &StrongRuleNode,
-               pseudo: Option<<&PseudoElement>,
                guards: &StylesheetGuards,
                parent_style: Option<<&ComputedValues>,
                layout_parent_style: Option<<&ComputedValues>,
@@ -2161,7 +2159,6 @@ pub fn cascade(device: &Device,
     };
     apply_declarations(device,
                        is_root_element,
-                       pseudo,
                        iter_declarations,
                        inherited_style,
                        layout_parent_style,
@@ -2177,7 +2174,6 @@ pub fn cascade(device: &Device,
 #[allow(unused_mut)] // conditionally compiled code for "position"
 pub fn apply_declarations<'a, F, I>(device: &Device,
                                     is_root_element: bool,
-                                    pseudo: Option<<&PseudoElement>,
                                     iter_declarations: F,
                                     inherited_style: &ComputedValues,
                                     layout_parent_style: &ComputedValues,
@@ -2386,7 +2382,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
 
     let mut style = context.style;
 
-    StyleAdjuster::new(&mut style, is_root_element, pseudo)
+    StyleAdjuster::new(&mut style, is_root_element)
         .adjust(context.layout_parent_style,
                 flags.contains(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP));
 

components/style/style_adjuster.rs

@@ -11,23 +11,20 @@ use properties::longhands::display::computed_value::T as display;
 use properties::longhands::float::computed_value::T as float;
 use properties::longhands::overflow_x::computed_value::T as overflow;
 use properties::longhands::position::computed_value::T as position;
-use selector_parser::PseudoElement;
+
 
 /// An unsized struct that implements all the adjustment methods.
-#[allow(dead_code)] // `pseudo` field is currently unused by Servo
 pub struct StyleAdjuster<'a> {
     style: &'a mut ComputedValues,
     is_root_element: bool,
-    pseudo: Option<&'a PseudoElement>,
 }
 
 impl<'a> StyleAdjuster<'a> {
     /// Trivially constructs a new StyleAdjuster.
-    pub fn new(style: &'a mut ComputedValues, is_root_element: bool, pseudo: Option<&'a PseudoElement>) -> Self {
+    pub fn new(style: &'a mut ComputedValues, is_root_element: bool) -> Self {
         StyleAdjuster {
             style: style,
             is_root_element: is_root_element,
-            pseudo: pseudo,
         }
     }
 
@@ -90,23 +87,37 @@ impl<'a> StyleAdjuster<'a> {
         }
     }
 
-    /// Change writing mode of text frame for text-combine-upright.
+    /// Adjust the style for text style.
-    /// It is safe to look at the parent's style because we are looking at
+    ///
-    /// inherited properties, and ::-moz-text never matches any rules.
+    /// The adjustments here are a subset of the adjustments generally, because
+    /// text only inherits properties.
+    ///
+    /// Note that this, for Gecko, comes through Servo_ComputedValues_Inherit.
     #[cfg(feature = "gecko")]
-    fn adjust_for_text_combine_upright(&mut self,
+    pub fn adjust_for_text(&mut self) {
-                                       layout_parent_style: &ComputedValues) {
+        self.adjust_for_text_combine_upright();
-        if let Some(p) = self.pseudo {
+    }
-            if *p.as_atom() == atom!(":-moz-text") {
+
-                use computed_values::text_combine_upright::T as text_combine_upright;
+    /// Change writing mode of the text frame for text-combine-upright.
-                use computed_values::writing_mode::T as writing_mode;
+    ///
-                let parent_writing_mode = layout_parent_style.get_inheritedbox().clone_writing_mode();
+    /// It is safe to look at our own style because we are looking at inherited
-                let parent_text_combine_upright = layout_parent_style.get_inheritedtext().clone_text_combine_upright();
+    /// properties, and text is just plain inheritance.
-                if parent_writing_mode != writing_mode::horizontal_tb &&
+    ///
-                   parent_text_combine_upright == text_combine_upright::all {
+    /// TODO(emilio): we should (Gecko too) revise these adjustments in presence
-                    self.style.mutate_inheritedbox().set_writing_mode(writing_mode::horizontal_tb);
+    /// of display: contents.
-                }
+    #[cfg(feature = "gecko")]
-            }
+    fn adjust_for_text_combine_upright(&mut self) {
+        use computed_values::text_combine_upright::T as text_combine_upright;
+        use computed_values::writing_mode::T as writing_mode;
+
+        let writing_mode =
+            self.style.get_inheritedbox().clone_writing_mode();
+        let text_combine_upright =
+            self.style.get_inheritedtext().clone_text_combine_upright();
+
+        if writing_mode != writing_mode::horizontal_tb &&
+           text_combine_upright == text_combine_upright::all {
+            self.style.mutate_inheritedbox().set_writing_mode(writing_mode::horizontal_tb);
         }
     }
 
@@ -248,15 +259,14 @@ impl<'a> StyleAdjuster<'a> {
         }
     }
 
-    /// Adjusts the style to account for various fixups that don't fit naturally into the cascade.
+    /// Adjusts the style to account for various fixups that don't fit naturally
-    /// When comparing to Gecko, this is similar to the work done by `nsStyleContext::ApplyStyleFixups`.
+    /// into the cascade.
+    ///
+    /// When comparing to Gecko, this is similar to the work done by
+    /// `nsStyleContext::ApplyStyleFixups`.
     pub fn adjust(mut self,
                   layout_parent_style: &ComputedValues,
                   skip_root_and_element_display_fixup: bool) {
-        #[cfg(feature = "gecko")]
-        {
-            self.adjust_for_text_combine_upright(layout_parent_style);
-        }
         self.adjust_for_top_layer();
         self.blockify_if_necessary(layout_parent_style,
                                    skip_root_and_element_display_fixup);

components/style/stylist.rs

@@ -418,7 +418,6 @@ impl Stylist {
         let computed =
             properties::cascade(&self.device,
                                 &rule_node,
-                                Some(pseudo),
                                 guards,
                                 parent.map(|p| &**p),
                                 parent.map(|p| &**p),
@@ -543,7 +542,6 @@ impl Stylist {
         let computed =
             properties::cascade(&self.device,
                                 &rule_node,
-                                Some(pseudo),
                                 guards,
                                 Some(&**parent),
                                 Some(&**parent),
@@ -889,7 +887,6 @@ impl Stylist {
         let metrics = get_metrics_provider_for_product();
         Arc::new(properties::cascade(&self.device,
                                      &rule_node,
-                                     None,
                                      guards,
                                      Some(parent_style),
                                      Some(parent_style),

ports/geckolib/glue.rs

@@ -83,6 +83,7 @@ use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
+use style::style_adjuster::StyleAdjuster;
 use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers};
 use style::stylesheets::{ImportRule, MediaRule, NamespaceRule, Origin};
 use style::stylesheets::{PageRule, Stylesheet, StyleRule, SupportsRule};
@@ -467,7 +468,7 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawSe
     };
 
     let provider = get_metrics_provider_for_product();
-    element.get_base_style(shared_context, &provider, &styles.primary, pseudo.as_ref(), pseudo_style)
+    element.get_base_style(shared_context, &provider, &styles.primary, pseudo_style)
            .into_strong()
 }
 
@@ -992,15 +993,28 @@ fn get_pseudo_style(guard: &SharedRwLockReadGuard,
 #[no_mangle]
 pub extern "C" fn Servo_ComputedValues_Inherit(
   raw_data: RawServoStyleSetBorrowed,
-  parent_style: ServoComputedValuesBorrowedOrNull)
+  parent_style: ServoComputedValuesBorrowedOrNull,
+  target: structs::InheritTarget)
      -> ServoComputedValuesStrong {
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
     let maybe_arc = ComputedValues::arc_from_borrowed(&parent_style);
+
+    let for_text = target == structs::InheritTarget::Text;
     let style = if let Some(reference) = maybe_arc.as_ref() {
-        ComputedValues::inherit_from(reference, &data.default_computed_values())
+        let mut style =
+            ComputedValues::inherit_from(reference,
+                                         &data.default_computed_values());
+        if for_text {
+            StyleAdjuster::new(&mut style, /* is_root = */ false)
+                .adjust_for_text();
+        }
+
+        Arc::new(style)
     } else {
+        debug_assert!(!for_text);
         data.default_computed_values().clone()
     };
+
     style.into_strong()
 }
 
@@ -1037,8 +1051,11 @@ pub extern "C" fn Servo_ParseProperty(property: nsCSSPropertyID, value: *const n
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&data) };
     let reporter = RustLogReporter;
-    let context = ParserContext::new(Origin::Author, url_data, &reporter,
+    let context = ParserContext::new(Origin::Author,
-                                     Some(CssRuleType::Style), LengthParsingMode::Default,
+                                     url_data,
+                                     &reporter,
+                                     Some(CssRuleType::Style),
+                                     LengthParsingMode::Default,
                                      QuirksMode::NoQuirks);
 
     match ParsedDeclaration::parse(id, &context, &mut Parser::new(value)) {
@@ -1061,8 +1078,11 @@ pub extern "C" fn Servo_ParseEasing(easing: *const nsAString,
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&data) };
     let reporter = RustLogReporter;
-    let context = ParserContext::new(Origin::Author, url_data, &reporter,
+    let context = ParserContext::new(Origin::Author,
-                                     Some(CssRuleType::Style), LengthParsingMode::Default,
+                                     url_data,
+                                     &reporter,
+                                     Some(CssRuleType::Style),
+                                     LengthParsingMode::Default,
                                      QuirksMode::NoQuirks);
     let easing = unsafe { (*easing).to_string() };
     match transition_timing_function::single_value::parse(&context, &mut Parser::new(&easing)) {