diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs index ee83717b080..6bd5ff4c23f 100644 --- a/components/net/blob_loader.rs +++ b/components/net/blob_loader.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use filemanager_thread::{FileManager, UIProvider}; +use filemanager_thread::FileManager; use hyper::header::{Charset, ContentLength, ContentType, Headers}; use hyper::header::{ContentDisposition, DispositionParam, DispositionType}; use hyper_serde::Serde; @@ -12,7 +12,7 @@ use mime_classifier::MimeClassifier; use net_traits::{LoadConsumer, LoadData, Metadata, NetworkError}; use net_traits::ProgressMsg::{Done, Payload}; use net_traits::blob_url_store::parse_blob_url; -use net_traits::filemanager_thread::{FileManagerThreadMsg, ReadFileProgress}; +use net_traits::filemanager_thread::ReadFileProgress; use net_traits::response::HttpsState; use resource_thread::{send_error, start_sending_sniffed_opt}; use resource_thread::CancellationListener; @@ -24,7 +24,7 @@ use util::thread::spawn_named; // TODO: Check on GET // https://w3c.github.io/FileAPI/#requestResponseModel -pub fn factory(filemanager: FileManager) +pub fn factory(filemanager: FileManager) -> Box, CancellationListener) + Send> { box move |load_data: LoadData, start_chan, classifier, cancel_listener| { spawn_named(format!("blob loader for {}", load_data.url), move || { @@ -33,16 +33,14 @@ pub fn factory(filemanager: FileManager) } } -fn load_blob - (load_data: LoadData, start_chan: LoadConsumer, +fn load_blob(load_data: LoadData, start_chan: LoadConsumer, classifier: Arc, - filemanager: FileManager, + filemanager: FileManager, cancel_listener: CancellationListener) { let (chan, recv) = ipc::channel().unwrap(); if let Ok((id, origin, _fragment)) = parse_blob_url(&load_data.url.clone()) { let check_url_validity = true; - let msg = FileManagerThreadMsg::ReadFile(chan, id, check_url_validity, origin); - let _ = filemanager.handle(msg, Some(cancel_listener)); + filemanager.read_file(chan, id, check_url_validity, origin, Some(cancel_listener)); // Receive first chunk match recv.recv().unwrap() { @@ -123,9 +121,9 @@ fn load_blob /// https://fetch.spec.whatwg.org/#concept-basic-fetch (partial) // TODO: make async. -pub fn load_blob_sync +pub fn load_blob_sync (url: ServoUrl, - filemanager: FileManager) + filemanager: FileManager) -> Result<(Headers, Vec), NetworkError> { let (id, origin) = match parse_blob_url(&url) { Ok((id, origin, _fragment)) => (id, origin), @@ -137,8 +135,7 @@ pub fn load_blob_sync let (sender, receiver) = ipc::channel().unwrap(); let check_url_validity = true; - let msg = FileManagerThreadMsg::ReadFile(sender, id, check_url_validity, origin); - let _ = filemanager.handle(msg, None); + filemanager.read_file(sender, id, check_url_validity, origin, None); let blob_buf = match receiver.recv().unwrap() { Ok(ReadFileProgress::Meta(blob_buf)) => blob_buf, diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 82bfd0f7c1d..f6a6c62f489 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -7,7 +7,7 @@ use connector::create_http_connector; use data_loader::decode; use devtools_traits::DevtoolsControlMsg; use fetch::cors_cache::CorsCache; -use filemanager_thread::{FileManager, UIProvider}; +use filemanager_thread::FileManager; use http_loader::{HttpState, set_default_accept_encoding, set_default_accept_language, set_request_cookies}; use http_loader::{NetworkHttpRequestFactory, ReadResult, StreamedResponse, obtain_response, read_block}; use http_loader::{auth_from_cache, determine_request_referrer, set_cookies_from_headers}; @@ -52,28 +52,28 @@ enum Data { Done, } -pub struct FetchContext { +pub struct FetchContext { pub state: HttpState, pub user_agent: Cow<'static, str>, pub devtools_chan: Option>, - pub filemanager: FileManager, + pub filemanager: FileManager, } type DoneChannel = Option<(Sender, Receiver)>; /// [Fetch](https://fetch.spec.whatwg.org#concept-fetch) -pub fn fetch(request: Rc, - target: &mut Target, - context: &FetchContext) - -> Response { +pub fn fetch(request: Rc, + target: &mut Target, + context: &FetchContext) + -> Response { fetch_with_cors_cache(request, &mut CorsCache::new(), target, context) } -pub fn fetch_with_cors_cache(request: Rc, - cache: &mut CorsCache, - target: &mut Target, - context: &FetchContext) - -> Response { +pub fn fetch_with_cors_cache(request: Rc, + cache: &mut CorsCache, + target: &mut Target, + context: &FetchContext) + -> Response { // Step 1 if request.window.get() == Window::Client { // TODO: Set window to request's client object if client is a Window object @@ -136,14 +136,14 @@ pub fn fetch_with_cors_cache(request: Rc, } /// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch) -fn main_fetch(request: Rc, - cache: &mut CorsCache, - cors_flag: bool, - recursive_flag: bool, - target: &mut Target, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn main_fetch(request: Rc, + cache: &mut CorsCache, + cors_flag: bool, + recursive_flag: bool, + target: &mut Target, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { // TODO: Implement main fetch spec // Step 1 @@ -400,12 +400,12 @@ fn main_fetch(request: Rc, } /// [Basic fetch](https://fetch.spec.whatwg.org#basic-fetch) -fn basic_fetch(request: Rc, - cache: &mut CorsCache, - target: &mut Target, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn basic_fetch(request: Rc, + cache: &mut CorsCache, + target: &mut Target, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { let url = request.current_url(); match url.scheme() { @@ -492,15 +492,15 @@ fn basic_fetch(request: Rc, } /// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch) -fn http_fetch(request: Rc, - cache: &mut CorsCache, - cors_flag: bool, - cors_preflight_flag: bool, - authentication_fetch_flag: bool, - target: &mut Target, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn http_fetch(request: Rc, + cache: &mut CorsCache, + cors_flag: bool, + cors_preflight_flag: bool, + authentication_fetch_flag: bool, + target: &mut Target, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { // This is a new async fetch, reset the channel we are waiting on *done_chan = None; // Step 1 @@ -667,14 +667,14 @@ fn http_fetch(request: Rc, } /// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch) -fn http_redirect_fetch(request: Rc, - cache: &mut CorsCache, - response: Response, - cors_flag: bool, - target: &mut Target, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn http_redirect_fetch(request: Rc, + cache: &mut CorsCache, + response: Response, + cors_flag: bool, + target: &mut Target, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { // Step 1 assert_eq!(response.return_internal.get(), true); @@ -748,12 +748,12 @@ fn http_redirect_fetch(request: Rc, } /// [HTTP network or cache fetch](https://fetch.spec.whatwg.org#http-network-or-cache-fetch) -fn http_network_or_cache_fetch(request: Rc, - credentials_flag: bool, - authentication_fetch_flag: bool, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn http_network_or_cache_fetch(request: Rc, + credentials_flag: bool, + authentication_fetch_flag: bool, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { // TODO: Implement Window enum for Request let request_has_no_window = true; @@ -970,11 +970,11 @@ fn http_network_or_cache_fetch(request: Rc, } /// [HTTP network fetch](https://fetch.spec.whatwg.org/#http-network-fetch) -fn http_network_fetch(request: Rc, - credentials_flag: bool, - done_chan: &mut DoneChannel, - context: &FetchContext) - -> Response { +fn http_network_fetch(request: Rc, + credentials_flag: bool, + done_chan: &mut DoneChannel, + context: &FetchContext) + -> Response { // TODO: Implement HTTP network fetch spec // Step 1 @@ -1158,10 +1158,10 @@ fn http_network_fetch(request: Rc, } /// [CORS preflight fetch](https://fetch.spec.whatwg.org#cors-preflight-fetch) -fn cors_preflight_fetch(request: Rc, - cache: &mut CorsCache, - context: &FetchContext) - -> Response { +fn cors_preflight_fetch(request: Rc, + cache: &mut CorsCache, + context: &FetchContext) + -> Response { // Step 1 let mut preflight = Request::new(request.current_url(), Some(request.origin.borrow().clone()), request.is_service_worker_global_scope, request.pipeline_id.get()); diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 5b520b9e31d..f109c77c251 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -112,64 +112,81 @@ enum FileImpl { Sliced(Uuid, RelativePos), } -pub struct FileManager { - store: Arc>, +#[derive(Clone)] +pub struct FileManager { + store: Arc, } -// Not derived to avoid an unnecessary `UI: Clone` bound. -impl Clone for FileManager { - fn clone(&self) -> Self { +impl FileManager { + pub fn new() -> FileManager { FileManager { - store: self.store.clone(), + store: Arc::new(FileManagerStore::new()), } } -} -impl FileManager { - pub fn new(ui: &'static UI) -> FileManager { - FileManager { - store: Arc::new(FileManagerStore::new(ui)), - } + pub fn read_file(&self, + sender: IpcSender>, + id: Uuid, + check_url_validity: bool, + origin: FileOrigin, + cancel_listener: Option) { + let store = self.store.clone(); + spawn_named("read file".to_owned(), move || { + if let Err(e) = store.try_read_file(&sender, id, check_url_validity, + origin, cancel_listener) { + let _ = sender.send(Err(FileManagerThreadError::BlobURLStoreError(e))); + } + }) + } + + pub fn promote_memory(&self, + blob_buf: BlobBuf, + set_valid: bool, + sender: IpcSender>, + origin: FileOrigin) { + let store = self.store.clone(); + spawn_named("transfer memory".to_owned(), move || { + store.promote_memory(blob_buf, set_valid, sender, origin); + }) } /// Message handler - pub fn handle(&self, msg: FileManagerThreadMsg, cancel_listener: Option) { - let store = self.store.clone(); + pub fn handle(&self, + msg: FileManagerThreadMsg, + cancel_listener: Option, + ui: &'static UI) + where UI: UIProvider + 'static, + { match msg { FileManagerThreadMsg::SelectFile(filter, sender, origin, opt_test_path) => { + let store = self.store.clone(); spawn_named("select file".to_owned(), move || { - store.select_file(filter, sender, origin, opt_test_path); + store.select_file(filter, sender, origin, opt_test_path, ui); }); } FileManagerThreadMsg::SelectFiles(filter, sender, origin, opt_test_paths) => { + let store = self.store.clone(); spawn_named("select files".to_owned(), move || { - store.select_files(filter, sender, origin, opt_test_paths); + store.select_files(filter, sender, origin, opt_test_paths, ui); }) } FileManagerThreadMsg::ReadFile(sender, id, check_url_validity, origin) => { - spawn_named("read file".to_owned(), move || { - if let Err(e) = store.try_read_file(&sender, id, check_url_validity, - origin, cancel_listener) { - let _ = sender.send(Err(FileManagerThreadError::BlobURLStoreError(e))); - } - }) + self.read_file(sender, id, check_url_validity, origin, cancel_listener); } FileManagerThreadMsg::PromoteMemory(blob_buf, set_valid, sender, origin) => { - spawn_named("transfer memory".to_owned(), move || { - store.promote_memory(blob_buf, set_valid, sender, origin); - }) + self.promote_memory(blob_buf, set_valid, sender, origin); } FileManagerThreadMsg::AddSlicedURLEntry(id, rel_pos, sender, origin) =>{ - store.add_sliced_url_entry(id, rel_pos, sender, origin); + self.store.add_sliced_url_entry(id, rel_pos, sender, origin); } FileManagerThreadMsg::DecRef(id, origin, sender) => { - let _ = sender.send(store.dec_ref(&id, &origin)); + let _ = sender.send(self.store.dec_ref(&id, &origin)); } FileManagerThreadMsg::RevokeBlobURL(id, origin, sender) => { - let _ = sender.send(store.set_blob_url_validity(false, &id, &origin)); + let _ = sender.send(self.store.set_blob_url_validity(false, &id, &origin)); } FileManagerThreadMsg::ActivateBlobURL(id, sender, origin) => { - let _ = sender.send(store.set_blob_url_validity(true, &id, &origin)); + let _ = sender.send(self.store.set_blob_url_validity(true, &id, &origin)); } } } @@ -178,16 +195,14 @@ impl FileManager { /// File manager's data store. It maintains a thread-safe mapping /// from FileID to FileStoreEntry which might have different backend implementation. /// Access to the content is encapsulated as methods of this struct. -struct FileManagerStore { +struct FileManagerStore { entries: RwLock>, - ui: &'static UI, } -impl FileManagerStore { - fn new(ui: &'static UI) -> Self { +impl FileManagerStore { + fn new() -> Self { FileManagerStore { entries: RwLock::new(HashMap::new()), - ui: ui, } } @@ -257,16 +272,21 @@ impl FileManagerStore { } } - fn select_file(&self, patterns: Vec, - sender: IpcSender>, - origin: FileOrigin, opt_test_path: Option) { + fn select_file(&self, + patterns: Vec, + sender: IpcSender>, + origin: FileOrigin, + opt_test_path: Option, + ui: &UI) + where UI: UIProvider, + { // Check if the select_files preference is enabled // to ensure process-level security against compromised script; // Then try applying opt_test_path directly for testing convenience let opt_s = if select_files_pref_enabled() { opt_test_path } else { - self.ui.open_file_dialog("", patterns) + ui.open_file_dialog("", patterns) }; match opt_s { @@ -282,16 +302,21 @@ impl FileManagerStore { } } - fn select_files(&self, patterns: Vec, - sender: IpcSender>>, - origin: FileOrigin, opt_test_paths: Option>) { + fn select_files(&self, + patterns: Vec, + sender: IpcSender>>, + origin: FileOrigin, + opt_test_paths: Option>, + ui: &UI) + where UI: UIProvider, + { // Check if the select_files preference is enabled // to ensure process-level security against compromised script; // Then try applying opt_test_paths directly for testing convenience let opt_v = if select_files_pref_enabled() { opt_test_paths } else { - self.ui.open_file_dialog_multi("", patterns) + ui.open_file_dialog_multi("", patterns) }; match opt_v { diff --git a/components/net/resource_thread.rs b/components/net/resource_thread.rs index aaef8b68773..17835b52747 100644 --- a/components/net/resource_thread.rs +++ b/components/net/resource_thread.rs @@ -285,7 +285,7 @@ impl ResourceChannelManager { CoreResourceMsg::Synchronize(sender) => { let _ = sender.send(()); } - CoreResourceMsg::ToFileManager(msg) => self.resource_manager.filemanager.handle(msg, None), + CoreResourceMsg::ToFileManager(msg) => self.resource_manager.filemanager.handle(msg, None, TFD_PROVIDER), CoreResourceMsg::Exit(sender) => { if let Some(ref config_dir) = self.config_dir { match group.auth_cache.read() { @@ -455,7 +455,7 @@ pub struct CoreResourceManager { devtools_chan: Option>, swmanager_chan: Option>, profiler_chan: ProfilerChan, - filemanager: FileManager, + filemanager: FileManager, cancel_load_map: HashMap>, next_resource_id: ResourceId, } @@ -470,7 +470,7 @@ impl CoreResourceManager { devtools_chan: devtools_channel, swmanager_chan: None, profiler_chan: profiler_chan, - filemanager: FileManager::new(TFD_PROVIDER), + filemanager: FileManager::new(), cancel_load_map: HashMap::new(), next_resource_id: ResourceId(0), } diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 2b973b6f774..be1878667a4 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -98,7 +98,6 @@ fn test_fetch_aboutblank() { fn test_fetch_blob() { use ipc_channel::ipc; use net_traits::blob_url_store::BlobBuf; - use net_traits::filemanager_thread::FileManagerThreadMsg; let context = new_fetch_context(None); @@ -113,8 +112,7 @@ fn test_fetch_blob() { let origin = ServoUrl::parse("http://www.example.org/").unwrap(); let (sender, receiver) = ipc::channel().unwrap(); - let message = FileManagerThreadMsg::PromoteMemory(blob_buf, true, sender, "http://www.example.org".into()); - context.filemanager.handle(message, None); + context.filemanager.promote_memory(blob_buf, true, sender, "http://www.example.org".into()); let id = receiver.recv().unwrap().unwrap(); let url = ServoUrl::parse(&format!("blob:{}{}", origin.as_str(), id.simple())).unwrap(); diff --git a/tests/unit/net/filemanager_thread.rs b/tests/unit/net/filemanager_thread.rs index 75facb0b6aa..fbbe4aa4ca4 100644 --- a/tests/unit/net/filemanager_thread.rs +++ b/tests/unit/net/filemanager_thread.rs @@ -26,7 +26,7 @@ impl UIProvider for TestProvider { #[test] fn test_filemanager() { - let filemanager = FileManager::new(TEST_PROVIDER); + let filemanager = FileManager::new(); // Try to open a dummy file "tests/unit/net/test.jpeg" in tree let mut handler = File::open("test.jpeg").expect("test.jpeg is stolen"); @@ -41,7 +41,8 @@ fn test_filemanager() { { // Try to select a dummy file "tests/unit/net/test.jpeg" let (tx, rx) = ipc::channel().unwrap(); - filemanager.handle(FileManagerThreadMsg::SelectFile(patterns.clone(), tx, origin.clone(), None), None); + filemanager.handle(FileManagerThreadMsg::SelectFile(patterns.clone(), tx, origin.clone(), None), + None, TEST_PROVIDER); let selected = rx.recv().expect("Broken channel") .expect("The file manager failed to find test.jpeg"); @@ -52,7 +53,8 @@ fn test_filemanager() { // Test by reading, expecting same content { let (tx2, rx2) = ipc::channel().unwrap(); - filemanager.handle(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), false, origin.clone()), None); + filemanager.handle(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), false, origin.clone()), + None, TEST_PROVIDER); let msg = rx2.recv().expect("Broken channel"); @@ -82,7 +84,8 @@ fn test_filemanager() { // Delete the id { let (tx2, rx2) = ipc::channel().unwrap(); - filemanager.handle(FileManagerThreadMsg::DecRef(selected.id.clone(), origin.clone(), tx2), None); + filemanager.handle(FileManagerThreadMsg::DecRef(selected.id.clone(), origin.clone(), tx2), + None, TEST_PROVIDER); let ret = rx2.recv().expect("Broken channel"); assert!(ret.is_ok(), "DecRef is not okay"); @@ -91,7 +94,8 @@ fn test_filemanager() { // Test by reading again, expecting read error because we invalidated the id { let (tx2, rx2) = ipc::channel().unwrap(); - filemanager.handle(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), false, origin.clone()), None); + filemanager.handle(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), false, origin.clone()), + None, TEST_PROVIDER); let msg = rx2.recv().expect("Broken channel"); diff --git a/tests/unit/net/lib.rs b/tests/unit/net/lib.rs index 400356c6f3b..c9890544d94 100644 --- a/tests/unit/net/lib.rs +++ b/tests/unit/net/lib.rs @@ -35,7 +35,6 @@ extern crate util; #[cfg(test)] mod filemanager_thread; use devtools_traits::DevtoolsControlMsg; -use filemanager_thread::{TestProvider, TEST_PROVIDER}; use hyper::server::{Handler, Listening, Server}; use net::fetch::methods::{FetchContext, fetch}; use net::filemanager_thread::FileManager; @@ -54,12 +53,12 @@ struct FetchResponseCollector { sender: Sender, } -fn new_fetch_context(dc: Option>) -> FetchContext { +fn new_fetch_context(dc: Option>) -> FetchContext { FetchContext { state: HttpState::new(), user_agent: DEFAULT_USER_AGENT.into(), devtools_chan: dc, - filemanager: FileManager::new(TEST_PROVIDER), + filemanager: FileManager::new(), } } impl FetchTaskTarget for FetchResponseCollector {