Bug 1474959 part 3 - Pass writing mode into visited-only cascading. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Thu, 12 Jul 2018 21:42:06 +1000
changeset 817333 589ef910cbc71ebd1ed83c40bde81e0fc4c2add5
parent 817332 0832a4a9fb22be2083acf1f6729434ab6e030b0f
child 817334 4a75f87f670f3a5cf558346e7134bc91e317aa2a
push id116022
push userxquan@mozilla.com
push dateThu, 12 Jul 2018 12:09:47 +0000
reviewersemilio
bugs1474959
milestone63.0a1
Bug 1474959 part 3 - Pass writing mode into visited-only cascading. r?emilio MozReview-Commit-ID: KXPT7F6l6TC
servo/components/style/animation.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/stylist.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -502,16 +502,17 @@ where
                 &context.guards,
                 iter,
                 Some(previous_style),
                 Some(previous_style),
                 Some(previous_style),
                 font_metrics_provider,
                 CascadeFlags::empty(),
                 context.quirks_mode(),
+                /* writing_mode = */ None,
                 /* rule_cache = */ None,
                 &mut Default::default(),
                 /* element = */ None,
             );
             computed.shareable()
         },
     }
 }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -3613,16 +3613,17 @@ pub fn cascade<E>(
     rule_node: &StrongRuleNode,
     guards: &StylesheetGuards,
     parent_style: Option<<&ComputedValues>,
     parent_style_ignoring_first_line: Option<<&ComputedValues>,
     layout_parent_style: Option<<&ComputedValues>,
     font_metrics_provider: &FontMetricsProvider,
     flags: CascadeFlags,
     quirks_mode: QuirksMode,
+    writing_mode: Option<WritingMode>,
     rule_cache: Option<<&RuleCache>,
     rule_cache_conditions: &mut RuleCacheConditions,
     element: Option<E>,
 ) -> UniqueArc<ComputedValues>
 where
     E: TElement,
 {
     debug_assert_eq!(parent_style.is_some(), parent_style_ignoring_first_line.is_some());
@@ -3675,16 +3676,17 @@ where
         guards,
         iter_declarations,
         parent_style,
         parent_style_ignoring_first_line,
         layout_parent_style,
         font_metrics_provider,
         flags,
         quirks_mode,
+        writing_mode,
         rule_cache,
         rule_cache_conditions,
         element,
     )
 }
 
 /// NOTE: This function expects the declaration with more priority to appear
 /// first.
@@ -3695,16 +3697,17 @@ pub fn apply_declarations<'a, E, F, I>(
     guards: &StylesheetGuards,
     iter_declarations: F,
     parent_style: Option<<&ComputedValues>,
     parent_style_ignoring_first_line: Option<<&ComputedValues>,
     layout_parent_style: Option<<&ComputedValues>,
     font_metrics_provider: &FontMetricsProvider,
     flags: CascadeFlags,
     quirks_mode: QuirksMode,
+    writing_mode: Option<WritingMode>,
     rule_cache: Option<<&RuleCache>,
     rule_cache_conditions: &mut RuleCacheConditions,
     element: Option<E>,
 ) -> UniqueArc<ComputedValues>
 where
     E: TElement,
     F: Fn() -> I,
     I: Iterator<Item = (&'a PropertyDeclaration, CascadeLevel)>,
@@ -3760,16 +3763,20 @@ where
         for_non_inherited_property: None,
         font_metrics_provider,
         quirks_mode,
         rule_cache_conditions: RefCell::new(rule_cache_conditions),
     };
 
     let ignore_colors = !device.use_document_colors();
 
+    let for_visited = flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY);
+    debug_assert_eq!(for_visited, writing_mode.is_some(),
+                     "writing mode should be provided iff it's visited-only cascading");
+
     // Set computed values, overwriting earlier declarations for the same
     // property.
     let mut seen = LonghandIdSet::new();
 
     // Declaration blocks are stored in increasing precedence order, we want
     // them in decreasing order here.
     //
     // We could (and used to) use a pattern match here, but that bloats this
@@ -3809,18 +3816,17 @@ where
             let physical_longhand_id = longhand_id ${maybe_to_physical};
             if seen.contains(physical_longhand_id) {
                 continue
             }
 
             // Only a few properties are allowed to depend on the visited state
             // of links.  When cascading visited styles, we can save time by
             // only processing these properties.
-            if flags.contains(CascadeFlags::VISITED_DEPENDENT_ONLY) &&
-               !physical_longhand_id.is_visited_dependent() {
+            if for_visited && !physical_longhand_id.is_visited_dependent() {
                 continue
             }
 
             let mut declaration = match *declaration {
                 PropertyDeclaration::WithVariables(ref declaration) => {
                     if !declaration.id.inherited() {
                         context.rule_cache_conditions.borrow_mut()
                             .set_uncacheable();
@@ -3874,18 +3880,21 @@ where
                     continue;
                 }
             % endif
 
             let discriminant = longhand_id as usize;
             (CASCADE_PROPERTY[discriminant])(&*declaration, &mut context);
         }
         % if category_to_cascade_now == "early":
-            let writing_mode =
-                WritingMode::new(context.builder.get_inherited_box());
+            let writing_mode = if for_visited {
+                writing_mode.unwrap()
+            } else {
+                WritingMode::new(context.builder.get_inherited_box())
+            };
             context.builder.writing_mode = writing_mode;
 
             let mut _skip_font_family = false;
 
             % if product == "gecko":
 
                 // <svg:text> is not affected by text zoom, and it uses a preshint to
                 // disable it. We fix up the struct when this happens by unzooming
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -10,16 +10,17 @@ use context::{CascadeInputs, QuirksMode}
 use dom::{TElement, TShadowRoot};
 use element_state::{DocumentState, ElementState};
 use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::{ServoStyleSetSizes, StyleRuleInclusion};
 use hashglobe::FailedAllocationError;
 use invalidation::element::invalidation_map::InvalidationMap;
 use invalidation::media_queries::{EffectiveMediaQueryResults, ToMediaListKey};
+use logical_geometry::WritingMode;
 #[cfg(feature = "gecko")]
 use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
 #[cfg(feature = "gecko")]
 use malloc_size_of::MallocUnconditionalShallowSizeOf;
 use media_queries::Device;
 use properties::{self, CascadeFlags, ComputedValues};
 use properties::{AnimationRules, PropertyDeclarationBlock};
 use rule_cache::{RuleCache, RuleCacheConditions};
@@ -848,27 +849,28 @@ impl Stylist {
         let cascade_flags = pseudo.map_or(CascadeFlags::empty(), |p| p.cascade_flags());
 
         // Read the comment on `precomputed_values_for_pseudo` to see why it's
         // difficult to assert that display: contents nodes never arrive here
         // (tl;dr: It doesn't apply for replaced elements and such, but the
         // computed value is still "contents").
         //
         // FIXME(emilio): We should assert that it holds if pseudo.is_none()!
-        let mut values = properties::cascade::<E>(
+        let mut style = properties::cascade::<E>(
             &self.device,
             pseudo,
             inputs.rules.as_ref().unwrap_or(self.rule_tree.root()),
             guards,
             parent_style,
             parent_style_ignoring_first_line,
             layout_parent_style,
             font_metrics,
             cascade_flags,
             self.quirks_mode,
+            /* writing_mode = */ None,
             rule_cache,
             rule_cache_conditions,
             element,
         );
 
         // We need to compute visited values if we have visited rules or if our
         // parent has visited values.
         if inputs.visited_rules.is_some() || parent_style.and_then(|s| s.visited_style()).is_some()
@@ -893,35 +895,37 @@ impl Stylist {
                 inherited_style = parent_style
                     .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style));
                 inherited_style_ignoring_first_line = parent_style_ignoring_first_line
                     .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style));
                 layout_parent_style_for_visited = layout_parent_style
                     .map(|parent_style| parent_style.visited_style().unwrap_or(parent_style));
             }
 
+            let writing_mode = WritingMode::new(style.get_inherited_box());
             let visited_style = properties::cascade::<E>(
                 &self.device,
                 pseudo,
                 rule_node,
                 guards,
                 inherited_style,
                 inherited_style_ignoring_first_line,
                 layout_parent_style_for_visited,
                 font_metrics,
                 cascade_flags | CascadeFlags::VISITED_DEPENDENT_ONLY,
                 self.quirks_mode,
+                Some(writing_mode),
                 rule_cache,
                 rule_cache_conditions,
                 element,
             ).shareable();
-            values.set_visited_style(parent_style, visited_style, element);
+            style.set_visited_style(parent_style, visited_style, element);
         }
 
-        values.shareable()
+        style.shareable()
     }
 
     /// Computes the cascade inputs for a lazily-cascaded pseudo-element.
     ///
     /// See the documentation on lazy pseudo-elements in
     /// docs/components/style.md
     fn lazy_pseudo_rules<E>(
         &self,
@@ -1582,16 +1586,17 @@ impl Stylist {
             guards,
             iter_declarations,
             Some(parent_style),
             Some(parent_style),
             Some(parent_style),
             &metrics,
             CascadeFlags::empty(),
             self.quirks_mode,
+            /* writing_mode = */ None,
             /* rule_cache = */ None,
             &mut Default::default(),
             /* element = */ None,
         ).shareable()
     }
 
     /// Accessor for a shared reference to the device.
     #[inline]