Fix running servoshell and unit tests through a symlink (#30537)

* Fix running servoshell and unit tests through a symlink

* make filename an inherent method of Resource

* fix mach test-unit behaviour through symlink

* unit tests only need current_dir and ancestors

* fix macOS package smoketest breakage

components/embedder_traits/resources.rs

@@ -16,11 +16,6 @@ pub fn set(reader: Box<dyn ResourceReaderMethods + Sync + Send>) {
     *RES.write().unwrap() = Some(reader);
 }
 
-pub fn set_for_tests() {
-    static ONCE: Once = Once::new();
-    ONCE.call_once(|| set(resources_for_tests()));
-}
-
 pub fn read_bytes(res: Resource) -> Vec<u8> {
     RES.read()
         .unwrap()
@@ -66,26 +61,9 @@ pub enum Resource {
     CrashHTML,
 }
 
-pub trait ResourceReaderMethods {
+impl Resource {
-    fn read(&self, res: Resource) -> Vec<u8>;
+    pub fn filename(&self) -> &'static str {
-    fn sandbox_access_files(&self) -> Vec<PathBuf>;
+        match self {
-    fn sandbox_access_files_dirs(&self) -> Vec<PathBuf>;
-}
-
-fn resources_for_tests() -> Box<dyn ResourceReaderMethods + Sync + Send> {
-    use std::env;
-    use std::fs::File;
-    use std::io::Read;
-    struct ResourceReader;
-    impl ResourceReaderMethods for ResourceReader {
-        fn sandbox_access_files(&self) -> Vec<PathBuf> {
-            vec![]
-        }
-        fn sandbox_access_files_dirs(&self) -> Vec<PathBuf> {
-            vec![]
-        }
-        fn read(&self, file: Resource) -> Vec<u8> {
-            let file = match file {
             Resource::Preferences => "prefs.json",
             Resource::BluetoothBlocklist => "gatt_blocklist.txt",
             Resource::DomainList => "public_domains.txt",
@@ -100,23 +78,65 @@ fn resources_for_tests() -> Box<dyn ResourceReaderMethods + Sync + Send> {
             Resource::MediaControlsCSS => "media-controls.css",
             Resource::MediaControlsJS => "media-controls.js",
             Resource::CrashHTML => "crash.html",
-            };
+        }
-            let mut path = env::current_exe().unwrap();
+    }
-            path = path.canonicalize().unwrap();
+}
-            while path.pop() {
+
+pub trait ResourceReaderMethods {
+    fn read(&self, res: Resource) -> Vec<u8>;
+    fn sandbox_access_files(&self) -> Vec<PathBuf>;
+    fn sandbox_access_files_dirs(&self) -> Vec<PathBuf>;
+}
+
+// Can’t #[cfg(test)] the following because it breaks tests in dependent crates.
+
+pub fn set_for_tests() {
+    static ONCE: Once = Once::new();
+    ONCE.call_once(|| set(resources_for_tests()));
+}
+
+lazy_static::lazy_static! {
+    static ref CMD_RESOURCE_DIR: std::sync::Mutex<Option<PathBuf>> = std::sync::Mutex::new(None);
+}
+
+fn resources_dir_path_for_tests() -> PathBuf {
+    // This needs to be called before the process is sandboxed
+    // as we only give permission to read inside the resources directory,
+    // not the permissions the "search" for the resources directory.
+    let mut dir = CMD_RESOURCE_DIR.lock().unwrap();
+    if let Some(ref path) = *dir {
+        return PathBuf::from(path);
+    }
+
+    // Try ./resources in the current directory, then each of its ancestors.
+    let mut path = std::env::current_dir().unwrap();
+    loop {
         path.push("resources");
         if path.is_dir() {
-                    break;
+            *dir = Some(path);
+            return dir.clone().unwrap();
         }
         path.pop();
+
+        if !path.pop() {
+            panic!("Can't find resources directory")
         }
-            path.push(file);
+    }
-            let mut buffer = vec![];
+}
-            File::open(path)
+
-                .expect(&format!("Can't find file: {}", file))
+fn resources_for_tests() -> Box<dyn ResourceReaderMethods + Sync + Send> {
-                .read_to_end(&mut buffer)
+    struct ResourceReader;
-                .expect("Can't read file");
+    impl ResourceReaderMethods for ResourceReader {
-            buffer
+        fn sandbox_access_files(&self) -> Vec<PathBuf> {
+            vec![]
+        }
+        fn sandbox_access_files_dirs(&self) -> Vec<PathBuf> {
+            vec![resources_dir_path_for_tests()]
+        }
+        fn read(&self, file: Resource) -> Vec<u8> {
+            let mut path = resources_dir_path_for_tests();
+            path.push(file.filename());
+            std::fs::read(path).expect("Can't read file")
         }
     }
     Box::new(ResourceReader)

ports/gstplugin/resources.rs

@@ -8,82 +8,73 @@
 
 use std::path::PathBuf;
 use std::sync::Mutex;
-use std::{env, fs, io};
+use std::{env, fs};
 
-use lazy_static::lazy_static;
 use servo::embedder_traits::resources::{self, Resource};
 
-lazy_static! {
+lazy_static::lazy_static! {
-    static ref CMD_RESOURCE_DIR: Mutex<Option<String>> = Mutex::new(None);
+    static ref CMD_RESOURCE_DIR: Mutex<Option<PathBuf>> = Mutex::new(None);
 }
 
 struct ResourceReader;
 
-fn filename(file: Resource) -> &'static str {
-    match file {
-        Resource::Preferences => "prefs.json",
-        Resource::BluetoothBlocklist => "gatt_blocklist.txt",
-        Resource::DomainList => "public_domains.txt",
-        Resource::HstsPreloadList => "hsts_preload.json",
-        Resource::BadCertHTML => "badcert.html",
-        Resource::NetErrorHTML => "neterror.html",
-        Resource::UserAgentCSS => "user-agent.css",
-        Resource::ServoCSS => "servo.css",
-        Resource::PresentationalHintsCSS => "presentational-hints.css",
-        Resource::QuirksModeCSS => "quirks-mode.css",
-        Resource::RippyPNG => "rippy.png",
-        Resource::MediaControlsCSS => "media-controls.css",
-        Resource::MediaControlsJS => "media-controls.js",
-        Resource::CrashHTML => "crash.html",
-    }
-}
-
 pub fn init() {
     resources::set(Box::new(ResourceReader));
 }
 
-fn resources_dir_path() -> io::Result<PathBuf> {
+fn resources_dir_path() -> PathBuf {
     // This needs to be called before the process is sandboxed
     // as we only give permission to read inside the resources directory,
     // not the permissions the "search" for the resources directory.
     let mut dir = CMD_RESOURCE_DIR.lock().unwrap();
     if let Some(ref path) = *dir {
-        return Ok(PathBuf::from(path));
+        return PathBuf::from(path);
     }
 
-    // FIXME: Find a way to not rely on the executable being
+    // Try ./resources and ./Resources relative to the directory containing the
-    // under `<servo source>[/$target_triple]/target/debug`
+    // canonicalised executable path, then each of its ancestors.
-    // or `<servo source>[/$target_triple]/target/release`.
+    let mut path = env::current_exe().unwrap().canonicalize().unwrap();
-    let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    // Follow symlink
-    path = path.canonicalize()?;
-
     while path.pop() {
         path.push("resources");
         if path.is_dir() {
-            break;
+            *dir = Some(path);
+            return dir.clone().unwrap();
         }
         path.pop();
+
         // Check for Resources on mac when using a case sensitive filesystem.
         path.push("Resources");
         if path.is_dir() {
-            break;
+            *dir = Some(path);
+            return dir.clone().unwrap();
         }
         path.pop();
     }
-    *dir = Some(path.to_str().unwrap().to_owned());
+
-    Ok(path)
+    // Try ./resources in the current directory, then each of its ancestors.
+    let mut path = std::env::current_dir().unwrap();
+    loop {
+        path.push("resources");
+        if path.is_dir() {
+            *dir = Some(path);
+            return dir.clone().unwrap();
+        }
+        path.pop();
+
+        if !path.pop() {
+            panic!("Can't find resources directory")
+        }
+    }
 }
 
 impl resources::ResourceReaderMethods for ResourceReader {
     fn read(&self, file: Resource) -> Vec<u8> {
-        let file = filename(file);
+        let mut path = resources_dir_path();
-        let mut path = resources_dir_path().expect("Can't find resources directory");
+        path.push(file.filename());
-        path.push(file);
+        fs::read(path).expect("Can't read file")
-        fs::read(path.clone()).expect(&format!("Can't read file {:?}", path))
     }
     fn sandbox_access_files_dirs(&self) -> Vec<PathBuf> {
-        vec![resources_dir_path().expect("Can't find resources directory")]
+        vec![resources_dir_path()]
     }
     fn sandbox_access_files(&self) -> Vec<PathBuf> {
         vec![]

ports/servoshell/prefs.rs

@@ -6,10 +6,9 @@ use std::collections::HashMap;
 use std::fs::File;
 use std::io::Read;
 
-use getopts::{Matches, Options};
+use getopts::Matches;
-use servo::config::opts::{self, ArgumentParsingResult};
+use servo::config::opts;
 use servo::config::prefs::{self, PrefValue};
-use servo::embedder_traits;
 use servo::servo_config::basedir;
 
 pub fn register_user_prefs(opts_matches: &Matches) {
@@ -61,16 +60,15 @@ pub fn register_user_prefs(opts_matches: &Matches) {
     prefs::add_user_prefs(userprefs);
 }
 
-// Use for test
+#[cfg(test)]
-#[allow(dead_code)]
 fn test_parse_pref(arg: &str) {
-    embedder_traits::resources::set_for_tests();
+    servo::embedder_traits::resources::set_for_tests();
-    let mut opts = Options::new();
+    let mut opts = getopts::Options::new();
     opts.optmulti("", "pref", "", "");
     let args = vec!["servo".to_string(), "--pref".to_string(), arg.to_string()];
     let matches = match opts::from_cmdline_args(opts, &args) {
-        ArgumentParsingResult::ContentProcess(m, _) => m,
+        opts::ArgumentParsingResult::ContentProcess(m, _) => m,
-        ArgumentParsingResult::ChromeProcess(m) => m,
+        opts::ArgumentParsingResult::ChromeProcess(m) => m,
     };
     register_user_prefs(&matches);
 }

ports/servoshell/resources.rs

@@ -4,81 +4,73 @@
 
 use std::path::PathBuf;
 use std::sync::Mutex;
-use std::{env, fs, io};
+use std::{env, fs};
 
 use servo::embedder_traits::resources::{self, Resource};
 
 lazy_static::lazy_static! {
-    static ref CMD_RESOURCE_DIR: Mutex<Option<String>> = Mutex::new(None);
+    static ref CMD_RESOURCE_DIR: Mutex<Option<PathBuf>> = Mutex::new(None);
 }
 
 struct ResourceReader;
 
-fn filename(file: Resource) -> &'static str {
-    match file {
-        Resource::Preferences => "prefs.json",
-        Resource::BluetoothBlocklist => "gatt_blocklist.txt",
-        Resource::DomainList => "public_domains.txt",
-        Resource::HstsPreloadList => "hsts_preload.json",
-        Resource::BadCertHTML => "badcert.html",
-        Resource::NetErrorHTML => "neterror.html",
-        Resource::UserAgentCSS => "user-agent.css",
-        Resource::ServoCSS => "servo.css",
-        Resource::PresentationalHintsCSS => "presentational-hints.css",
-        Resource::QuirksModeCSS => "quirks-mode.css",
-        Resource::RippyPNG => "rippy.png",
-        Resource::MediaControlsCSS => "media-controls.css",
-        Resource::MediaControlsJS => "media-controls.js",
-        Resource::CrashHTML => "crash.html",
-    }
-}
-
 pub fn init() {
     resources::set(Box::new(ResourceReader));
 }
 
-fn resources_dir_path() -> io::Result<PathBuf> {
+fn resources_dir_path() -> PathBuf {
     // This needs to be called before the process is sandboxed
     // as we only give permission to read inside the resources directory,
     // not the permissions the "search" for the resources directory.
     let mut dir = CMD_RESOURCE_DIR.lock().unwrap();
     if let Some(ref path) = *dir {
-        return Ok(PathBuf::from(path));
+        return PathBuf::from(path);
     }
 
-    // FIXME: Find a way to not rely on the executable being
+    // Try ./resources and ./Resources relative to the directory containing the
-    // under `<servo source>[/$target_triple]/target/debug`
+    // canonicalised executable path, then each of its ancestors.
-    // or `<servo source>[/$target_triple]/target/release`.
+    let mut path = env::current_exe().unwrap().canonicalize().unwrap();
-    let mut path = env::current_exe()?;
-    // Follow symlink
-    path = path.canonicalize()?;
-
     while path.pop() {
         path.push("resources");
         if path.is_dir() {
-            break;
+            *dir = Some(path);
+            return dir.clone().unwrap();
         }
         path.pop();
+
         // Check for Resources on mac when using a case sensitive filesystem.
         path.push("Resources");
         if path.is_dir() {
-            break;
+            *dir = Some(path);
+            return dir.clone().unwrap();
         }
         path.pop();
     }
-    *dir = Some(path.to_str().unwrap().to_owned());
+
-    Ok(path)
+    // Try ./resources in the current directory, then each of its ancestors.
+    let mut path = std::env::current_dir().unwrap();
+    loop {
+        path.push("resources");
+        if path.is_dir() {
+            *dir = Some(path);
+            return dir.clone().unwrap();
+        }
+        path.pop();
+
+        if !path.pop() {
+            panic!("Can't find resources directory")
+        }
+    }
 }
 
 impl resources::ResourceReaderMethods for ResourceReader {
     fn read(&self, file: Resource) -> Vec<u8> {
-        let file = filename(file);
+        let mut path = resources_dir_path();
-        let mut path = resources_dir_path().expect("Can't find resources directory");
+        path.push(file.filename());
-        path.push(file);
         fs::read(path).expect("Can't read file")
     }
     fn sandbox_access_files_dirs(&self) -> Vec<PathBuf> {
-        vec![resources_dir_path().expect("Can't find resources directory")]
+        vec![resources_dir_path()]
     }
     fn sandbox_access_files(&self) -> Vec<PathBuf> {
         vec![]