style: Remove dependency on servo_config (was #31409) (#31411)

* Initial style_config crate

* Remove servo_config from style

* Remove servo_config from tests/unit/style

* Plumb servo prefs into stylo

* Clean up dependencies

* Fix formatting

* Add unit tests

* Add comment about avoiding clone

* Fix bug where getters acquire unnecessary write lock

* Remove stray dbg!()

* Plumb default prefs into Stylo as well

* Add comments about logging and mapping new pref types

Cargo.lock

@@ -5396,6 +5396,7 @@ dependencies = [
  "servo_config_plugins",
  "servo_geometry",
  "servo_url",
+ "style_config",
  "url",
 ]
 
@@ -5781,12 +5782,12 @@ dependencies = [
  "serde",
  "servo_arc",
  "servo_atoms",
- "servo_config",
  "smallbitvec",
  "smallvec",
  "static_assertions",
  "static_prefs",
  "string_cache",
+ "style_config",
  "style_derive",
  "style_traits",
  "thin-vec",
@@ -5802,6 +5803,13 @@ dependencies = [
  "walkdir",
 ]
 
+[[package]]
+name = "style_config"
+version = "0.0.1"
+dependencies = [
+ "lazy_static",
+]
+
 [[package]]
 name = "style_derive"
 version = "0.0.1"
@@ -5827,7 +5835,6 @@ dependencies = [
  "serde_json",
  "servo_arc",
  "servo_atoms",
- "servo_config",
  "style",
  "style_traits",
  "url",

Cargo.toml

@@ -95,6 +95,7 @@ smallvec = "1.13"
 sparkle = "0.1.26"
 string_cache = "0.8"
 string_cache_codegen = "0.5"
+style_config = { path = "components/style_config" }
 style_traits = { path = "components/style_traits", features = ["servo"] }
 # NOTE: the sm-angle feature only enables ANGLE on Windows, not other platforms!
 surfman = { version = "0.9", features = ["chains", "sm-angle", "sm-angle-default"] }

components/config/Cargo.toml

@@ -22,6 +22,7 @@ serde_json = { workspace = true }
 servo_config_plugins = { path = "../config_plugins" }
 servo_geometry = { path = "../geometry" }
 servo_url = { path = "../url" }
+style_config = { path = "../style_config" }
 url = { workspace = true }
 
 [target.'cfg(not(target_os = "android"))'.dependencies]

components/config/prefs.rs

@@ -4,10 +4,12 @@
 
 use std::borrow::ToOwned;
 use std::collections::HashMap;
+use std::convert::{TryFrom, TryInto};
 
 use embedder_traits::resources::{self, Resource};
 use gen::Prefs;
 use lazy_static::lazy_static;
+use log::warn;
 use serde_json::{self, Value};
 
 use crate::pref_util::Preferences;
@@ -17,7 +19,11 @@ lazy_static! {
     static ref PREFS: Preferences<'static, Prefs> = {
         let def_prefs: Prefs = serde_json::from_str(&resources::read_string(Resource::Preferences))
             .expect("Failed to initialize config preferences.");
-        Preferences::new(def_prefs, &gen::PREF_ACCESSORS)
+        let result = Preferences::new(def_prefs, &gen::PREF_ACCESSORS);
+        for (key, value) in result.iter() {
+            set_stylo_pref_ref(&key, &value);
+        }
+        result
     };
 }
 
@@ -38,9 +44,11 @@ macro_rules! pref {
 #[macro_export]
 macro_rules! set_pref {
     ($($segment: ident).+, $value: expr) => {{
+        let value = $value;
+        $crate::prefs::set_stylo_pref(stringify!($($segment).+), value);
         let values = $crate::prefs::pref_map().values();
         let mut lock = values.write().unwrap();
-        lock$ (.$segment)+ = $value;
+        lock$ (.$segment)+ = value;
     }};
 }
 
@@ -56,11 +64,62 @@ pub fn pref_map() -> &'static Preferences<'static, Prefs> {
 }
 
 pub fn add_user_prefs(prefs: HashMap<String, PrefValue>) {
+    for (key, value) in prefs.iter() {
+        set_stylo_pref_ref(key, value);
+    }
     if let Err(error) = PREFS.set_all(prefs.into_iter()) {
         panic!("Error setting preference: {:?}", error);
     }
 }
 
+pub fn set_stylo_pref(key: &str, value: impl Into<PrefValue>) {
+    set_stylo_pref_ref(key, &value.into());
+}
+
+fn set_stylo_pref_ref(key: &str, value: &PrefValue) {
+    match value.try_into() {
+        Ok(StyloPrefValue::Bool(value)) => style_config::set_bool(key, value),
+        Ok(StyloPrefValue::Int(value)) => style_config::set_i32(key, value),
+        Err(TryFromPrefValueError::IntegerOverflow(value)) => {
+            // TODO: logging doesn’t actually work this early, so we should
+            // split PrefValue into i32 and i64 variants.
+            warn!("Pref value too big for Stylo: {} ({})", key, value);
+        },
+        Err(TryFromPrefValueError::UnmappedType) => {
+            // Most of Servo’s prefs will hit this. When adding a new pref type
+            // in Stylo, update TryFrom<&PrefValue> for StyloPrefValue as well.
+        },
+    }
+}
+
+enum StyloPrefValue {
+    Bool(bool),
+    Int(i32),
+}
+
+enum TryFromPrefValueError {
+    IntegerOverflow(i64),
+    UnmappedType,
+}
+
+impl TryFrom<&PrefValue> for StyloPrefValue {
+    type Error = TryFromPrefValueError;
+
+    fn try_from(value: &PrefValue) -> Result<Self, Self::Error> {
+        match value {
+            &PrefValue::Int(value) => {
+                if let Ok(value) = value.try_into() {
+                    Ok(Self::Int(value))
+                } else {
+                    Err(TryFromPrefValueError::IntegerOverflow(value))
+                }
+            },
+            &PrefValue::Bool(value) => Ok(Self::Bool(value)),
+            _ => Err(TryFromPrefValueError::UnmappedType),
+        }
+    }
+}
+
 pub fn read_prefs_map(txt: &str) -> Result<HashMap<String, PrefValue>, PrefError> {
     let prefs: HashMap<String, Value> =
         serde_json::from_str(txt).map_err(|e| PrefError::JsonParseErr(e))?;

components/style/Cargo.toml

@@ -22,7 +22,6 @@ servo = [
     "serde",
     "style_traits/servo",
     "servo_atoms",
-    "servo_config",
     "html5ever",
     "cssparser/serde",
     "encoding_rs",
@@ -68,12 +67,12 @@ selectors = { path = "../selectors" }
 serde = { version = "1.0", optional = true, features = ["derive"] }
 servo_arc = { path = "../servo_arc" }
 servo_atoms = { path = "../atoms", optional = true }
-servo_config = { path = "../config", optional = true }
 smallbitvec = "2.3.0"
 smallvec = "1.0"
 static_assertions = "1.1"
 static_prefs = { path = "../style_static_prefs" }
 string_cache = { version = "0.8", optional = true }
+style_config = { path = "../style_config" }
 style_derive = { path = "../style_derive" }
 style_traits = { path = "../style_traits" }
 time = "0.1"

components/style/global_style_data.rs

@@ -114,8 +114,7 @@ impl StyleThreadPool {
 
 #[cfg(feature = "servo")]
 fn stylo_threads_pref() -> i32 {
-    use servo_config::pref;
+    style_config::get_i32("layout.threads")
-    pref!(layout.threads) as i32
 }
 
 #[cfg(feature = "gecko")]

components/style/properties/properties.mako.rs

@@ -32,7 +32,6 @@ use fxhash::FxHashMap;
 use crate::media_queries::Device;
 use crate::parser::ParserContext;
 use crate::selector_parser::PseudoElement;
-#[cfg(feature = "servo")] use servo_config::prefs;
 use style_traits::{CssWriter, KeywordsCollectFn, ParseError, ParsingMode};
 use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss};
 use to_shmem::impl_trivial_to_shmem;
@@ -525,7 +524,7 @@ impl NonCustomPropertyId {
                     Some(pref) => pref,
                 };
 
-                prefs::pref_map().get(pref).as_bool().unwrap_or(false)
+                style_config::get_bool(pref)
             % endif
         };
 

components/style/values/specified/box.rs

@@ -27,10 +27,7 @@ fn flexbox_enabled() -> bool {
 
 #[cfg(feature = "servo")]
 fn flexbox_enabled() -> bool {
-    servo_config::prefs::pref_map()
+    style_config::get_bool("layout.flexbox.enabled")
-        .get("layout.flexbox.enabled")
-        .as_bool()
-        .unwrap_or(false)
 }
 
 /// Defines an element’s display type, which consists of

components/style_config/Cargo.toml

@@ -0,0 +1,14 @@
+[package]
+name = "style_config"
+version = "0.0.1"
+authors = ["The Servo Project Developers"]
+license = "MPL-2.0"
+edition = "2021"
+publish = false
+
+[lib]
+name = "style_config"
+path = "lib.rs"
+
+[dependencies]
+lazy_static = { workspace = true }

components/style_config/lib.rs

@@ -0,0 +1,97 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+
+use std::collections::HashMap;
+use std::sync::RwLock;
+
+use lazy_static::lazy_static;
+
+lazy_static! {
+    static ref PREFS: Preferences = Preferences::default();
+}
+
+#[derive(Debug, Default)]
+pub struct Preferences {
+    // When adding a new pref type, be sure to update the TryFrom<&PrefValue> in
+    // servo_config, to plumb the values in from Servo.
+    bool_prefs: RwLock<HashMap<String, bool>>,
+    i32_prefs: RwLock<HashMap<String, i32>>,
+}
+
+impl Preferences {
+    pub fn get_bool(&self, key: &str) -> bool {
+        let prefs = self.bool_prefs.read().expect("RwLock is poisoned");
+        *prefs.get(key).unwrap_or(&false)
+    }
+
+    pub fn get_i32(&self, key: &str) -> i32 {
+        let prefs = self.i32_prefs.read().expect("RwLock is poisoned");
+        *prefs.get(key).unwrap_or(&0)
+    }
+
+    pub fn set_bool(&self, key: &str, value: bool) {
+        let mut prefs = self.bool_prefs.write().expect("RwLock is poisoned");
+
+        // Avoid cloning the key if it exists.
+        if let Some(pref) = prefs.get_mut(key) {
+            *pref = value;
+        } else {
+            prefs.insert(key.to_owned(), value);
+        }
+    }
+
+    pub fn set_i32(&self, key: &str, value: i32) {
+        let mut prefs = self.i32_prefs.write().expect("RwLock is poisoned");
+
+        // Avoid cloning the key if it exists.
+        if let Some(pref) = prefs.get_mut(key) {
+            *pref = value;
+        } else {
+            prefs.insert(key.to_owned(), value);
+        }
+    }
+}
+
+pub fn get_bool(key: &str) -> bool {
+    PREFS.get_bool(key)
+}
+
+pub fn get_i32(key: &str) -> i32 {
+    PREFS.get_i32(key)
+}
+
+pub fn set_bool(key: &str, value: bool) {
+    PREFS.set_bool(key, value)
+}
+
+pub fn set_i32(key: &str, value: i32) {
+    PREFS.set_i32(key, value)
+}
+
+#[test]
+fn test() {
+    let mut prefs = Preferences::default();
+
+    // Prefs have default values when unset.
+    assert_eq!(prefs.get_bool("foo"), false);
+    assert_eq!(prefs.get_i32("bar"), 0);
+
+    // Prefs can be set and retrieved.
+    prefs.set_bool("foo", true);
+    prefs.set_i32("bar", 1);
+    assert_eq!(prefs.get_bool("foo"), true);
+    assert_eq!(prefs.get_i32("bar"), 1);
+    prefs.set_bool("foo", false);
+    prefs.set_i32("bar", 2);
+    assert_eq!(prefs.get_bool("foo"), false);
+    assert_eq!(prefs.get_i32("bar"), 2);
+
+    // Each value type currently has an independent namespace.
+    prefs.set_i32("foo", 3);
+    prefs.set_bool("bar", true);
+    assert_eq!(prefs.get_i32("foo"), 3);
+    assert_eq!(prefs.get_bool("foo"), false);
+    assert_eq!(prefs.get_bool("bar"), true);
+    assert_eq!(prefs.get_i32("bar"), 2);
+}

python/servo/testing_commands.py

@@ -141,6 +141,7 @@ class MachCommands(CommandBase):
             "servo_remutex",
             "crown",
             "constellation",
+            "style_config",
         ]
         if not packages:
             packages = set(os.listdir(path.join(self.context.topdir, "tests", "unit"))) - set(['.DS_Store'])

tests/unit/style/Cargo.toml

@@ -19,7 +19,6 @@ serde_json = { workspace = true }
 selectors = {path = "../../../components/selectors" }
 servo_arc = {path = "../../../components/servo_arc"}
 servo_atoms = {path = "../../../components/atoms"}
-servo_config = {path = "../../../components/config"}
 style = {path = "../../../components/style", features = ["servo"]}
 style_traits = {path = "../../../components/style_traits"}
 url = { workspace = true }