#8539 Config preferences backend restructure

components/config/Cargo.toml

@@ -9,8 +9,6 @@ publish = false
 [lib]
 name = "servo_config"
 path = "lib.rs"
-test = false
-doctest = false
 
 [dependencies]
 euclid = "0.19"
@@ -20,10 +18,12 @@ lazy_static = "1"
 log = "0.4"
 num_cpus = "1.1.0"
 serde = "1.0"
+serde_derive = "1.0"
 serde_json = "1.0"
 servo_geometry = {path = "../geometry"}
 servo_url = {path = "../url"}
 url = "1.2"
+servo_config_plugins = { path = "../config_plugins" }
 
 [dev-dependencies]
 env_logger = "0.6"

components/config/lib.rs

@@ -2,6 +2,7 @@
  * 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/. */
 
+#![feature(core_intrinsics)]
 #![deny(unsafe_code)]
 
 #[macro_use]
@@ -11,10 +12,13 @@ extern crate log;
 #[macro_use]
 extern crate serde;
 
+pub mod pref_util;
+#[macro_use]
+pub mod prefs;
+
 pub mod basedir;
 #[allow(unsafe_code)]
 pub mod opts;
-pub mod prefs;
 
 pub fn servo_version() -> String {
     let cargo_version = env!("CARGO_PKG_VERSION");

components/config/opts.rs

@@ -5,13 +5,12 @@
 //! Configuration options for a single run of the servo application. Created
 //! from command line arguments.
 
-use crate::prefs::{self, PrefValue, PREFS};
+use crate::prefs::{self, PrefValue};
 use euclid::TypedSize2D;
 use getopts::Options;
 use servo_geometry::DeviceIndependentPixel;
 use servo_url::ServoUrl;
 use std::borrow::Cow;
-use std::cmp;
 use std::default::Default;
 use std::env;
 use std::fs::{self, File};
@@ -975,17 +974,11 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
         })
         .collect();
 
-    let do_not_use_native_titlebar = opt_match.opt_present("b") ||
-        !PREFS
-            .get("shell.native-titlebar.enabled")
-            .as_boolean()
-            .unwrap();
+    let do_not_use_native_titlebar =
+        opt_match.opt_present("b") || !(pref!(shell.native_titlebar.enabled));
 
-    let enable_subpixel_text_antialiasing = !debug_options.disable_subpixel_aa &&
-        PREFS
-            .get("gfx.subpixel-text-antialiasing.enabled")
-            .as_boolean()
-            .unwrap();
+    let enable_subpixel_text_antialiasing =
+        !debug_options.disable_subpixel_aa && pref!(gfx.subpixel_text_antialiasing.enabled);
 
     let is_printing_version = opt_match.opt_present("v") || opt_match.opt_present("version");
 
@@ -1065,15 +1058,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
     }
 
     if let Some(layout_threads) = layout_threads {
-        PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
-    } else if let Some(layout_threads) = PREFS.get("layout.threads").as_string() {
-        PREFS.set(
-            "layout.threads",
-            PrefValue::Number(layout_threads.parse::<f64>().unwrap()),
-        );
-    } else if *PREFS.get("layout.threads") == PrefValue::Missing {
-        let layout_threads = cmp::max(num_cpus::get() * 3 / 4, 1);
-        PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
+        set_pref!(layout.threads, layout_threads as i64);
     }
 
     ArgumentParsingResult::ChromeProcess
@@ -1104,15 +1089,26 @@ pub fn get() -> RwLockReadGuard<'static, Opts> {
 pub fn parse_pref_from_command_line(pref: &str) {
     let split: Vec<&str> = pref.splitn(2, '=').collect();
     let pref_name = split[0];
-    let value = split.get(1);
-    match value {
-        Some(&"false") => PREFS.set(pref_name, PrefValue::Boolean(false)),
-        Some(&"true") | None => PREFS.set(pref_name, PrefValue::Boolean(true)),
-        Some(value) => match value.parse::<f64>() {
-            Ok(v) => PREFS.set(pref_name, PrefValue::Number(v)),
-            Err(_) => PREFS.set(pref_name, PrefValue::String(value.to_string())),
+    let pref_value = parse_cli_pref_value(split.get(1).cloned());
+    prefs::pref_map()
+        .set(pref_name, pref_value)
+        .expect(format!("Error setting preference: {}", pref).as_str());
+}
+
+fn parse_cli_pref_value(input: Option<&str>) -> PrefValue {
+    match input {
+        Some("true") | None => PrefValue::Bool(true),
+        Some("false") => PrefValue::Bool(false),
+        Some(string) => {
+            if let Some(int) = string.parse::<i64>().ok() {
+                PrefValue::Int(int)
+            } else if let Some(float) = string.parse::<f64>().ok() {
+                PrefValue::Float(float)
+            } else {
+                PrefValue::from(string)
+            }
         },
-    };
+    }
 }
 
 pub fn parse_url_or_filename(cwd: &Path, input: &str) -> Result<ServoUrl, ()> {

components/config/pref_util.rs

@@ -0,0 +1,282 @@
+/* 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 serde_json::Value;
+use std::collections::HashMap;
+use std::fmt;
+use std::str::FromStr;
+use std::sync::{Arc, RwLock};
+
+#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
+pub enum PrefValue {
+    Float(f64),
+    Int(i64),
+    Str(String),
+    Bool(bool),
+    Missing,
+}
+
+impl PrefValue {
+    pub fn as_str(&self) -> Option<&str> {
+        if let PrefValue::Str(val) = self {
+            Some(val)
+        } else {
+            None
+        }
+    }
+
+    pub fn as_i64(&self) -> Option<i64> {
+        if let PrefValue::Int(val) = self {
+            Some(*val)
+        } else {
+            None
+        }
+    }
+
+    pub fn as_f64(&self) -> Option<f64> {
+        if let PrefValue::Float(val) = self {
+            Some(*val)
+        } else {
+            None
+        }
+    }
+
+    pub fn as_bool(&self) -> Option<bool> {
+        if let PrefValue::Bool(val) = self {
+            Some(*val)
+        } else {
+            None
+        }
+    }
+
+    pub fn is_missing(&self) -> bool {
+        match self {
+            PrefValue::Missing => true,
+            _ => false,
+        }
+    }
+
+    pub fn from_json_value(value: &Value) -> Option<Self> {
+        match value {
+            Value::Bool(b) => Some(PrefValue::Bool(*b)),
+            Value::Number(n) if n.is_i64() => Some(PrefValue::Int(n.as_i64().unwrap())),
+            Value::Number(n) if n.is_f64() => Some(PrefValue::Float(n.as_f64().unwrap())),
+            Value::String(s) => Some(PrefValue::Str(s.to_owned())),
+            _ => None,
+        }
+    }
+}
+
+impl FromStr for PrefValue {
+    type Err = PrefError;
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s == "false" {
+            Ok(PrefValue::Bool(false))
+        } else if s == "true" {
+            Ok(PrefValue::Bool(true))
+        } else if let Ok(float) = s.parse::<f64>() {
+            Ok(PrefValue::Float(float))
+        } else if let Ok(integer) = s.parse::<i64>() {
+            Ok(PrefValue::Int(integer))
+        } else {
+            Ok(PrefValue::from(s))
+        }
+    }
+}
+
+macro_rules! impl_pref_from {
+    ($($t: ty => $variant: path,)*) => {
+        $(
+            impl From<$t> for PrefValue {
+                fn from(other: $t) -> Self {
+                    $variant(other.into())
+                }
+            }
+        )+
+        $(
+            impl From<Option<$t>> for PrefValue {
+                fn from(other: Option<$t>) -> Self {
+                    other.map(|val| $variant(val.into())).unwrap_or(PrefValue::Missing)
+                }
+            }
+        )+
+    }
+}
+
+macro_rules! impl_from_pref {
+    ($($variant: path => $t: ty,)*) => {
+        $(
+            impl From<PrefValue> for $t {
+                #[allow(unsafe_code)]
+                fn from(other: PrefValue) -> Self {
+                    if let $variant(value) = other {
+                        value.into()
+                    } else {
+                        panic!(
+                            format!("Cannot convert {:?} to {:?}",
+                                other,
+                                unsafe { std::intrinsics::type_name::<$t>() }
+                            )
+                        );
+                    }
+                }
+            }
+        )+
+        $(
+            impl From<PrefValue> for Option<$t> {
+                fn from(other: PrefValue) -> Self {
+                    if let PrefValue::Missing = other {
+                        None
+                    } else {
+                        Some(other.into())
+                    }
+                }
+            }
+        )+
+    }
+}
+
+impl_pref_from! {
+    f64 => PrefValue::Float,
+    i64 => PrefValue::Int,
+    String => PrefValue::Str,
+    &str => PrefValue::Str,
+    bool => PrefValue::Bool,
+}
+
+impl_from_pref! {
+    PrefValue::Float => f64,
+    PrefValue::Int => i64,
+    PrefValue::Str => String,
+    PrefValue::Bool => bool,
+}
+
+#[derive(Debug)]
+pub enum PrefError {
+    NoSuchPref(String),
+    InvalidValue(String),
+    JsonParseErr(serde_json::error::Error),
+}
+
+impl fmt::Display for PrefError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            PrefError::NoSuchPref(s) | PrefError::InvalidValue(s) => f.write_str(&s),
+            PrefError::JsonParseErr(e) => e.fmt(f),
+        }
+    }
+}
+
+impl std::error::Error for PrefError {}
+
+pub struct Accessor<P, V> {
+    pub getter: Box<Fn(&P) -> V + Sync>,
+    pub setter: Box<Fn(&mut P, V) + Sync>,
+}
+
+impl<P, V> Accessor<P, V> {
+    pub fn new<G, S>(getter: G, setter: S) -> Self
+    where
+        G: Fn(&P) -> V + Sync + 'static,
+        S: Fn(&mut P, V) + Sync + 'static,
+    {
+        Accessor {
+            getter: Box::new(getter),
+            setter: Box::new(setter),
+        }
+    }
+}
+
+pub struct Preferences<'m, P> {
+    user_prefs: Arc<RwLock<P>>,
+    default_prefs: P,
+    accessors: &'m HashMap<String, Accessor<P, PrefValue>>,
+}
+
+impl<'m, P: Clone> Preferences<'m, P> {
+    /// Create a new `Preferences` object. The values provided in `default_prefs` are immutable and
+    /// can always be restored using `reset` or `reset_all`.
+    pub fn new(default_prefs: P, accessors: &'m HashMap<String, Accessor<P, PrefValue>>) -> Self {
+        Self {
+            user_prefs: Arc::new(RwLock::new(default_prefs.clone())),
+            default_prefs,
+            accessors,
+        }
+    }
+
+    /// Access to the data structure holding the preference values.
+    pub fn values(&self) -> Arc<RwLock<P>> {
+        Arc::clone(&self.user_prefs)
+    }
+
+    /// Retrieve a preference using its key
+    pub fn get(&self, key: &str) -> PrefValue {
+        if let Some(accessor) = self.accessors.get(key) {
+            let prefs = self.user_prefs.read().unwrap();
+            (accessor.getter)(&prefs)
+        } else {
+            PrefValue::Missing
+        }
+    }
+
+    /// Creates an iterator over all keys and values
+    pub fn iter<'a>(&'a self) -> impl Iterator<Item = (String, PrefValue)> + 'a {
+        let prefs = self.user_prefs.read().unwrap();
+        self.accessors
+            .iter()
+            .map(move |(k, accessor)| (k.clone(), (accessor.getter)(&prefs)))
+    }
+
+    /// Creates an iterator over all keys
+    pub fn keys<'a>(&'a self) -> impl Iterator<Item = &'a str> + 'a {
+        self.accessors.keys().map(String::as_str)
+    }
+
+    fn set_inner<V>(&self, key: &str, mut prefs: &mut P, val: V) -> Result<(), PrefError>
+    where
+        V: Into<PrefValue>,
+    {
+        if let Some(accessor) = self.accessors.get(key) {
+            Ok((accessor.setter)(&mut prefs, val.into()))
+        } else {
+            Err(PrefError::NoSuchPref(String::from(key)))
+        }
+    }
+
+    /// Set a new value for a preference, using its key.
+    pub fn set<V>(&self, key: &str, val: V) -> Result<(), PrefError>
+    where
+        V: Into<PrefValue>,
+    {
+        let mut prefs = self.user_prefs.write().unwrap();
+        self.set_inner(key, &mut prefs, val)
+    }
+
+    pub fn set_all<M>(&self, values: M) -> Result<(), PrefError>
+    where
+        M: IntoIterator<Item = (String, PrefValue)>,
+    {
+        let mut prefs = self.user_prefs.write().unwrap();
+        for (k, v) in values.into_iter() {
+            self.set_inner(&k, &mut prefs, v)?;
+        }
+        Ok(())
+    }
+
+    pub fn reset(&self, key: &str) -> Result<PrefValue, PrefError> {
+        if let Some(accessor) = self.accessors.get(key) {
+            let mut prefs = self.user_prefs.write().unwrap();
+            let old_pref = (accessor.getter)(&prefs);
+            let default_pref = (accessor.getter)(&self.default_prefs);
+            (accessor.setter)(&mut prefs, default_pref);
+            Ok(old_pref)
+        } else {
+            Err(PrefError::NoSuchPref(String::from(key)))
+        }
+    }
+
+    pub fn reset_all(&self) {
+        *self.user_prefs.write().unwrap() = self.default_prefs.clone();
+    }
+}

components/config/prefs.rs

@@ -7,251 +7,475 @@ use crate::opts;
 use embedder_traits::resources::{self, Resource};
 use serde_json::{self, Value};
 use std::borrow::ToOwned;
-use std::cmp::max;
 use std::collections::HashMap;
 use std::fs::File;
 use std::io::{stderr, Read, Write};
 use std::path::PathBuf;
-use std::sync::{Arc, RwLock};
+
+use crate::pref_util::Preferences;
+pub use crate::pref_util::{PrefError, PrefValue};
+use gen::Prefs;
 
 lazy_static! {
-    pub static ref PREFS: Preferences = {
-        let defaults = default_prefs();
-        if let Ok(prefs) = read_prefs(&resources::read_string(Resource::Preferences)) {
-            defaults.extend(prefs);
-        }
-        defaults
+    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)
     };
 }
 
-#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
-pub enum PrefValue {
-    Boolean(bool),
-    String(String),
-    Number(f64),
-    Missing,
+/// A convenience macro for accessing a preference value using its static path.
+/// Passing an invalid path is a compile-time error.
+#[macro_export]
+macro_rules! pref {
+    ($($segment: ident).+) => {{
+        let values = $crate::prefs::pref_map().values();
+        let lock = values.read()
+            .map(|prefs| prefs $(.$segment)+.clone());
+        lock.unwrap()
+    }};
 }
 
-impl PrefValue {
-    pub fn from_json(data: Value) -> Result<PrefValue, ()> {
-        let value = match data {
-            Value::Bool(x) => PrefValue::Boolean(x),
-            Value::String(x) => PrefValue::String(x),
-            Value::Number(x) => {
-                if let Some(v) = x.as_f64() {
-                    PrefValue::Number(v)
-                } else {
-                    return Err(());
-                }
-            },
-            _ => return Err(()),
-        };
-        Ok(value)
-    }
-
-    pub fn as_boolean(&self) -> Option<bool> {
-        match *self {
-            PrefValue::Boolean(value) => Some(value),
-            _ => None,
-        }
-    }
-
-    pub fn as_string(&self) -> Option<&str> {
-        match *self {
-            PrefValue::String(ref value) => Some(&value),
-            _ => None,
-        }
-    }
-
-    pub fn as_i64(&self) -> Option<i64> {
-        match *self {
-            PrefValue::Number(x) => Some(x as i64),
-            _ => None,
-        }
-    }
-
-    pub fn as_u64(&self) -> Option<u64> {
-        match *self {
-            PrefValue::Number(x) => Some(x as u64),
-            _ => None,
-        }
-    }
+/// A convenience macro for updating a preference value using its static path.
+/// Passing an invalid path is a compile-time error.
+#[macro_export]
+macro_rules! set_pref {
+    ($($segment: ident).+, $value: expr) => {{
+        let values = $crate::prefs::pref_map().values();
+        let mut lock = values.write().unwrap();
+        lock$ (.$segment)+ = $value;
+    }};
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub enum Pref {
-    NoDefault(Arc<PrefValue>),
-    WithDefault(Arc<PrefValue>, Option<Arc<PrefValue>>),
-}
-
-impl Pref {
-    pub fn new(value: PrefValue) -> Pref {
-        Pref::NoDefault(Arc::new(value))
-    }
-
-    fn new_default(value: PrefValue) -> Pref {
-        Pref::WithDefault(Arc::new(value), None)
-    }
-
-    fn from_json(data: Value) -> Result<Pref, ()> {
-        let value = PrefValue::from_json(data)?;
-        Ok(Pref::new_default(value))
-    }
-
-    pub fn value(&self) -> &Arc<PrefValue> {
-        match *self {
-            Pref::NoDefault(ref x) => x,
-            Pref::WithDefault(ref default, ref override_value) => match *override_value {
-                Some(ref x) => x,
-                None => default,
-            },
-        }
-    }
-
-    fn set(&mut self, value: PrefValue) {
-        // TODO - this should error if we try to override a pref of one type
-        // with a value of a different type
-        match *self {
-            Pref::NoDefault(ref mut pref_value) => *pref_value = Arc::new(value),
-            Pref::WithDefault(_, ref mut override_value) => *override_value = Some(Arc::new(value)),
-        }
-    }
-}
-
-pub fn default_prefs() -> Preferences {
-    let prefs = Preferences(Arc::new(RwLock::new(HashMap::new())));
-    prefs.set(
-        "layout.threads",
-        PrefValue::Number(max(num_cpus::get() * 3 / 4, 1) as f64),
-    );
-    prefs
-}
-
-pub fn read_prefs(txt: &str) -> Result<HashMap<String, Pref>, ()> {
-    let json: Value = serde_json::from_str(txt).or_else(|e| {
-        println!("Ignoring invalid JSON in preferences: {:?}.", e);
-        Err(())
-    })?;
-
-    let mut prefs = HashMap::new();
-    if let Value::Object(obj) = json {
-        for (name, value) in obj.into_iter() {
-            match Pref::from_json(value) {
-                Ok(x) => {
-                    prefs.insert(name, x);
-                },
-                Err(_) => println!(
-                    "Ignoring non-boolean/string/i64 preference value for {:?}",
-                    name
-                ),
-            }
-        }
-    }
-    Ok(prefs)
+/// Access preferences using their `String` keys. Note that the key may be different from the
+/// static path because legacy keys contain hyphens, or because a preference name has been renamed.
+///
+/// When retrieving a preference, the value will always be a `PrefValue`. When setting a value, it
+/// may be a `PrefValue` or the type that converts into the correct underlying value; one of `bool`,
+/// `i64`, `f64` or `String`.
+#[inline]
+pub fn pref_map() -> &'static Preferences<'static, Prefs> {
+    &PREFS
 }
 
 pub fn add_user_prefs() {
-    match opts::get().config_dir {
-        Some(ref config_path) => {
-            let mut path = PathBuf::from(config_path);
-            init_user_prefs(&mut path);
-        },
-        None => {
-            if let Some(mut path) = default_config_dir() {
-                if path.join("prefs.json").exists() {
-                    init_user_prefs(&mut path);
-                }
-            }
-        },
+    if let Some(path) = user_prefs_path() {
+        init_user_prefs(path);
     }
 }
 
-fn init_user_prefs(path: &mut PathBuf) {
-    path.push("prefs.json");
-    if let Ok(mut file) = File::open(path) {
+fn user_prefs_path() -> Option<PathBuf> {
+    opts::get()
+        .config_dir
+        .clone()
+        .or_else(|| default_config_dir())
+        .map(|path| path.join("prefs.json"))
+        .filter(|path| path.exists())
+}
+
+fn init_user_prefs(path: PathBuf) {
+    if let Ok(mut file) = File::open(&path) {
         let mut txt = String::new();
-        file.read_to_string(&mut txt).expect("Can't read use prefs");
-        if let Ok(prefs) = read_prefs(&txt) {
-            PREFS.extend(prefs);
+        file.read_to_string(&mut txt)
+            .expect("Can't read user prefs");
+        match read_prefs_map(&txt) {
+            Ok(prefs) => {
+                if let Err(error) = PREFS.set_all(prefs.into_iter()) {
+                    writeln!(&mut stderr(), "Error setting preference: {:?}", error)
+                } else {
+                    Ok(())
+                }
+            },
+            Err(error) => writeln!(&mut stderr(), "Error parsing prefs.json: {:?}", error),
         }
     } else {
-        writeln!(
-            &mut stderr(),
-            "Error opening prefs.json from config directory"
-        )
-        .expect("failed printing to stderr");
+        writeln!(&mut stderr(), "Error opening user prefs from {:?}", path)
     }
+    .expect("failed printing to stderr");
 }
 
-pub struct Preferences(Arc<RwLock<HashMap<String, Pref>>>);
+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))?;
+    prefs
+        .into_iter()
+        .map(|(k, pref_value)| {
+            Ok({
+                let v = match &pref_value {
+                    Value::Bool(b) => PrefValue::Bool(*b),
+                    Value::Number(n) if n.is_i64() => PrefValue::Int(n.as_i64().unwrap()),
+                    Value::Number(n) if n.is_f64() => PrefValue::Float(n.as_f64().unwrap()),
+                    Value::String(s) => PrefValue::Str(s.to_owned()),
+                    _ => {
+                        return Err(PrefError::InvalidValue(format!(
+                            "Invalid value: {}",
+                            pref_value
+                        )));
+                    },
+                };
+                (k.to_owned(), v)
+            })
+        })
+        .collect()
+}
 
-impl Preferences {
-    pub fn get(&self, name: &str) -> Arc<PrefValue> {
-        self.0
-            .read()
-            .unwrap()
-            .get(name)
-            .map_or(Arc::new(PrefValue::Missing), |x| x.value().clone())
+mod gen {
+    use servo_config_plugins::build_structs;
+
+    // The number of layout threads is calculated if it is not present in `prefs.json`.
+    fn default_layout_threads() -> i64 {
+        std::cmp::max(num_cpus::get() * 3 / 4, 1) as i64
     }
 
-    pub fn cloned(&self) -> HashMap<String, Pref> {
-        self.0.read().unwrap().clone()
+    fn black() -> i64 {
+        0x000000
     }
 
-    pub fn set(&self, name: &str, value: PrefValue) {
-        let mut prefs = self.0.write().unwrap();
-        if let Some(pref) = prefs.get_mut(name) {
-            pref.set(value);
-            return;
-        }
-        prefs.insert(name.to_owned(), Pref::new(value));
+    fn white() -> i64 {
+        0xFFFFFF
     }
 
-    pub fn reset(&self, name: &str) -> Arc<PrefValue> {
-        let mut prefs = self.0.write().unwrap();
-        let result = match prefs.get_mut(name) {
-            None => return Arc::new(PrefValue::Missing),
-            Some(&mut Pref::NoDefault(_)) => Arc::new(PrefValue::Missing),
-            Some(&mut Pref::WithDefault(ref default, ref mut set_value)) => {
-                *set_value = None;
-                default.clone()
+    build_structs! {
+        // type of the accessors
+        accessor_type = crate::pref_util::Accessor::<Prefs, crate::pref_util::PrefValue>,
+        // name of the constant, which will hold a HashMap of preference accessors
+        gen_accessors = PREF_ACCESSORS,
+        // tree of structs to generate
+        gen_types = Prefs {
+            browser: {
+                display: {
+                    #[serde(default = "white")]
+                    background_color: i64,
+                    #[serde(default = "black")]
+                    foreground_color: i64,
+                }
+            },
+            css: {
+                animations: {
+                    testing: {
+                        #[serde(default)]
+                        enabled: bool,
+                    },
+                },
+            },
+            dom: {
+                bluetooth: {
+                    enabled: bool,
+                    testing: {
+                        enabled: bool,
+                    }
+                },
+                canvas_text: {
+                    #[serde(rename = "dom.canvas-text.enabled")]
+                    enabled: bool,
+                },
+                composition_event: {
+                    #[serde(rename = "dom.compositionevent.enabled")]
+                    enabled: bool,
+                },
+                custom_elements: {
+                    #[serde(rename = "dom.customelements.enabled")]
+                    enabled: bool,
+                },
+                document: {
+                    dblclick_timeout: i64,
+                    dblclick_dist: i64,
+                },
+                forcetouch: {
+                    enabled: bool,
+                },
+                fullscreen: {
+                    test: bool,
+                },
+                gamepad: {
+                    enabled: bool,
+                },
+                microdata: {
+                    testing: {
+                        enabled: bool,
+                    }
+                },
+                mouse_event: {
+                    which: {
+                        #[serde(rename = "dom.mouseevent.which.enabled")]
+                        enabled: bool,
+                    }
+                },
+                mutation_observer: {
+                    enabled: bool,
+                },
+                offscreen_canvas: {
+                    enabled: bool,
+                },
+                permissions: {
+                    enabled: bool,
+                    testing: {
+                        allowed_in_nonsecure_contexts: bool,
+                    }
+                },
+                serviceworker: {
+                    enabled: bool,
+                    timeout_seconds: i64,
+                },
+                servoparser: {
+                    async_html_tokenizer: {
+                        enabled: bool,
+                    }
+                },
+                svg: {
+                    enabled: bool,
+                },
+                testable_crash: {
+                    enabled: bool,
+                },
+                testbinding: {
+                    enabled: bool,
+                    prefcontrolled: {
+                        #[serde(default)]
+                        enabled: bool,
+                    },
+                    prefcontrolled2: {
+                        #[serde(default)]
+                        enabled: bool,
+                    },
+                    preference_value: {
+                        #[serde(default)]
+                        falsy: bool,
+                        #[serde(default)]
+                        quote_string_test: String,
+                        #[serde(default)]
+                        space_string_test: String,
+                        #[serde(default)]
+                        string_empty: String,
+                        #[serde(default)]
+                        string_test: String,
+                        #[serde(default)]
+                        truthy: bool,
+                    },
+                },
+                testing: {
+                    element: {
+                        activation: {
+                            #[serde(default)]
+                            enabled: bool,
+                        }
+                    },
+                    html_input_element: {
+                        select_files: {
+                            #[serde(rename = "dom.testing.htmlinputelement.select_files.enabled")]
+                            enabled: bool,
+                        }
+                    },
+                },
+                testperf: {
+                    #[serde(default)]
+                    enabled: bool,
+                },
+                webgl: {
+                    dom_to_texture: {
+                        enabled: bool,
+                    }
+                },
+                webgl2: {
+                    enabled: bool,
+                },
+                webrtc: {
+                    #[serde(default)]
+                    enabled: bool,
+                },
+                webvr: {
+                    enabled: bool,
+                    event_polling_interval: i64,
+                    test: bool,
+                },
+                webxr: {
+                    #[serde(default)]
+                    enabled: bool,
+                },
+                worklet: {
+                    blockingsleep: {
+                        #[serde(default)]
+                        enabled: bool,
+                    },
+                    #[serde(default)]
+                    enabled: bool,
+                    testing: {
+                        #[serde(default)]
+                        enabled: bool,
+                    },
+                    timeout_ms: i64,
+                },
+            },
+            gfx: {
+                subpixel_text_antialiasing: {
+                    #[serde(rename = "gfx.subpixel-text-antialiasing.enabled")]
+                    enabled: bool,
+                }
+            },
+            js: {
+                asmjs: {
+                    enabled: bool,
+                },
+                asyncstack: {
+                    enabled: bool,
+                },
+                baseline: {
+                    enabled: bool,
+                    unsafe_eager_compilation: {
+                        enabled: bool,
+                    },
+                },
+                discard_system_source: {
+                    enabled: bool,
+                },
+                dump_stack_on_debuggee_would_run: {
+                    enabled: bool,
+                },
+                ion: {
+                    enabled: bool,
+                    offthread_compilation: {
+                        enabled: bool,
+                    },
+                    unsafe_eager_compilation: {
+                        enabled: bool,
+                    },
+                },
+                mem: {
+                    gc: {
+                        allocation_threshold_mb: i64,
+                        allocation_threshold_factor: i64,
+                        allocation_threshold_avoid_interrupt_factor: i64,
+                        compacting: {
+                            enabled: bool,
+                        },
+                        decommit_threshold_mb: i64,
+                        dynamic_heap_growth: {
+                            enabled: bool,
+                        },
+                        dynamic_mark_slice: {
+                            enabled: bool,
+                        },
+                        empty_chunk_count_max: i64,
+                        empty_chunk_count_min: i64,
+                        high_frequency_heap_growth_max: i64,
+                        high_frequency_heap_growth_min: i64,
+                        high_frequency_high_limit_mb: i64,
+                        high_frequency_low_limit_mb: i64,
+                        high_frequency_time_limit_ms: i64,
+                        incremental: {
+                            enabled: bool,
+                            slice_ms: i64,
+                        },
+                        low_frequency_heap_growth: i64,
+                        per_compartment: {
+                            enabled: bool,
+                        },
+                        per_zone: {
+                            enabled: bool,
+                        },
+                        zeal: {
+                            frequency: i64,
+                            level: i64,
+                        },
+                    },
+                    high_water_mark: i64,
+                    max: i64,
+                },
+                native_regex: {
+                    enabled: bool,
+                },
+                offthread_compilation: {
+                    enabled: bool,
+                },
+                parallel_parsing: {
+                    enabled: bool,
+                },
+                shared_memory: {
+                    enabled: bool,
+                },
+                strict: {
+                    debug: {
+                        enabled: bool,
+                    },
+                    enabled: bool,
+                },
+                throw_on_asmjs_validation_failure: {
+                    enabled: bool,
+                },
+                throw_on_debuggee_would_run: {
+                    enabled: bool,
+                },
+                timers: {
+                    minimum_duration: i64,
+                },
+                wasm: {
+                    baseline: {
+                        enabled: bool,
+                    },
+                    enabled: bool,
+                    ion: {
+                        enabled: bool,
+                    }
+                },
+                werror: {
+                    enabled: bool,
+                },
+            },
+            layout: {
+                animations: {
+                    test: {
+                        enabled: bool,
+                    }
+                },
+                columns: {
+                    enabled: bool,
+                },
+                #[serde(default = "default_layout_threads")]
+                threads: i64,
+                viewport: {
+                    enabled: bool,
+                },
+                writing_mode: {
+                    #[serde(rename = "layout.writing-mode.enabled")]
+                    enabled: bool,
+                }
+            },
+            media: {
+                testing: {
+                    enabled: bool,
+                }
+            },
+            network: {
+                http_cache: {
+                    #[serde(rename = "network.http-cache.disabled")]
+                    disabled: bool,
+                },
+                mime: {
+                    sniff: bool,
+                }
+            },
+            session_history: {
+                #[serde(rename = "session-history.max-length")]
+                max_length: i64,
+            },
+            shell: {
+                homepage: String,
+                keep_screen_on: {
+                    enabled: bool,
+                },
+                #[serde(rename = "shell.native-orientation")]
+                native_orientation: String,
+                native_titlebar: {
+                    #[serde(rename = "shell.native-titlebar.enabled")]
+                    enabled: bool,
+                },
+                searchpage: String,
+            },
+            webgl: {
+                testing: {
+                    context_creation_error: bool,
+                }
             },
-        };
-        if *result == PrefValue::Missing {
-            prefs.remove(name);
         }
-        result
-    }
-
-    pub fn reset_all(&self) {
-        let names = {
-            self.0
-                .read()
-                .unwrap()
-                .keys()
-                .cloned()
-                .collect::<Vec<String>>()
-        };
-        for name in names.iter() {
-            self.reset(name);
-        }
-    }
-
-    pub fn extend(&self, extension: HashMap<String, Pref>) {
-        self.0.write().unwrap().extend(extension);
-    }
-
-    pub fn is_webvr_enabled(&self) -> bool {
-        self.get("dom.webvr.enabled").as_boolean().unwrap_or(false)
-    }
-
-    pub fn is_dom_to_texture_enabled(&self) -> bool {
-        self.get("dom.webgl.dom_to_texture.enabled")
-            .as_boolean()
-            .unwrap_or(false)
-    }
-
-    pub fn is_webgl2_enabled(&self) -> bool {
-        self.get("dom.webgl2.enabled").as_boolean().unwrap_or(false)
     }
 }

components/config/tests/opts.rs

@@ -2,8 +2,11 @@
  * 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/. */
 
+#[macro_use]
+extern crate servo_config;
+
 use servo_config::opts::{parse_pref_from_command_line, parse_url_or_filename};
-use servo_config::prefs::{PrefValue, PREFS};
+use servo_config::{prefs, prefs::PrefValue};
 use std::path::Path;
 
 #[cfg(not(target_os = "windows"))]
@@ -86,25 +89,53 @@ fn test_argument_parsing_special() {
     assert_eq!(url.fragment(), None);
 }
 
+#[test]
+fn test_invalid_prefs_from_command_line_panics() {
+    let err_msg = std::panic::catch_unwind(|| {
+        parse_pref_from_command_line("doesntexist=true");
+    })
+    .err()
+    .and_then(|a| a.downcast_ref::<String>().cloned())
+    .expect("Should panic");
+    assert!(
+        err_msg.starts_with("Error setting preference"),
+        "Message should describe the problem"
+    );
+    assert!(
+        err_msg.contains("doesntexist"),
+        "Message should mention the name of the preference"
+    );
+}
+
 #[test]
 fn test_parse_pref_from_command_line() {
     // Test with boolean values.
-    parse_pref_from_command_line("testtrue=true");
-    assert_eq!(*PREFS.get("testtrue"), PrefValue::Boolean(true));
-    parse_pref_from_command_line("testfalse=false");
-    assert_eq!(*PREFS.get("testfalse"), PrefValue::Boolean(false));
+    parse_pref_from_command_line("dom.bluetooth.enabled=true");
+    assert_eq!(
+        prefs::pref_map().get("dom.bluetooth.enabled"),
+        PrefValue::Bool(true)
+    );
+    assert_eq!(pref!(dom.bluetooth.enabled), true);
 
-    // Test with numbers.
-    parse_pref_from_command_line("testint=42");
-    assert_eq!(*PREFS.get("testint"), PrefValue::Number(42 as f64));
-    parse_pref_from_command_line("testfloat=4.2");
-    assert_eq!(*PREFS.get("testfloat"), PrefValue::Number(4.2));
+    parse_pref_from_command_line("dom.bluetooth.enabled=false");
+    assert_eq!(
+        prefs::pref_map().get("dom.bluetooth.enabled"),
+        PrefValue::Bool(false)
+    );
+    assert_eq!(pref!(dom.bluetooth.enabled), false);
 
-    // Test default (string).
-    parse_pref_from_command_line("teststr=str");
-    assert_eq!(*PREFS.get("teststr"), PrefValue::String("str".to_owned()));
+    // Test with numbers
+    parse_pref_from_command_line("layout.threads=42");
+    assert_eq!(pref!(layout.threads), 42);
 
-    // Test with no value.
-    parse_pref_from_command_line("testempty");
-    assert_eq!(*PREFS.get("testempty"), PrefValue::Boolean(true));
+    // Test string.
+    parse_pref_from_command_line("shell.homepage=str");
+    assert_eq!(pref!(shell.homepage), "str");
+
+    // Test with no value (defaults to true).
+    prefs::pref_map()
+        .set("dom.bluetooth.enabled", false)
+        .unwrap();
+    parse_pref_from_command_line("dom.bluetooth.enabled");
+    assert_eq!(pref!(dom.bluetooth.enabled), true);
 }

components/config/tests/prefs.rs

@@ -2,60 +2,246 @@
  * 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/. */
 
+#[macro_use]
+extern crate serde_derive;
+
 use servo_config::basedir;
-use servo_config::prefs::{read_prefs, PrefValue, PREFS};
+use servo_config::pref_util::Preferences;
+use servo_config::prefs::{read_prefs_map, PrefValue};
+use std::collections::HashMap;
+use std::error::Error;
 use std::fs::{self, File};
 use std::io::{Read, Write};
 
 #[test]
-fn test_create_pref() {
-    let json_str = "{\
-                    \"layout.writing-mode.enabled\": true,\
-                    \"network.mime.sniff\": false,\
-                    \"shell.homepage\": \"https://servo.org\"\
-                    }";
-
-    let prefs = read_prefs(json_str);
+fn test_create_prefs_map() {
+    let json_str = "{
+        \"layout.writing-mode.enabled\": true,
+        \"network.mime.sniff\": false,
+        \"shell.homepage\": \"https://servo.org\"
+    }";
+    let prefs = read_prefs_map(json_str);
     assert!(prefs.is_ok());
     let prefs = prefs.unwrap();
-
     assert_eq!(prefs.len(), 3);
 }
 
 #[test]
-fn test_get_set_reset_extend() {
-    let json_str = "{\
-                    \"layout.writing-mode.enabled\": true,\
-                    \"extra.stuff\": false,\
-                    \"shell.homepage\": \"https://google.com\"\
-                    }";
+fn test_generated_accessors_get() {
+    let prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR).unwrap();
+    let map: HashMap<String, PrefValue> = gen::TEST_PREF_ACCESSORS
+        .iter()
+        .map(move |(key, accessor)| {
+            let pref_value = (accessor.getter)(&prefs);
+            (key.clone(), pref_value)
+        })
+        .collect();
 
-    assert_eq!(*PREFS.get("test"), PrefValue::Missing);
-    PREFS.set("test", PrefValue::String("hi".to_owned()));
-    assert_eq!(*PREFS.get("test"), PrefValue::String("hi".to_owned()));
+    assert_eq!(&PrefValue::from("hello"), map.get("pref_string").unwrap());
+    assert_eq!(&PrefValue::from(23_i64), map.get("pref_i64").unwrap());
+    assert_eq!(&PrefValue::from(1.5_f64), map.get("pref_f64").unwrap());
+    assert_eq!(&PrefValue::from(true), map.get("pref_bool").unwrap());
     assert_eq!(
-        *PREFS.get("shell.homepage"),
-        PrefValue::String("https://servo.org".to_owned())
-    );
-    PREFS.set("shell.homepage", PrefValue::Boolean(true));
-    assert_eq!(*PREFS.get("shell.homepage"), PrefValue::Boolean(true));
-    PREFS.reset("shell.homepage");
-    assert_eq!(
-        *PREFS.get("shell.homepage"),
-        PrefValue::String("https://servo.org".to_owned())
+        &PrefValue::from(333_i64),
+        map.get("group.nested.nested_i64").unwrap()
     );
+    assert_eq!(&PrefValue::from(42_i64), map.get("a.renamed.pref").unwrap());
+}
 
-    let extension = read_prefs(json_str).unwrap();
-    PREFS.extend(extension);
-    assert_eq!(
-        *PREFS.get("shell.homepage"),
-        PrefValue::String("https://google.com".to_owned())
+#[test]
+fn test_generated_accessors_set() {
+    let mut prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR).unwrap();
+    let setters: HashMap<String, _> = gen::TEST_PREF_ACCESSORS
+        .iter()
+        .map(|(key, accessor)| (key.clone(), &accessor.setter))
+        .collect();
+
+    (setters.get("pref_string").unwrap())(&mut prefs, PrefValue::Str(String::from("boo")));
+    (setters.get("pref_i64").unwrap())(&mut prefs, PrefValue::Int(-25));
+    (setters.get("pref_f64").unwrap())(&mut prefs, PrefValue::Float(-1.9));
+    (setters.get("pref_bool").unwrap())(&mut prefs, PrefValue::Bool(false));
+    (setters.get("group.nested.nested_i64").unwrap())(&mut prefs, PrefValue::Int(10));
+    (setters.get("a.renamed.pref").unwrap())(&mut prefs, PrefValue::Int(11));
+
+    assert_eq!("boo", prefs.pref_string);
+    assert_eq!(-25, prefs.pref_i64);
+    assert_eq!(-1.9, prefs.pref_f64);
+    assert_eq!(false, prefs.pref_bool);
+    assert_eq!(10, prefs.group.nested.nested_i64);
+    assert_eq!(11, prefs.group.nested.renamed);
+}
+
+#[test]
+fn test_static_struct() {
+    let prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR).unwrap();
+    assert_eq!("hello", prefs.pref_string);
+    assert_eq!(23, prefs.pref_i64);
+    assert_eq!(1.5, prefs.pref_f64);
+    assert_eq!(true, prefs.pref_bool);
+    assert_eq!(333, prefs.group.nested.nested_i64);
+    assert_eq!(42, prefs.group.nested.renamed);
+}
+
+#[test]
+fn test_set_pref() {
+    let prefs = Preferences::new(gen::TestPrefs::default(), &gen::TEST_PREF_ACCESSORS);
+    assert_eq!(Some(0), prefs.get("group.nested.nested_i64").as_i64());
+    let result = prefs.set("group.nested.nested_i64", 1);
+    assert_eq!(true, result.is_ok());
+    assert_eq!(Some(1), prefs.get("group.nested.nested_i64").as_i64());
+    assert_eq!(1, prefs.values().read().unwrap().group.nested.nested_i64);
+}
+
+#[test]
+fn test_set_unknown_pref_is_err() -> Result<(), Box<dyn Error>> {
+    let prefs = Preferences::new(gen::TestPrefs::default(), &gen::TEST_PREF_ACCESSORS);
+    let result = prefs.set("unknown_pref", 1);
+    assert_eq!(true, result.is_err());
+    Ok(())
+}
+
+#[test]
+fn test_reset_pref() -> Result<(), Box<dyn Error>> {
+    let mut def_prefs = gen::TestPrefs::default();
+    def_prefs.group.nested.nested_i64 = 999;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    assert_eq!(Some(999), prefs.get("group.nested.nested_i64").as_i64());
+
+    prefs.set("group.nested.nested_i64", 1)?;
+    assert_eq!(Some(1), prefs.get("group.nested.nested_i64").as_i64());
+
+    prefs.reset("group.nested.nested_i64")?;
+    assert_eq!(Some(999), prefs.get("group.nested.nested_i64").as_i64());
+    assert_eq!(999, prefs.values().read().unwrap().group.nested.nested_i64);
+    Ok(())
+}
+
+#[test]
+fn test_default_values() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    assert_eq!(Some(0), prefs.get("default_value").as_i64());
+    assert_eq!(Some(555), prefs.get("computed_default_value").as_i64());
+    Ok(())
+}
+
+#[test]
+fn test_override_default_values() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(WITHOUT_DEFAULTS_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    assert_eq!(Some(-1), prefs.get("default_value").as_i64());
+    assert_eq!(Some(-1), prefs.get("computed_default_value").as_i64());
+    Ok(())
+}
+
+#[test]
+fn test_update_reset_default_values() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+
+    prefs.set("default_value", 99)?;
+    prefs.set("computed_default_value", 199)?;
+    assert_eq!(Some(99), prefs.get("default_value").as_i64());
+    assert_eq!(Some(199), prefs.get("computed_default_value").as_i64());
+
+    prefs.reset("default_value")?;
+    prefs.reset("computed_default_value")?;
+    assert_eq!(Some(0), prefs.get("default_value").as_i64());
+    assert_eq!(Some(555), prefs.get("computed_default_value").as_i64());
+    Ok(())
+}
+
+#[test]
+fn test_update_reset_overridden_default_values() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(WITHOUT_DEFAULTS_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    prefs.set("default_value", 99)?;
+    prefs.set("computed_default_value", 199)?;
+    assert_eq!(Some(99), prefs.get("default_value").as_i64());
+    assert_eq!(Some(199), prefs.get("computed_default_value").as_i64());
+
+    prefs.reset("default_value")?;
+    prefs.reset("computed_default_value")?;
+    assert_eq!(Some(-1), prefs.get("default_value").as_i64());
+    assert_eq!(Some(-1), prefs.get("computed_default_value").as_i64());
+    Ok(())
+}
+
+#[test]
+fn test_user_prefs_override_and_reset() -> Result<(), Box<dyn Error>> {
+    let mut def_prefs = gen::TestPrefs::default();
+    def_prefs.group.nested.nested_i64 = 999;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+
+    prefs.set("group.nested.nested_i64", 45)?;
+    assert_eq!(Some(45), prefs.get("group.nested.nested_i64").as_i64());
+
+    prefs.reset("group.nested.nested_i64")?;
+    assert_eq!(Some(999), prefs.get("group.nested.nested_i64").as_i64());
+    Ok(())
+}
+
+#[test]
+fn test_reset_all() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    prefs.set_all(read_prefs_map(USER_JSON_STR)?)?;
+
+    let values = prefs.values();
+    assert_eq!("bye", values.read().unwrap().pref_string);
+    assert_eq!(-1, values.read().unwrap().pref_i64);
+    assert_eq!(-1.0, values.read().unwrap().pref_f64);
+    assert_eq!(false, values.read().unwrap().pref_bool);
+    assert_eq!(-1, values.read().unwrap().group.nested.nested_i64);
+    assert_eq!(-1, values.read().unwrap().group.nested.renamed);
+
+    prefs.reset_all();
+
+    let values = prefs.values();
+    assert_eq!("hello", values.read().unwrap().pref_string);
+    assert_eq!(23, values.read().unwrap().pref_i64);
+    assert_eq!(1.5, values.read().unwrap().pref_f64);
+    assert_eq!(true, values.read().unwrap().pref_bool);
+    assert_eq!(333, values.read().unwrap().group.nested.nested_i64);
+    assert_eq!(42, values.read().unwrap().group.nested.renamed);
+    Ok(())
+}
+
+#[test]
+fn test_set_all_from_map() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+    prefs.set_all(read_prefs_map(USER_JSON_STR)?)?;
+
+    let mut overrides = HashMap::new();
+    overrides.insert(String::from("pref_string"), PrefValue::from("new value"));
+    overrides.insert(
+        String::from("group.nested.nested_i64"),
+        PrefValue::from(1001),
     );
-    assert_eq!(
-        *PREFS.get("layout.writing-mode.enabled"),
-        PrefValue::Boolean(true)
-    );
-    assert_eq!(*PREFS.get("extra.stuff"), PrefValue::Boolean(false));
+    overrides.insert(String::from("a.renamed.pref"), PrefValue::from(47));
+
+    let result = prefs.set_all(overrides.into_iter());
+    assert_eq!(true, result.is_ok());
+
+    let values = prefs.values();
+    assert_eq!("new value", values.read().unwrap().pref_string);
+    assert_eq!(1001, values.read().unwrap().group.nested.nested_i64);
+    assert_eq!(47, values.read().unwrap().group.nested.renamed);
+    Ok(())
+}
+
+#[test]
+fn test_set_all_error_on_unknown_field() -> Result<(), Box<dyn Error>> {
+    let def_prefs: gen::TestPrefs = serde_json::from_str(DEF_JSON_STR)?;
+    let prefs = Preferences::new(def_prefs, &gen::TEST_PREF_ACCESSORS);
+
+    let mut overrides = HashMap::new();
+    overrides.insert(String::from("doesnt_exist"), PrefValue::from(1001));
+
+    let result = prefs.set_all(overrides.into_iter());
+    assert_eq!(true, result.is_err());
+    Ok(())
 }
 
 #[cfg(not(target_os = "android"))]
@@ -86,3 +272,63 @@ fn test_default_config_dir_create_read_write() {
 
     fs::remove_file(&json_path).unwrap();
 }
+
+static DEF_JSON_STR: &'static str = r#"{
+    "pref_string": "hello",
+    "pref_i64": 23,
+    "pref_f64": 1.5,
+    "pref_bool": true,
+    "group.nested.nested_i64": 333,
+    "a.renamed.pref": 42
+}"#;
+
+static USER_JSON_STR: &'static str = r#"{
+    "pref_string": "bye",
+    "pref_i64": -1,
+    "pref_f64": -1.0,
+    "pref_bool": false,
+    "group.nested.nested_i64": -1,
+    "a.renamed.pref": -1
+}"#;
+
+static WITHOUT_DEFAULTS_JSON_STR: &'static str = r#"{
+    "pref_string": "bye",
+    "pref_i64": -1,
+    "pref_f64": -1.0,
+    "pref_bool": false,
+    "group.nested.nested_i64": -1,
+    "a.renamed.pref": -1,
+    "computed_default_value": -1,
+    "default_value": -1
+}"#;
+
+mod gen {
+    use servo_config::pref_util::{Accessor, PrefValue};
+    use servo_config_plugins::build_structs;
+
+    fn compute_default() -> i64 {
+        555
+    }
+
+    build_structs! {
+        accessor_type = Accessor::<TestPrefs, PrefValue>,
+        gen_accessors = TEST_PREF_ACCESSORS,
+        gen_types = TestPrefs {
+            pref_string: String,
+            pref_i64: i64,
+            pref_f64: f64,
+            pref_bool: bool,
+            #[serde(default)]
+            default_value: i64,
+            #[serde(default = "compute_default")]
+            computed_default_value: i64,
+            group: {
+                nested: {
+                    nested_i64: i64,
+                    #[serde(rename = "a.renamed.pref")]
+                    renamed: i64,
+                }
+            }
+        }
+    }
+}