style: Fix visited handling after bug 1763750

Before bug 1763750, we unconditionally called compute_writing_mode,
which got the writing mode from the cascade mode for visited styles.

However after that bug we only do that if we apply any
writing-mode-related property.

We could just call compute_writing_mode unconditionally, but instead it
seems better to skip all that work for visited cascade and reuse the
mechanism introduced in that bug to only apply the visited-dependent
longhands.

We assert that all visited-dependent longhands are "late" longhands, so
as to also avoid applying the font group and such.

Differential Revision: https://phabricator.services.mozilla.com/D143490

components/style/properties/cascade.rs

@@ -296,9 +296,20 @@ where
         rule_cache_conditions: RefCell::new(rule_cache_conditions),
     };
 
-    let using_cached_reset_properties = {
+    let using_cached_reset_properties;
     let mut cascade = Cascade::new(&mut context, cascade_mode, &referenced_properties);
     let mut shorthand_cache = ShorthandsWithPropertyReferencesCache::default();
+
+    let properties_to_apply = match cascade.cascade_mode {
+        CascadeMode::Visited { writing_mode } => {
+            cascade.context.builder.writing_mode = writing_mode;
+            // We never insert visited styles into the cache so we don't need to
+            // try looking it up. It also wouldn't be super-profitable, only a
+            // handful reset properties are non-inherited.
+            using_cached_reset_properties = false;
+            LonghandIdSet::visited_dependent()
+        },
+        CascadeMode::Unvisited { visited_rules } => {
             if cascade.apply_properties(
                 CanHaveLogicalProperties::No,
                 LonghandIdSet::writing_mode_group(),
@@ -317,21 +328,26 @@ where
                 cascade.fixup_font_stuff();
             }
 
+            if let Some(visited_rules) = visited_rules {
                 cascade.compute_visited_style_if_needed(
                     element,
                     parent_style,
                     parent_style_ignoring_first_line,
                     layout_parent_style,
+                    visited_rules,
                     guards,
                 );
+            }
 
-        let using_cached_reset_properties =
+            using_cached_reset_properties =
                 cascade.try_to_use_cached_reset_properties(rule_cache, guards);
 
-        let properties_to_apply = if using_cached_reset_properties {
+            if using_cached_reset_properties {
                 LonghandIdSet::late_group_only_inherited()
             } else {
                 LonghandIdSet::late_group()
+            }
+        }
     };
 
     cascade.apply_properties(
@@ -343,9 +359,6 @@ where
 
     cascade.finished_applying_properties();
 
-        using_cached_reset_properties
-    };
-
     context.builder.clear_modified_reset();
 
     if matches!(cascade_mode, CascadeMode::Unvisited { .. }) {
@@ -627,15 +640,6 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 }
             }
 
-            // 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 matches!(self.cascade_mode, CascadeMode::Visited { .. }) &&
-                !physical_longhand_id.is_visited_dependent()
-            {
-                continue;
-            }
-
             let mut declaration =
                 self.substitute_variables_if_needed(declaration, &mut shorthand_cache);
 
@@ -708,13 +712,9 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
     }
 
     fn compute_writing_mode(&mut self) {
-        let writing_mode = match self.cascade_mode {
+        debug_assert!(matches!(self.cascade_mode, CascadeMode::Unvisited { .. }));
-            CascadeMode::Unvisited { .. } => {
+        self.context.builder.writing_mode =
             WritingMode::new(self.context.builder.get_inherited_box())
-            },
-            CascadeMode::Visited { writing_mode } => writing_mode,
-        };
-        self.context.builder.writing_mode = writing_mode;
     }
 
     fn compute_visited_style_if_needed<E>(
@@ -723,20 +723,12 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
         parent_style: Option<&ComputedValues>,
         parent_style_ignoring_first_line: Option<&ComputedValues>,
         layout_parent_style: Option<&ComputedValues>,
+        visited_rules: &StrongRuleNode,
         guards: &StylesheetGuards,
     ) where
         E: TElement,
     {
-        let visited_rules = match self.cascade_mode {
+        debug_assert!(matches!(self.cascade_mode, CascadeMode::Unvisited { .. }));
-            CascadeMode::Unvisited { visited_rules } => visited_rules,
-            CascadeMode::Visited { .. } => return,
-        };
-
-        let visited_rules = match visited_rules {
-            Some(rules) => rules,
-            None => return,
-        };
-
         let is_link = self.context.builder.pseudo.is_none() && element.unwrap().is_link();
 
         macro_rules! visited_parent {

components/style/properties/properties.mako.rs

@@ -934,6 +934,29 @@ CASCADE_GROUPS = {
 def in_late_group(p):
     return p.name not in CASCADE_GROUPS["writing_mode"] and p.name not in CASCADE_GROUPS["fonts_and_color"]
 
+def is_visited_dependent(p):
+    return p.name in [
+        "column-rule-color",
+        "text-emphasis-color",
+        "-webkit-text-fill-color",
+        "-webkit-text-stroke-color",
+        "text-decoration-color",
+        "fill",
+        "stroke",
+        "caret-color",
+        "background-color",
+        "border-top-color",
+        "border-right-color",
+        "border-bottom-color",
+        "border-left-color",
+        "border-block-start-color",
+        "border-inline-end-color",
+        "border-block-end-color",
+        "border-inline-start-color",
+        "outline-color",
+        "color",
+    ]
+
 %>
 
 impl LonghandIdSet {
@@ -972,6 +995,18 @@ impl LonghandIdSet {
         &IGNORED_WHEN_COLORS_DISABLED
     }
 
+    /// 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.
+    fn visited_dependent() -> &'static Self {
+        ${static_longhand_id_set(
+            "VISITED_DEPENDENT",
+            lambda p: is_visited_dependent(p)
+        )}
+        debug_assert!(Self::late_group().contains_all(&VISITED_DEPENDENT));
+        &VISITED_DEPENDENT
+    }
+
     #[inline]
     fn writing_mode_group() -> &'static Self {
         ${static_longhand_id_set(
@@ -1404,31 +1439,6 @@ impl LonghandId {
         PropertyFlags::from_bits_truncate(FLAGS[self as usize])
     }
 
-    /// 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.
-    fn is_visited_dependent(&self) -> bool {
-        matches!(*self,
-            % if engine == "gecko":
-            LonghandId::ColumnRuleColor |
-            LonghandId::TextEmphasisColor |
-            LonghandId::WebkitTextFillColor |
-            LonghandId::WebkitTextStrokeColor |
-            LonghandId::TextDecorationColor |
-            LonghandId::Fill |
-            LonghandId::Stroke |
-            LonghandId::CaretColor |
-            % endif
-            LonghandId::BackgroundColor |
-            LonghandId::BorderTopColor |
-            LonghandId::BorderRightColor |
-            LonghandId::BorderBottomColor |
-            LonghandId::BorderLeftColor |
-            LonghandId::OutlineColor |
-            LonghandId::Color
-        )
-    }
-
     /// Returns true if the property is one that is ignored when document
     /// colors are disabled.
     #[inline]