Auto merge of #7651 - jgraham:prefs_reset, r=Ms2ger

Implement resettable and String-valued prefs.



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7651)
<!-- Reviewable:end -->

components/compositing/constellation.rs

@@ -905,7 +905,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
                                    containing_pipeline_id: PipelineId,
                                    subpage_id: SubpageId,
                                    event: MozBrowserEvent) {
-        assert!(prefs::get_pref("dom.mozbrowser.enabled").unwrap_or(false));
+        assert!(prefs::get_pref("dom.mozbrowser.enabled").as_boolean().unwrap_or(false));
 
         // Find the script channel for the given parent pipeline,
         // and pass the event to that script task.
@@ -1387,7 +1387,7 @@ impl<LTF: LayoutTaskFactory, STF: ScriptTaskFactory> Constellation<LTF, STF> {
 
     // https://developer.mozilla.org/en-US/docs/Web/Events/mozbrowserlocationchange
     fn trigger_mozbrowserlocationchange(&self, pipeline_id: PipelineId) {
-        if prefs::get_pref("dom.mozbrowser.enabled").unwrap_or(false) {
+        if prefs::get_pref("dom.mozbrowser.enabled").as_boolean().unwrap_or(false) {
             // Work around borrow checker
             let event_info = {
                 let pipeline = self.pipeline(pipeline_id);

components/compositing/pipeline.rs

@@ -289,7 +289,7 @@ impl Pipeline {
     pub fn trigger_mozbrowser_event(&self,
                                      subpage_id: SubpageId,
                                      event: MozBrowserEvent) {
-        assert!(prefs::get_pref("dom.mozbrowser.enabled").unwrap_or(false));
+        assert!(prefs::get_pref("dom.mozbrowser.enabled").as_boolean().unwrap_or(false));
 
         let event = ConstellationControlMsg::MozBrowserEvent(self.id,
                                                              subpage_id,

components/script/dom/htmliframeelement.rs

@@ -40,7 +40,7 @@ use util::str::DOMString;
 use util::str::{self, LengthOrPercentageOrAuto};
 
 pub fn mozbrowser_enabled() -> bool {
-    prefs::get_pref("dom.mozbrowser.enabled").unwrap_or(false)
+    prefs::get_pref("dom.mozbrowser.enabled").as_boolean().unwrap_or(false)
 }
 
 #[derive(HeapSizeOf)]

components/script/dom/mouseevent.rs

@@ -171,7 +171,7 @@ impl MouseEventMethods for MouseEvent {
     // This returns the same result as current gecko.
     // https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/which
     fn Which(&self) -> i32 {
-        if prefs::get_pref("dom.mouseevent.which.enabled").unwrap_or(false) {
+        if prefs::get_pref("dom.mouseevent.which.enabled").as_boolean().unwrap_or(false) {
             (self.button.get() + 1) as i32
         } else {
             0

components/style/properties.mako.rs

@@ -477,7 +477,8 @@ pub mod longhands {
                 % for value in values[:-1]:
                     "${value}" => {
                         % if value in experimental_values:
-                            if !::util::prefs::get_pref("layout.${value}.enabled").unwrap_or(false) {
+                            if !::util::prefs::get_pref("layout.${value}.enabled")
+                                .as_boolean().unwrap_or(false) {
                                 return Err(())
                             }
                         % endif
@@ -487,7 +488,8 @@ pub mod longhands {
                 % for value in values[-1:]:
                     "${value}" => {
                         % if value in experimental_values:
-                            if !::util::prefs::get_pref("layout.${value}.enabled".unwrap_or(false) {
+                            if !::util::prefs::get_pref("layout.${value}.enabled")
+                                .as_boolean().unwrap_or(false) {
                                 return Err(())
                             }
                         % endif
@@ -5945,7 +5947,8 @@ impl PropertyDeclaration {
                 % if property.derived_from is None:
                     "${property.name}" => {
                         % if property.experimental:
-                            if !::util::prefs::get_pref("${property.experimental}").unwrap_or(false) {
+                            if !::util::prefs::get_pref("${property.experimental}")
+                                .as_boolean().unwrap_or(false) {
                                 return PropertyDeclarationParseResult::ExperimentalProperty
                             }
                         % endif
@@ -5964,7 +5967,8 @@ impl PropertyDeclaration {
             % for shorthand in SHORTHANDS:
                 "${shorthand.name}" => {
                     % if shorthand.experimental:
-                        if !::util::prefs::get_pref("${shorthand.experimental}").unwrap_or(false) {
+                        if !::util::prefs::get_pref("${shorthand.experimental}")
+                            .as_boolean().unwrap_or(false) {
                             return PropertyDeclarationParseResult::ExperimentalProperty
                         }
                     % endif

components/style/stylesheets.rs

@@ -426,7 +426,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
                 Ok(AtRuleType::WithBlock(AtRulePrelude::FontFace))
             },
             "viewport" => {
-                if ::util::prefs::get_pref("layout.viewport.enabled").unwrap_or(false) {
+                if ::util::prefs::get_pref("layout.viewport.enabled").as_boolean().unwrap_or(false) {
                     Ok(AtRuleType::WithBlock(AtRulePrelude::Viewport))
                 } else {
                     Err(())

components/util/opts.rs

@@ -9,7 +9,8 @@ use euclid::size::{Size2D, TypedSize2D};
 use geometry::ScreenPx;
 use getopts::Options;
 use num_cpus;
-use prefs;
+use prefs::{self, PrefValue};
+use resource_files::set_resources_path;
 use std::cmp;
 use std::default::Default;
 use std::env;
@@ -155,9 +156,6 @@ pub struct Opts {
     /// Whether to show an error when display list geometry escapes flow overflow regions.
     pub validate_display_list_geometry: bool,
 
-    /// A specific path to find required resources (such as user-agent.css).
-    pub resources_path: Option<String>,
-
     /// Whether MIME sniffing should be used
     pub sniff_mime_types: bool,
 
@@ -409,7 +407,6 @@ pub fn default_opts() -> Opts {
         validate_display_list_geometry: false,
         profile_tasks: false,
         profile_script_events: false,
-        resources_path: None,
         sniff_mime_types: false,
         disable_share_style_cache: false,
         parallel_display_list_building: false,
@@ -460,6 +457,8 @@ pub fn from_cmdline_args(args: &[String]) {
         Err(f) => args_fail(&f.to_string()),
     };
 
+    set_resources_path(opt_match.opt_str("resources-path"));
+
     if opt_match.opt_present("h") || opt_match.opt_present("help") {
         print_usage(app_name, &opts);
         process::exit(0);
@@ -480,12 +479,21 @@ pub fn from_cmdline_args(args: &[String]) {
     }
 
     let cwd = env::current_dir().unwrap();
-    let url = if opt_match.free.is_empty() {
+    let homepage_pref = prefs::get_pref("shell.homepage");
-        print_usage(app_name, &opts);
+    let url_opt = if !opt_match.free.is_empty() {
-        args_fail("servo asks that you provide a URL")
+        Some(&opt_match.free[0][..])
     } else {
-        parse_url_or_filename(&cwd, &opt_match.free[0])
+        homepage_pref.as_string()
-            .unwrap_or_else(|()| args_fail("URL parsing failed"))
+    };
+    let url = match url_opt {
+        Some(url_string) => {
+            parse_url_or_filename(&cwd, url_string)
+                .unwrap_or_else(|()| args_fail("URL parsing failed"))
+        },
+        None => {
+            print_usage(app_name, &opts);
+            args_fail("servo asks that you provide a URL")
+        }
     };
 
     let tile_size: usize = match opt_match.opt_str("s") {
@@ -602,7 +610,6 @@ pub fn from_cmdline_args(args: &[String]) {
         dump_display_list_optimized: debug_options.dump_display_list_optimized,
         relayout_event: debug_options.relayout_event,
         validate_display_list_geometry: debug_options.validate_display_list_geometry,
-        resources_path: opt_match.opt_str("resources-path"),
         sniff_mime_types: opt_match.opt_present("sniff-mime-types"),
         disable_share_style_cache: debug_options.disable_share_style_cache,
         parallel_display_list_building: debug_options.parallel_display_list_building,
@@ -614,7 +621,7 @@ pub fn from_cmdline_args(args: &[String]) {
     // This must happen after setting the default options, since the prefs rely on
     // on the resource path.
     for pref in opt_match.opt_strs("pref").iter() {
-        prefs::set_pref(pref, true);
+        prefs::set_pref(pref, PrefValue::Boolean(true));
     }
 }
 

components/util/prefs.rs

@@ -3,19 +3,122 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use resource_files::resources_dir_path;
-use rustc_serialize::json::Json;
+use rustc_serialize::json::{Json, ToJson};
+use std::borrow::ToOwned;
 use std::collections::HashMap;
 use std::fs::File;
 use std::sync::{Arc, Mutex};
 
 lazy_static! {
-    static ref PREFS: Arc<Mutex<HashMap<String, bool>>> = {
+    static ref PREFS: Arc<Mutex<HashMap<String, Pref>>> = {
         let prefs = read_prefs().unwrap_or(HashMap::new());
         Arc::new(Mutex::new(prefs))
     };
 }
 
-fn read_prefs() -> Result<HashMap<String, bool>, ()> {
+#[derive(PartialEq, Clone, Debug)]
+pub enum PrefValue {
+    Boolean(bool),
+    String(String),
+    Missing
+}
+
+impl PrefValue {
+    pub fn from_json(data: Json) -> Result<PrefValue, ()> {
+        let value = match data {
+            Json::Boolean(x) => PrefValue::Boolean(x),
+            Json::String(x) => PrefValue::String(x),
+            _ => 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
+        }
+    }
+}
+
+impl ToJson for PrefValue {
+    fn to_json(&self) -> Json {
+        match *self {
+            PrefValue::Boolean(x) => {
+                Json::Boolean(x)
+            },
+            PrefValue::String(ref x) => {
+                Json::String(x.clone())
+            }
+            PrefValue::Missing => Json::Null
+        }
+    }
+}
+
+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: Json) -> Result<Pref, ()> {
+        let value = try!(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 {
+            &mut Pref::NoDefault(ref mut pref_value) => {
+                *pref_value = Arc::new(value)
+            },
+            &mut Pref::WithDefault(_, ref mut override_value) => {
+                *override_value = Some(Arc::new(value))
+            }
+        }
+    }
+}
+
+impl ToJson for Pref {
+    fn to_json(&self) -> Json {
+        self.value().to_json()
+    }
+}
+
+fn read_prefs() -> Result<HashMap<String, Pref>, ()> {
     let mut path = resources_dir_path();
     path.push("prefs.json");
 
@@ -29,22 +132,53 @@ fn read_prefs() -> Result<HashMap<String, bool>, ()> {
     }));
 
     let mut prefs = HashMap::new();
-    if let Some(obj) = json.as_object() {
+    if let Json::Object(obj) = json {
-        for (name, value) in obj.iter() {
+        for (name, value) in obj.into_iter() {
-            if let Some(bool_value) = value.as_boolean() {
+            match Pref::from_json(value) {
-                prefs.insert(name.clone(), bool_value);
+                Ok(x) => {
-            } else {
+                    prefs.insert(name, x);
-                println!("Ignoring non-boolean preference value for {:?}", name);
+                },
+                Err(_) => println!("Ignoring non-boolean/string preference value for {:?}", name)
             }
         }
     }
     Ok(prefs)
 }
 
-pub fn get_pref(name: &str) -> Option<bool> {
+pub fn get_pref(name: &str) -> Arc<PrefValue> {
-    PREFS.lock().unwrap().get(name).cloned()
+    PREFS.lock().unwrap().get(name).map(|x| x.value().clone()).unwrap_or(Arc::new(PrefValue::Missing))
 }
 
-pub fn set_pref(name: &str, value: bool) {
+pub fn set_pref(name: &str, value: PrefValue) {
-    let _ = PREFS.lock().unwrap().insert(name.to_owned(), value);
+    let mut prefs = PREFS.lock().unwrap();
+    if let Some(pref) = prefs.get_mut(name) {
+        pref.set(value);
+        return;
+    }
+    prefs.insert(name.to_owned(), Pref::new(value));
+}
+
+pub fn reset_pref(name: &str) -> Arc<PrefValue> {
+    let mut prefs = PREFS.lock().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()
+        },
+    };
+    if *result == PrefValue::Missing {
+        prefs.remove(name);
+    }
+    result
+}
+
+pub fn reset_all_prefs() {
+    let names = {
+        PREFS.lock().unwrap().keys().map(|x| x.clone()).collect::<Vec<String>>()
+    };
+    for name in names.iter() {
+        reset_pref(name);
+    }
 }

components/util/resource_files.rs

@@ -5,6 +5,18 @@
 use std::fs::File;
 use std::io::{self, Read};
 use std::path::PathBuf;
+use std::sync::{Arc, Mutex};
+
+lazy_static! {
+    static ref CMD_RESOURCE_DIR: Arc<Mutex<Option<String>>> = {
+        Arc::new(Mutex::new(None))
+    };
+}
+
+pub fn set_resources_path(path: Option<String>) {
+    let mut dir = CMD_RESOURCE_DIR.lock().unwrap();
+    *dir = path;
+}
 
 #[cfg(target_os = "android")]
 pub fn resources_dir_path() -> PathBuf {
@@ -13,11 +25,10 @@ pub fn resources_dir_path() -> PathBuf {
 
 #[cfg(not(target_os = "android"))]
 pub fn resources_dir_path() -> PathBuf {
-    use opts;
     use std::env;
     use std::fs::PathExt;
 
-    match opts::get().resources_path {
+    match *CMD_RESOURCE_DIR.lock().unwrap() {
         Some(ref path) => PathBuf::from(path),
         None => {
             // FIXME: Find a way to not rely on the executable being

components/webdriver_server/lib.rs

@@ -35,7 +35,7 @@ use std::collections::BTreeMap;
 use std::net::SocketAddr;
 use std::thread::{self, sleep_ms};
 use url::Url;
-use util::prefs::{get_pref, set_pref};
+use util::prefs::{get_pref, reset_all_prefs, reset_pref, set_pref, PrefValue};
 use util::task::spawn_named;
 use uuid::Uuid;
 use webdriver::command::{GetParameters, JavascriptCommandParameters, LocatorParameters};
@@ -49,7 +49,8 @@ use webdriver::server::{self, Session, WebDriverHandler};
 
 fn extension_routes() -> Vec<(Method, &'static str, ServoExtensionRoute)> {
     return vec![(Post, "/session/{sessionId}/servo/prefs/get", ServoExtensionRoute::GetPrefs),
-                (Post, "/session/{sessionId}/servo/prefs/set", ServoExtensionRoute::SetPrefs)]
+                (Post, "/session/{sessionId}/servo/prefs/set", ServoExtensionRoute::SetPrefs),
+                (Post, "/session/{sessionId}/servo/prefs/reset", ServoExtensionRoute::ResetPrefs)]
 }
 
 pub fn start_server(port: u16, constellation_chan: ConstellationChan) {
@@ -77,6 +78,7 @@ struct Handler {
 enum ServoExtensionRoute {
     GetPrefs,
     SetPrefs,
+    ResetPrefs,
 }
 
 impl WebDriverExtensionRoute for ServoExtensionRoute {
@@ -94,6 +96,10 @@ impl WebDriverExtensionRoute for ServoExtensionRoute {
                 let parameters: SetPrefsParameters = try!(Parameters::from_json(&body_data));
                 ServoExtensionCommand::SetPrefs(parameters)
             }
+            &ServoExtensionRoute::ResetPrefs => {
+                let parameters: GetPrefsParameters = try!(Parameters::from_json(&body_data));
+                ServoExtensionCommand::ResetPrefs(parameters)
+            }
         };
         Ok(WebDriverCommand::Extension(command))
     }
@@ -102,14 +108,16 @@ impl WebDriverExtensionRoute for ServoExtensionRoute {
 #[derive(Clone, PartialEq)]
 enum ServoExtensionCommand {
     GetPrefs(GetPrefsParameters),
-    SetPrefs(SetPrefsParameters)
+    SetPrefs(SetPrefsParameters),
+    ResetPrefs(GetPrefsParameters),
 }
 
 impl WebDriverExtensionCommand for ServoExtensionCommand {
     fn parameters_json(&self) -> Option<Json> {
         match self {
             &ServoExtensionCommand::GetPrefs(ref x) => Some(x.to_json()),
-            &ServoExtensionCommand::SetPrefs(ref x) => Some(x.to_json())
+            &ServoExtensionCommand::SetPrefs(ref x) => Some(x.to_json()),
+            &ServoExtensionCommand::ResetPrefs(ref x) => Some(x.to_json()),
         }
     }
 }
@@ -150,7 +158,7 @@ impl ToJson for GetPrefsParameters {
 
 #[derive(Clone, PartialEq)]
 struct SetPrefsParameters {
-    prefs: Vec<(String, bool)>
+    prefs: Vec<(String, PrefValue)>
 }
 
 impl Parameters for SetPrefsParameters {
@@ -166,9 +174,9 @@ impl Parameters for SetPrefsParameters {
                 "prefs was not an array")));
         let mut params = Vec::with_capacity(items.len());
         for (name, val) in items.iter() {
-            let value = try!(val.as_boolean().ok_or(
+            let value = try!(PrefValue::from_json(val.clone()).or(
-                WebDriverError::new(ErrorStatus::InvalidArgument,
+                Err(WebDriverError::new(ErrorStatus::InvalidArgument,
-                                    "Pref is not a bool")));
+                                        "Pref is not a boolean or string"))));
             let key = name.to_owned();
             params.push((key, value));
         }
@@ -633,16 +641,31 @@ impl Handler {
             .iter()
             .map(|item| (item.clone(), get_pref(item).to_json()))
             .collect::<BTreeMap<_, _>>();
+
         Ok(WebDriverResponse::Generic(ValueResponse::new(prefs.to_json())))
     }
 
     fn handle_set_prefs(&self,
                         parameters: &SetPrefsParameters) -> WebDriverResult<WebDriverResponse> {
         for &(ref key, ref value) in parameters.prefs.iter() {
-            set_pref(key, *value);
+            set_pref(key, value.clone());
         }
         Ok(WebDriverResponse::Void)
     }
+
+    fn handle_reset_prefs(&self,
+                          parameters: &GetPrefsParameters) -> WebDriverResult<WebDriverResponse> {
+        let prefs = if parameters.prefs.len() == 0 {
+            reset_all_prefs();
+            BTreeMap::new()
+        } else {
+            parameters.prefs
+                .iter()
+                .map(|item| (item.clone(), reset_pref(item).to_json()))
+                .collect::<BTreeMap<_, _>>()
+        };
+        Ok(WebDriverResponse::Generic(ValueResponse::new(prefs.to_json())))
+    }
 }
 
 impl WebDriverHandler<ServoExtensionRoute> for Handler {
@@ -684,6 +707,7 @@ impl WebDriverHandler<ServoExtensionRoute> for Handler {
                 match extension {
                     &ServoExtensionCommand::GetPrefs(ref x) => self.handle_get_prefs(x),
                     &ServoExtensionCommand::SetPrefs(ref x) => self.handle_set_prefs(x),
+                    &ServoExtensionCommand::ResetPrefs(ref x) => self.handle_reset_prefs(x),
                 }
             }
             _ => Err(WebDriverError::new(ErrorStatus::UnsupportedOperation,

ports/cef/core.rs

@@ -74,7 +74,6 @@ pub extern "C" fn cef_initialize(args: *const cef_main_args_t,
     temp_opts.hard_fail = false;
     temp_opts.enable_text_antialiasing = true;
     temp_opts.enable_canvas_antialiasing = true;
-    temp_opts.resources_path = None;
     temp_opts.url = None;
     opts::set_defaults(temp_opts);
 

resources/prefs.json

@@ -9,5 +9,6 @@
   "layout.flex-direction.enabled": false,
   "layout.text-orientation.enabled": false,
   "layout.viewport.enabled": false,
-  "layout.writing-mode.enabled": false
+  "layout.writing-mode.enabled": false,
+  "shell.homepage": "http://doc.servo.org/servo/index.html"
 }

tests/reftest.rs

@@ -25,6 +25,7 @@ use std::io::{self, Read, Result};
 use std::path::{Path, PathBuf};
 use std::process;
 use std::process::{Command, Stdio};
+use std::thread::sleep_ms;
 use test::run_tests_console;
 use test::{AutoColor, DynTestName, DynTestFn, TestDesc, TestOpts, TestDescAndFn, ShouldPanic};
 use url::Url;
@@ -111,10 +112,22 @@ fn run(test_opts: TestOpts, all_tests: Vec<TestDescAndFn>,
     // Verify that we're passing in valid servo arguments. Otherwise, servo
     // will exit before we've run any tests, and it will appear to us as if
     // all the tests are failing.
-    let output = match Command::new(&servo_path()).args(&servo_args).output() {
+    let mut command = Command::new(&servo_path());
+    command
+        .args(&servo_args)
+        .arg("-z")
+        .arg("about:blank");
+
+    let mut child = match command.spawn() {
         Ok(p) => p,
         Err(e) => panic!("failed to execute process: {}", e),
     };
+
+    // Wait for the shell to launch or to fail
+    sleep_ms(1000);
+    child.kill().unwrap();
+    let output = try!(child.wait_with_output());
+
     let stderr = String::from_utf8(output.stderr).unwrap();
 
     if stderr.contains("Unrecognized") {

tests/unit/style/viewport.rs

@@ -26,7 +26,8 @@ fn test_viewport_rule<F>(css: &str,
                          callback: F)
     where F: Fn(&Vec<ViewportDescriptorDeclaration>, &str)
 {
-    ::util::prefs::set_pref("layout.viewport.enabled", true);
+    ::util::prefs::set_pref("layout.viewport.enabled",
+                            ::util::prefs::PrefValue::Boolean(true));
 
     let stylesheet = stylesheet!(css, Author);
     let mut rule_count = 0;
@@ -173,7 +174,8 @@ fn cascading_within_viewport_rule() {
 
 #[test]
 fn multiple_stylesheets_cascading() {
-    ::util::prefs::set_pref("layout.viewport.enabled", true);
+    ::util::prefs::set_pref("layout.viewport.enabled",
+                            ::util::prefs::PrefValue::Boolean(true));
     let device = Device::new(MediaType::Screen, Size2D::typed(800., 600.));
 
     let stylesheets = vec![