From 2fab94785becfc5a7c8922d283be426795a43b6d Mon Sep 17 00:00:00 2001 From: OJ Kwon Date: Sat, 14 Apr 2018 10:29:09 -0700 Subject: [PATCH] refactor(filemanager): uses embedderproxy directly --- Cargo.lock | 2 +- components/constellation/constellation.rs | 34 ++------------- components/net/Cargo.toml | 2 +- components/net/filemanager_thread.rs | 50 +++++++++++------------ components/net/lib.rs | 2 +- components/net/resource_thread.rs | 24 +++++------ components/script_traits/lib.rs | 2 +- components/script_traits/script_msg.rs | 8 ---- components/servo/lib.rs | 7 ++-- ports/servo/browser.rs | 7 +++- 10 files changed, 54 insertions(+), 84 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8b982fcc0e..9d8da41369e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,6 +1890,7 @@ version = "0.0.1" dependencies = [ "base64 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "brotli 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "compositing 0.0.1", "cookie 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", "devtools_traits 0.0.1", "embedder_traits 0.0.1", @@ -1910,7 +1911,6 @@ dependencies = [ "net_traits 0.0.1", "openssl 0.9.24 (registry+https://github.com/rust-lang/crates.io-index)", "profile_traits 0.0.1", - "script_traits 0.0.1", "serde 1.0.37 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)", "servo-websocket 0.21.0 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 0f1e6a37393..93581d9f884 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -125,10 +125,10 @@ use profile_traits::time; use script_traits::{AnimationState, AnimationTickType, CompositorEvent}; use script_traits::{ConstellationControlMsg, ConstellationMsg as FromCompositorMsg, DiscardBrowsingContext}; use script_traits::{DocumentActivity, DocumentState, LayoutControlMsg, LoadData}; -use script_traits::{FileManagerMsg, SWManagerMsg, ScopeThings, UpdatePipelineIdReason, WebDriverCommandMsg}; use script_traits::{IFrameLoadInfo, IFrameLoadInfoWithData, IFrameSandboxState, TimerSchedulerMsg}; use script_traits::{LayoutMsg as FromLayoutMsg, ScriptMsg as FromScriptMsg, ScriptThreadFactory}; use script_traits::{LogEntry, ScriptToConstellationChan, ServiceWorkerMsg, webdriver_msg}; +use script_traits::{SWManagerMsg, ScopeThings, UpdatePipelineIdReason, WebDriverCommandMsg}; use script_traits::{WindowSizeData, WindowSizeType}; use serde::{Deserialize, Serialize}; use servo_config::opts; @@ -174,10 +174,6 @@ pub struct Constellation { /// This is the constellation's view of `script_sender`. script_receiver: Receiver>, - /// A channel for the constellation to receive messages from filemanager threads. - /// This is the constellation's view of `filemanager_sender`. - filemanager_receiver: Receiver>, - /// An IPC channel for layout threads to send messages to the constellation. /// This is the layout threads' view of `layout_receiver`. layout_sender: IpcSender, @@ -550,22 +546,17 @@ impl Constellation STF: ScriptThreadFactory { /// Create a new constellation thread. - pub fn start(state: InitialConstellationState) - -> (Sender, IpcSender, IpcSender) { + pub fn start(state: InitialConstellationState) -> (Sender, IpcSender) { let (compositor_sender, compositor_receiver) = channel(); // service worker manager to communicate with constellation let (swmanager_sender, swmanager_receiver) = ipc::channel().expect("ipc channel failure"); let sw_mgr_clone = swmanager_sender.clone(); - let (filemanager_sender, filemanager_receiver) = ipc::channel().expect("ipc channel failure"); - thread::Builder::new().name("Constellation".to_owned()).spawn(move || { let (ipc_script_sender, ipc_script_receiver) = ipc::channel().expect("ipc channel failure"); let script_receiver = route_ipc_receiver_to_new_mpsc_receiver_preserving_errors(ipc_script_receiver); - let filemanager_receiver = route_ipc_receiver_to_new_mpsc_receiver_preserving_errors(filemanager_receiver); - let (ipc_layout_sender, ipc_layout_receiver) = ipc::channel().expect("ipc channel failure"); let layout_receiver = route_ipc_receiver_to_new_mpsc_receiver_preserving_errors(ipc_layout_receiver); @@ -579,7 +570,6 @@ impl Constellation script_sender: ipc_script_sender, layout_sender: ipc_layout_sender, script_receiver: script_receiver, - filemanager_receiver: filemanager_receiver, compositor_receiver: compositor_receiver, layout_receiver: layout_receiver, network_listener_sender: network_listener_sender, @@ -646,7 +636,7 @@ impl Constellation constellation.run(); }).expect("Thread spawning failed"); - (compositor_sender, swmanager_sender, filemanager_sender) + (compositor_sender, swmanager_sender) } /// The main event loop for the constellation. @@ -840,7 +830,6 @@ impl Constellation Layout(FromLayoutMsg), NetworkListener((PipelineId, FetchResponseMsg)), FromSWManager(SWManagerMsg), - FromFileManager(FileManagerMsg), } // Get one incoming request. @@ -860,7 +849,6 @@ impl Constellation let receiver_from_layout = &self.layout_receiver; let receiver_from_network_listener = &self.network_listener_receiver; let receiver_from_swmanager = &self.swmanager_receiver; - let receiver_from_filemanager = &self.filemanager_receiver; select! { msg = receiver_from_script.recv() => msg.expect("Unexpected script channel panic in constellation").map(Request::Script), @@ -873,9 +861,7 @@ impl Constellation msg.expect("Unexpected network listener channel panic in constellation") )), msg = receiver_from_swmanager.recv() => - msg.expect("Unexpected panic channel panic in constellation").map(Request::FromSWManager), - msg = receiver_from_filemanager.recv() => - msg.expect("Unexpected file manager channel panic in constellation").map(Request::FromFileManager) + msg.expect("Unexpected panic channel panic in constellation").map(Request::FromSWManager) } }; @@ -899,9 +885,6 @@ impl Constellation }, Request::FromSWManager(message) => { self.handle_request_from_swmanager(message); - }, - Request::FromFileManager(message) => { - self.handle_request_from_filemanager(message); } } } @@ -931,15 +914,6 @@ impl Constellation } } - fn handle_request_from_filemanager(&mut self, message: FileManagerMsg) { - match message { - FileManagerMsg::OpenFileSelectDialog(patterns, multiple_files, sender) => { - let msg = EmbedderMsg::GetSelectedFiles(patterns, multiple_files, sender); - self.embedder_proxy.send(msg); - } - } - } - fn handle_request_from_compositor(&mut self, message: FromCompositorMsg) { match message { FromCompositorMsg::Exit => { diff --git a/components/net/Cargo.toml b/components/net/Cargo.toml index 7f1c402a4c0..4aaeecf75d9 100644 --- a/components/net/Cargo.toml +++ b/components/net/Cargo.toml @@ -15,6 +15,7 @@ doctest = false base64 = "0.6" brotli = "1.0.6" cookie = "0.10" +compositing = {path = "../compositing"} devtools_traits = {path = "../devtools_traits"} embedder_traits = { path = "../embedder_traits" } flate2 = "1" @@ -34,7 +35,6 @@ msg = {path = "../msg"} net_traits = {path = "../net_traits"} openssl = "0.9" profile_traits = {path = "../profile_traits"} -script_traits = {path = "../script_traits"} serde = "1.0" serde_json = "1.0" servo_allocator = {path = "../allocator"} diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs index 5c59a6aae8c..0e5d8dafd75 100644 --- a/components/net/filemanager_thread.rs +++ b/components/net/filemanager_thread.rs @@ -2,14 +2,12 @@ * 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 compositing::compositor_thread::{EmbedderMsg, EmbedderProxy}; use ipc_channel::ipc::{self, IpcSender}; use mime_guess::guess_mime_type_opt; use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError}; use net_traits::filemanager_thread::{FileManagerResult, FileManagerThreadMsg, FileOrigin, FilterPattern}; use net_traits::filemanager_thread::{FileManagerThreadError, ReadFileProgress, RelativePos, SelectedFile}; -use script_traits::FileManagerMsg; -#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] -use servo_config::opts; use servo_config::prefs::PREFS; use std::collections::HashMap; use std::fs::File; @@ -19,7 +17,6 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use std::sync::atomic::{self, AtomicBool, AtomicUsize, Ordering}; use std::thread; -#[cfg(any(target_os = "macos", target_os = "linux", target_os = "windows"))] use url::Url; use uuid::Uuid; @@ -62,14 +59,14 @@ enum FileImpl { #[derive(Clone)] pub struct FileManager { - constellation_chan: IpcSender, + embedder_proxy: EmbedderProxy, store: Arc, } impl FileManager { - pub fn new(constellation_chan: IpcSender) -> FileManager { + pub fn new(embedder_proxy: EmbedderProxy) -> FileManager { FileManager { - constellation_chan: constellation_chan, + embedder_proxy: embedder_proxy, store: Arc::new(FileManagerStore::new()), } } @@ -104,16 +101,16 @@ impl FileManager { match msg { FileManagerThreadMsg::SelectFile(filter, sender, origin, opt_test_path) => { let store = self.store.clone(); - let constellation_chan = self.constellation_chan.clone(); + let embedder = self.embedder_proxy.clone(); thread::Builder::new().name("select file".to_owned()).spawn(move || { - store.select_file(filter, sender, origin, opt_test_path, constellation_chan); + store.select_file(filter, sender, origin, opt_test_path, embedder); }).expect("Thread spawning failed"); } FileManagerThreadMsg::SelectFiles(filter, sender, origin, opt_test_paths) => { let store = self.store.clone(); - let constellation_chan = self.constellation_chan.clone(); + let embedder = self.embedder_proxy.clone(); thread::Builder::new().name("select files".to_owned()).spawn(move || { - store.select_files(filter, sender, origin, opt_test_paths, constellation_chan); + store.select_files(filter, sender, origin, opt_test_paths, embedder); }).expect("Thread spawning failed"); } FileManagerThreadMsg::ReadFile(sender, id, check_url_validity, origin) => { @@ -218,18 +215,21 @@ impl FileManagerStore { } } - fn get_selected_files(&self, - patterns: Vec, - multiple_files: bool, - constellation_chan: IpcSender) -> Option> { - if opts::get().headless { - return None; - } - + fn query_files_from_embedder(&self, + patterns: Vec, + multiple_files: bool, + embedder_proxy: EmbedderProxy) -> Option> { let (ipc_sender, ipc_receiver) = ipc::channel().expect("Failed to create IPC channel!"); - let msg = FileManagerMsg::OpenFileSelectDialog(patterns, multiple_files, ipc_sender); + let msg = EmbedderMsg::GetSelectedFiles(patterns, multiple_files, ipc_sender); - constellation_chan.send(msg).map(|_| ipc_receiver.recv().unwrap()).unwrap_or_default() + embedder_proxy.send(msg); + match ipc_receiver.recv() { + Ok(result) => result, + Err(e) => { + warn!("Failed to receive files from embedder ({}).", e); + None + } + } } fn select_file(&self, @@ -237,14 +237,14 @@ impl FileManagerStore { sender: IpcSender>, origin: FileOrigin, opt_test_path: Option, - constellation_chan: IpcSender) { + embedder_proxy: EmbedderProxy) { // 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.get_selected_files(patterns, false, constellation_chan).map(|mut x| x.pop().unwrap()) + self.query_files_from_embedder(patterns, false, embedder_proxy).and_then(|mut x| x.pop()) }; match opt_s { @@ -265,14 +265,14 @@ impl FileManagerStore { sender: IpcSender>>, origin: FileOrigin, opt_test_paths: Option>, - constellation_chan: IpcSender) { + embedder_proxy: EmbedderProxy) { // 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.get_selected_files(patterns, true, constellation_chan) + self.query_files_from_embedder(patterns, true, embedder_proxy) }; match opt_v { diff --git a/components/net/lib.rs b/components/net/lib.rs index 6bfde683494..a8d802ae49f 100644 --- a/components/net/lib.rs +++ b/components/net/lib.rs @@ -6,6 +6,7 @@ extern crate base64; extern crate brotli; +extern crate compositing; extern crate cookie as cookie_rs; extern crate devtools_traits; extern crate embedder_traits; @@ -29,7 +30,6 @@ extern crate net_traits; extern crate openssl; #[macro_use] extern crate profile_traits; -extern crate script_traits; #[macro_use] extern crate serde; extern crate serde_json; extern crate servo_allocator; diff --git a/components/net/resource_thread.rs b/components/net/resource_thread.rs index 3f3e8c10843..d00c0323f38 100644 --- a/components/net/resource_thread.rs +++ b/components/net/resource_thread.rs @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ //! A thread that takes a URL and streams back the binary data. +use compositing::compositor_thread::EmbedderProxy; use connector::{create_http_connector, create_ssl_client}; use cookie; use cookie_rs; @@ -28,7 +29,6 @@ use net_traits::storage_thread::StorageThreadMsg; use profile_traits::mem::{Report, ReportsChan, ReportKind}; use profile_traits::mem::ProfilerChan as MemProfilerChan; use profile_traits::time::ProfilerChan; -use script_traits::FileManagerMsg; use serde::{Deserialize, Serialize}; use serde_json; use servo_allocator; @@ -52,18 +52,19 @@ pub fn new_resource_threads(user_agent: Cow<'static, str>, devtools_chan: Option>, time_profiler_chan: ProfilerChan, mem_profiler_chan: MemProfilerChan, + embedder_proxy: EmbedderProxy, config_dir: Option) - -> (ResourceThreads, ResourceThreads, IpcSender>) { - let (public_core, private_core, constellation_sender) = new_core_resource_thread( + -> (ResourceThreads, ResourceThreads) { + let (public_core, private_core) = new_core_resource_thread( user_agent, devtools_chan, time_profiler_chan, mem_profiler_chan, + embedder_proxy, config_dir.clone()); let storage: IpcSender = StorageThreadFactory::new(config_dir); (ResourceThreads::new(public_core, storage.clone()), - ResourceThreads::new(private_core, storage), - constellation_sender) + ResourceThreads::new(private_core, storage)) } @@ -72,17 +73,16 @@ pub fn new_core_resource_thread(user_agent: Cow<'static, str>, devtools_chan: Option>, time_profiler_chan: ProfilerChan, mem_profiler_chan: MemProfilerChan, + embedder_proxy: EmbedderProxy, config_dir: Option) - -> (CoreResourceThread, CoreResourceThread, IpcSender>) { + -> (CoreResourceThread, CoreResourceThread) { let (public_setup_chan, public_setup_port) = ipc::channel().unwrap(); let (private_setup_chan, private_setup_port) = ipc::channel().unwrap(); let (report_chan, report_port) = ipc::channel().unwrap(); - let (constellation_sender, constellation_receiver) = ipc::channel().unwrap(); thread::Builder::new().name("ResourceManager".to_owned()).spawn(move || { - let constellation_chan = constellation_receiver.recv().unwrap(); let resource_manager = CoreResourceManager::new( - user_agent, devtools_chan, time_profiler_chan, constellation_chan + user_agent, devtools_chan, time_profiler_chan, embedder_proxy ); let mut channel_manager = ResourceChannelManager { @@ -101,7 +101,7 @@ pub fn new_core_resource_thread(user_agent: Cow<'static, str>, |report_chan| report_chan); }).expect("Thread spawning failed"); - (public_setup_chan, private_setup_chan, constellation_sender) + (public_setup_chan, private_setup_chan) } struct ResourceChannelManager { @@ -377,12 +377,12 @@ impl CoreResourceManager { pub fn new(user_agent: Cow<'static, str>, devtools_channel: Option>, _profiler_chan: ProfilerChan, - constellation_chan: IpcSender) -> CoreResourceManager { + embedder_proxy: EmbedderProxy) -> CoreResourceManager { CoreResourceManager { user_agent: user_agent, devtools_chan: devtools_channel, swmanager_chan: None, - filemanager: FileManager::new(constellation_chan), + filemanager: FileManager::new(embedder_proxy), } } diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index b2cf6a97e43..272cfe3cedb 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -73,7 +73,7 @@ use webrender_api::{ExternalScrollId, DevicePixel, DeviceUintSize, DocumentId, I use webvr_traits::{WebVREvent, WebVRMsg}; pub use script_msg::{LayoutMsg, ScriptMsg, EventResult, LogEntry}; -pub use script_msg::{FileManagerMsg, ServiceWorkerMsg, ScopeThings, SWManagerMsg, SWManagerSenders, DOMMessage}; +pub use script_msg::{ServiceWorkerMsg, ScopeThings, SWManagerMsg, SWManagerSenders, DOMMessage}; /// The address of a node. Layout sends these back. They must be validated via /// `from_untrusted_node_address` before they can be used, because we do not trust layout. diff --git a/components/script_traits/script_msg.rs b/components/script_traits/script_msg.rs index 9df1e7ee077..d263753c7de 100644 --- a/components/script_traits/script_msg.rs +++ b/components/script_traits/script_msg.rs @@ -19,7 +19,6 @@ use ipc_channel::ipc::{IpcReceiver, IpcSender}; use msg::constellation_msg::{BrowsingContextId, HistoryStateId, PipelineId, TraversalDirection}; use msg::constellation_msg::{InputMethodType, Key, KeyModifiers, KeyState}; use net_traits::CoreResourceMsg; -use net_traits::filemanager_thread::FilterPattern; use net_traits::request::RequestInit; use net_traits::storage_thread::StorageType; use servo_url::ImmutableOrigin; @@ -218,10 +217,3 @@ pub enum SWManagerMsg { /// Provide the constellation with a means of communicating with the Service Worker Manager OwnSender(IpcSender), } - -/// Messages outgoing from the File Manager thread to constellation -#[derive(Deserialize, Serialize)] -pub enum FileManagerMsg { - /// Requesting to open file select dialog - OpenFileSelectDialog(Vec, bool, IpcSender>>) -} diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 65449d0d9af..bc06dc84ca7 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -457,11 +457,12 @@ fn create_constellation(user_agent: Cow<'static, str>, -> (Sender, SWManagerSenders) { let bluetooth_thread: IpcSender = BluetoothThreadFactory::new(embedder_proxy.clone()); - let (public_resource_threads, private_resource_threads, resource_constellation_sender) = + let (public_resource_threads, private_resource_threads) = new_resource_threads(user_agent, devtools_chan.clone(), time_profiler_chan.clone(), mem_profiler_chan.clone(), + embedder_proxy.clone(), config_dir); let font_cache_thread = FontCacheThread::new(public_resource_threads.sender(), webrender_api_sender.create_api()); @@ -523,7 +524,7 @@ fn create_constellation(user_agent: Cow<'static, str>, webgl_threads, webvr_chan, }; - let (constellation_chan, from_swmanager_sender, from_filemanager_sender) = + let (constellation_chan, from_swmanager_sender) = Constellation::::start(initial_state); @@ -533,8 +534,6 @@ fn create_constellation(user_agent: Cow<'static, str>, webvr_constellation_sender.send(constellation_chan.clone()).unwrap(); } - resource_constellation_sender.send(from_filemanager_sender.clone()).unwrap(); - // channels to communicate with Service Worker Manager let sw_senders = SWManagerSenders { swmanager_sender: from_swmanager_sender, diff --git a/ports/servo/browser.rs b/ports/servo/browser.rs index b1f2627fb68..62028867c94 100644 --- a/ports/servo/browser.rs +++ b/ports/servo/browser.rs @@ -13,6 +13,7 @@ use servo::msg::constellation_msg::{KeyModifiers, KeyState, TraversalDirection}; use servo::net_traits::filemanager_thread::FilterPattern; use servo::net_traits::pub_domains::is_reg_domain; use servo::script_traits::TouchEventType; +use servo::servo_config::opts; use servo::servo_config::prefs::PREFS; use servo::servo_url::ServoUrl; use servo::webrender_api::ScrollLocation; @@ -297,6 +298,9 @@ impl Browser { platform_get_selected_devices(devices, sender); }, EmbedderMsg::GetSelectedFiles(patterns, multiple_files, sender) => { + if opts::get().headless { + let _ = sender.send(None); + } platform_get_selected_files(patterns, multiple_files, sender); } EmbedderMsg::ShowIME(_browser_id, _kind) => { @@ -374,7 +378,8 @@ fn platform_get_selected_files(patterns: Vec, fn platform_get_selected_files(_patterns: Vec, _multiple_files: bool, sender: IpcSender>>) { - sender.send(None); + warn!("File picker not implemented"); + let _ = sender.send(None); } fn sanitize_url(request: &str) -> Option {