Bug 1917703 - Make sure visited style computation color-scheme matches unvisited style. r=dshin

Plumb it via the StyleBuilder like we do for the writing_mode for the
same reasons.

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

layout/style/GeckoBindings.cpp

@@ -703,14 +703,13 @@ bool Gecko_IsDocumentBody(const Element* aElement) {
 }
 
 bool Gecko_IsDarkColorScheme(const Document* aDoc,
-                             const StyleColorScheme* aStyle) {
-  return LookAndFeel::ColorSchemeForStyle(*aDoc, aStyle->bits) ==
-         ColorScheme::Dark;
+                             const StyleColorSchemeFlags* aStyle) {
+  return LookAndFeel::ColorSchemeForStyle(*aDoc, *aStyle) == ColorScheme::Dark;
 }
 
 nscolor Gecko_ComputeSystemColor(StyleSystemColor aColor, const Document* aDoc,
-                                 const StyleColorScheme* aStyle) {
-  auto colorScheme = LookAndFeel::ColorSchemeForStyle(*aDoc, aStyle->bits);
+                                 const StyleColorSchemeFlags* aStyle) {
+  auto colorScheme = LookAndFeel::ColorSchemeForStyle(*aDoc, *aStyle);
   const auto& prefs = PreferenceSheet::PrefsFor(*aDoc);
   if (prefs.mMustUseLightSystemColors) {
     colorScheme = ColorScheme::Light;

layout/style/GeckoBindings.h

@@ -492,10 +492,10 @@ GeckoImplicitScopeRoot Gecko_StyleSheet_ImplicitScopeRoot(
 bool Gecko_IsDocumentBody(const mozilla::dom::Element* element);
 
 bool Gecko_IsDarkColorScheme(const mozilla::dom::Document*,
-                             const mozilla::StyleColorScheme*);
+                             const mozilla::StyleColorSchemeFlags*);
 nscolor Gecko_ComputeSystemColor(mozilla::StyleSystemColor,
                                  const mozilla::dom::Document*,
-                                 const mozilla::StyleColorScheme*);
+                                 const mozilla::StyleColorSchemeFlags*);
 
 // We use an int32_t here instead of a LookAndFeel::IntID/FloatID because
 // forward-declaring a nested enum/struct is impossible.

layout/style/ServoBindings.toml

@@ -488,6 +488,7 @@ cbindgen-types = [
     { gecko = "StyleTouchAction", servo = "crate::values::computed::TouchAction" },
     { gecko = "StyleWillChange", servo = "crate::values::specified::box_::WillChange" },
     { gecko = "StyleColorScheme", servo = "crate::values::specified::color::ColorScheme" },
+    { gecko = "StyleColorSchemeFlags", servo = "crate::values::specified::color::ColorSchemeFlags" },
     { gecko = "StyleTextDecorationLine", servo = "crate::values::computed::TextDecorationLine" },
     { gecko = "StyleMasonryAutoFlow", servo = "crate::values::specified::MasonryAutoFlow" },
     { gecko = "StyleMasonryPlacement", servo = "crate::values::specified::MasonryPlacement" },

servo/components/style/gecko/media_queries.rs

@@ -17,7 +17,7 @@ use crate::properties::ComputedValues;
 use crate::string_cache::Atom;
 use crate::values::computed::font::GenericFontFamily;
 use crate::values::computed::{ColorScheme, Length, NonNegativeLength};
-use crate::values::specified::color::SystemColor;
+use crate::values::specified::color::{SystemColor, ColorSchemeFlags};
 use crate::values::specified::font::{FONT_MEDIUM_LINE_HEIGHT_PX, FONT_MEDIUM_PX};
 use crate::values::specified::ViewportVariant;
 use crate::values::{CustomIdent, KeyframesName};
@@ -507,14 +507,14 @@ impl Device {
     pub(crate) fn system_nscolor(
         &self,
         system_color: SystemColor,
-        color_scheme: &ColorScheme,
+        color_scheme: ColorSchemeFlags,
     ) -> u32 {
-        unsafe { bindings::Gecko_ComputeSystemColor(system_color, self.document(), color_scheme) }
+        unsafe { bindings::Gecko_ComputeSystemColor(system_color, self.document(), &color_scheme) }
     }
 
     /// Returns whether the used color-scheme for `color-scheme` should be dark.
-    pub(crate) fn is_dark_color_scheme(&self, color_scheme: &ColorScheme) -> bool {
-        unsafe { bindings::Gecko_IsDarkColorScheme(self.document(), color_scheme) }
+    pub(crate) fn is_dark_color_scheme(&self, color_scheme: ColorSchemeFlags) -> bool {
+        unsafe { bindings::Gecko_IsDarkColorScheme(self.document(), &color_scheme) }
     }
 
     /// Returns the default background color.
@@ -522,16 +522,14 @@ impl Device {
     /// This is only for forced-colors/high-contrast, so looking at light colors
     /// is ok.
     pub fn default_background_color(&self) -> AbsoluteColor {
-        let normal = ColorScheme::normal();
-        convert_nscolor_to_absolute_color(self.system_nscolor(SystemColor::Canvas, &normal))
+        convert_nscolor_to_absolute_color(self.system_nscolor(SystemColor::Canvas, ColorScheme::normal().bits))
     }
 
     /// Returns the default foreground color.
     ///
     /// See above for looking at light colors only.
     pub fn default_color(&self) -> AbsoluteColor {
-        let normal = ColorScheme::normal();
-        convert_nscolor_to_absolute_color(self.system_nscolor(SystemColor::Canvastext, &normal))
+        convert_nscolor_to_absolute_color(self.system_nscolor(SystemColor::Canvastext, ColorScheme::normal().bits))
     }
 
     /// Returns the current effective text zoom.

servo/components/style/properties/cascade.rs

@@ -306,6 +306,7 @@ where
         CascadeMode::Visited { unvisited_context } => {
             context.builder.custom_properties = unvisited_context.builder.custom_properties.clone();
             context.builder.writing_mode = unvisited_context.builder.writing_mode;
+            context.builder.color_scheme = unvisited_context.builder.color_scheme;
             // 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 :visited properties are
             // non-inherited.
@@ -762,11 +763,14 @@ impl<'b> Cascade<'b> {
 
         let has_writing_mode = apply!(WritingMode) | apply!(Direction) | apply!(TextOrientation);
         if has_writing_mode {
-            self.compute_writing_mode(context);
+            context.builder.writing_mode = WritingMode::new(context.builder.get_inherited_box())
         }
 
         if apply!(Zoom) {
-            self.compute_zoom(context);
+            context.builder.effective_zoom = context
+                .builder
+                .inherited_effective_zoom()
+                .compute_effective(context.builder.specified_zoom());
             // NOTE(emilio): This is a bit of a hack, but matches the shipped WebKit and Blink
             // behavior for now. Ideally, in the future, we have a pass over all
             // implicitly-or-explicitly-inherited properties that can contain lengths and
@@ -813,7 +817,9 @@ impl<'b> Cascade<'b> {
         apply!(ForcedColorAdjust);
         // color-scheme needs to be after forced-color-adjust, since it's one of the "skipped in
         // forced-colors-mode" properties.
-        apply!(ColorScheme);
+        if apply!(ColorScheme) {
+            context.builder.color_scheme = context.builder.get_inherited_ui().color_scheme_bits();
+        }
         apply!(LineHeight);
     }
 
@@ -944,17 +950,6 @@ impl<'b> Cascade<'b> {
         (CASCADE_PROPERTY[longhand_id as usize])(&declaration, context);
     }
 
-    fn compute_zoom(&self, context: &mut computed::Context) {
-        context.builder.effective_zoom = context
-            .builder
-            .inherited_effective_zoom()
-            .compute_effective(context.builder.specified_zoom());
-    }
-
-    fn compute_writing_mode(&self, context: &mut computed::Context) {
-        context.builder.writing_mode = WritingMode::new(context.builder.get_inherited_box())
-    }
-
     fn compute_visited_style_if_needed<E>(
         &self,
         context: &mut computed::Context,

servo/components/style/properties/gecko.mako.rs

@@ -1286,6 +1286,10 @@ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask-
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="InheritedUI">
+    #[inline]
+    pub fn color_scheme_bits(&self) -> values::specified::color::ColorSchemeFlags {
+        self.mColorScheme.bits
+    }
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Column"

servo/components/style/properties/properties.mako.rs

@@ -37,7 +37,7 @@ use crate::str::CssStringWriter;
 use crate::values::{
     computed,
     resolved,
-    specified::{font::SystemFont, length::LineHeightBase},
+    specified::{font::SystemFont, length::LineHeightBase, color::ColorSchemeFlags},
 };
 use std::cell::Cell;
 use super::{
@@ -2357,6 +2357,10 @@ pub struct StyleBuilder<'a> {
     /// TODO(emilio): Make private.
     pub writing_mode: WritingMode,
 
+    /// The color-scheme bits. Needed because they may otherwise be different between visited and
+    /// unvisited colors.
+    pub color_scheme: ColorSchemeFlags,
+
     /// The effective zoom.
     pub effective_zoom: computed::Zoom,
 
@@ -2386,7 +2390,7 @@ impl<'a> StyleBuilder<'a> {
         let inherited_style = parent_style.unwrap_or(reset_style);
 
         let flags = inherited_style.flags.inherited();
-        StyleBuilder {
+        Self {
             device,
             stylist,
             inherited_style,
@@ -2399,6 +2403,7 @@ impl<'a> StyleBuilder<'a> {
             invalid_non_custom_properties: LonghandIdSet::default(),
             writing_mode: inherited_style.writing_mode,
             effective_zoom: inherited_style.effective_zoom,
+            color_scheme: inherited_style.get_inherited_ui().color_scheme_bits(),
             flags: Cell::new(flags),
             visited_style: None,
             % for style_struct in data.active_style_structs():
@@ -2425,7 +2430,7 @@ impl<'a> StyleBuilder<'a> {
     ) -> Self {
         let reset_style = device.default_computed_values();
         let inherited_style = parent_style.unwrap_or(reset_style);
-        StyleBuilder {
+        Self {
             device,
             stylist,
             inherited_style,
@@ -2438,6 +2443,7 @@ impl<'a> StyleBuilder<'a> {
             invalid_non_custom_properties: LonghandIdSet::default(),
             writing_mode: style_to_derive_from.writing_mode,
             effective_zoom: style_to_derive_from.effective_zoom,
+            color_scheme: style_to_derive_from.get_inherited_ui().color_scheme_bits(),
             flags: Cell::new(style_to_derive_from.flags),
             visited_style: None,
             % for style_struct in data.active_style_structs():

servo/components/style/rule_cache.rs

@@ -95,7 +95,7 @@ impl CachedConditions {
         }
 
         if let Some(cs) = self.color_scheme {
-            if style.get_inherited_ui().clone_color_scheme().bits != cs {
+            if style.get_inherited_ui().color_scheme_bits() != cs {
                 return false;
             }
         }

servo/components/style/values/specified/color.rs

@@ -139,10 +139,9 @@ pub struct LightDark {
 
 impl LightDark {
     fn compute(&self, cx: &Context) -> ComputedColor {
-        let style_color_scheme = cx.style().get_inherited_ui().clone_color_scheme();
-        let dark = cx.device().is_dark_color_scheme(&style_color_scheme);
+        let dark = cx.device().is_dark_color_scheme(cx.builder.color_scheme);
         if cx.for_non_inherited_property {
-            cx.rule_cache_conditions.borrow_mut().set_color_scheme_dependency(style_color_scheme.bits);
+            cx.rule_cache_conditions.borrow_mut().set_color_scheme_dependency(cx.builder.color_scheme);
         }
         let used = if dark { &self.dark } else { &self.light };
         used.to_computed_value(cx)
@@ -421,11 +420,9 @@ impl SystemColor {
         use crate::gecko::values::convert_nscolor_to_absolute_color;
         use crate::gecko_bindings::bindings;
 
-        // TODO: We should avoid cloning here most likely, though it's cheap-ish.
-        let style_color_scheme = cx.style().get_inherited_ui().clone_color_scheme();
-        let color = cx.device().system_nscolor(*self, &style_color_scheme);
+        let color = cx.device().system_nscolor(*self, cx.builder.color_scheme);
         if cx.for_non_inherited_property {
-            cx.rule_cache_conditions.borrow_mut().set_color_scheme_dependency(style_color_scheme.bits);
+            cx.rule_cache_conditions.borrow_mut().set_color_scheme_dependency(cx.builder.color_scheme);
         }
         if color == bindings::NS_SAME_AS_FOREGROUND_COLOR {
             return ComputedColor::currentcolor();

testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/color-scheme-link-crash.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1917703">
+<script>
+document.addEventListener("DOMContentLoaded", () => {
+  const a = document.createElement("a");
+  a.setAttribute("style", "color-scheme: dark; background-color: MenuText")
+  a.setAttribute("href", "x")
+  document.documentElement.appendChild(a)
+})
+</script>