Auto merge of #14275 - servo:fm-ui, r=jdm

Remove the dependency of fetch on filemanager_thread::UIProvider.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14275)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-11-21 18:28:19 -06:00 committed by GitHub
commit d287f58d3f
7 changed files with 149 additions and 126 deletions

View file

@ -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<UI: 'static + UIProvider>(filemanager: FileManager<UI>)
pub fn factory(filemanager: FileManager)
-> Box<FnBox(LoadData, LoadConsumer, Arc<MimeClassifier>, 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<UI: 'static + UIProvider>(filemanager: FileManager<UI>)
}
}
fn load_blob<UI: 'static + UIProvider>
(load_data: LoadData, start_chan: LoadConsumer,
fn load_blob(load_data: LoadData, start_chan: LoadConsumer,
classifier: Arc<MimeClassifier>,
filemanager: FileManager<UI>,
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<UI: 'static + UIProvider>
/// https://fetch.spec.whatwg.org/#concept-basic-fetch (partial)
// TODO: make async.
pub fn load_blob_sync<UI: 'static + UIProvider>
pub fn load_blob_sync
(url: ServoUrl,
filemanager: FileManager<UI>)
filemanager: FileManager)
-> Result<(Headers, Vec<u8>), NetworkError> {
let (id, origin) = match parse_blob_url(&url) {
Ok((id, origin, _fragment)) => (id, origin),
@ -137,8 +135,7 @@ pub fn load_blob_sync<UI: 'static + UIProvider>
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,

View file

@ -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<UI: 'static + UIProvider> {
pub struct FetchContext {
pub state: HttpState,
pub user_agent: Cow<'static, str>,
pub devtools_chan: Option<Sender<DevtoolsControlMsg>>,
pub filemanager: FileManager<UI>,
pub filemanager: FileManager,
}
type DoneChannel = Option<(Sender<Data>, Receiver<Data>)>;
/// [Fetch](https://fetch.spec.whatwg.org#concept-fetch)
pub fn fetch<UI: 'static + UIProvider>(request: Rc<Request>,
target: &mut Target,
context: &FetchContext<UI>)
-> Response {
pub fn fetch(request: Rc<Request>,
target: &mut Target,
context: &FetchContext)
-> Response {
fetch_with_cors_cache(request, &mut CorsCache::new(), target, context)
}
pub fn fetch_with_cors_cache<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
target: &mut Target,
context: &FetchContext<UI>)
-> Response {
pub fn fetch_with_cors_cache(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch)
fn main_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
cors_flag: bool,
recursive_flag: bool,
target: &mut Target,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn main_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [Basic fetch](https://fetch.spec.whatwg.org#basic-fetch)
fn basic_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
target: &mut Target,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn basic_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [HTTP fetch](https://fetch.spec.whatwg.org#http-fetch)
fn http_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
cors_flag: bool,
cors_preflight_flag: bool,
authentication_fetch_flag: bool,
target: &mut Target,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn http_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [HTTP redirect fetch](https://fetch.spec.whatwg.org#http-redirect-fetch)
fn http_redirect_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
response: Response,
cors_flag: bool,
target: &mut Target,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn http_redirect_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [HTTP network or cache fetch](https://fetch.spec.whatwg.org#http-network-or-cache-fetch)
fn http_network_or_cache_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
credentials_flag: bool,
authentication_fetch_flag: bool,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn http_network_or_cache_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [HTTP network fetch](https://fetch.spec.whatwg.org/#http-network-fetch)
fn http_network_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
credentials_flag: bool,
done_chan: &mut DoneChannel,
context: &FetchContext<UI>)
-> Response {
fn http_network_fetch(request: Rc<Request>,
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<UI: 'static + UIProvider>(request: Rc<Request>,
}
/// [CORS preflight fetch](https://fetch.spec.whatwg.org#cors-preflight-fetch)
fn cors_preflight_fetch<UI: 'static + UIProvider>(request: Rc<Request>,
cache: &mut CorsCache,
context: &FetchContext<UI>)
-> Response {
fn cors_preflight_fetch(request: Rc<Request>,
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());

View file

@ -112,64 +112,81 @@ enum FileImpl {
Sliced(Uuid, RelativePos),
}
pub struct FileManager<UI: 'static + UIProvider> {
store: Arc<FileManagerStore<UI>>,
#[derive(Clone)]
pub struct FileManager {
store: Arc<FileManagerStore>,
}
// Not derived to avoid an unnecessary `UI: Clone` bound.
impl<UI: 'static + UIProvider> Clone for FileManager<UI> {
fn clone(&self) -> Self {
impl FileManager {
pub fn new() -> FileManager {
FileManager {
store: self.store.clone(),
store: Arc::new(FileManagerStore::new()),
}
}
}
impl<UI: 'static + UIProvider> FileManager<UI> {
pub fn new(ui: &'static UI) -> FileManager<UI> {
FileManager {
store: Arc::new(FileManagerStore::new(ui)),
}
pub fn read_file(&self,
sender: IpcSender<FileManagerResult<ReadFileProgress>>,
id: Uuid,
check_url_validity: bool,
origin: FileOrigin,
cancel_listener: Option<CancellationListener>) {
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<Result<Uuid, BlobURLStoreError>>,
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<CancellationListener>) {
let store = self.store.clone();
pub fn handle<UI>(&self,
msg: FileManagerThreadMsg,
cancel_listener: Option<CancellationListener>,
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<UI: 'static + UIProvider> FileManager<UI> {
/// 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<UI: 'static + UIProvider> {
struct FileManagerStore {
entries: RwLock<HashMap<Uuid, FileStoreEntry>>,
ui: &'static UI,
}
impl <UI: 'static + UIProvider> FileManagerStore<UI> {
fn new(ui: &'static UI) -> Self {
impl FileManagerStore {
fn new() -> Self {
FileManagerStore {
entries: RwLock::new(HashMap::new()),
ui: ui,
}
}
@ -257,16 +272,21 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
}
}
fn select_file(&self, patterns: Vec<FilterPattern>,
sender: IpcSender<FileManagerResult<SelectedFile>>,
origin: FileOrigin, opt_test_path: Option<String>) {
fn select_file<UI>(&self,
patterns: Vec<FilterPattern>,
sender: IpcSender<FileManagerResult<SelectedFile>>,
origin: FileOrigin,
opt_test_path: Option<String>,
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 <UI: 'static + UIProvider> FileManagerStore<UI> {
}
}
fn select_files(&self, patterns: Vec<FilterPattern>,
sender: IpcSender<FileManagerResult<Vec<SelectedFile>>>,
origin: FileOrigin, opt_test_paths: Option<Vec<String>>) {
fn select_files<UI>(&self,
patterns: Vec<FilterPattern>,
sender: IpcSender<FileManagerResult<Vec<SelectedFile>>>,
origin: FileOrigin,
opt_test_paths: Option<Vec<String>>,
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 {

View file

@ -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<Sender<DevtoolsControlMsg>>,
swmanager_chan: Option<IpcSender<CustomResponseMediator>>,
profiler_chan: ProfilerChan,
filemanager: FileManager<TFDProvider>,
filemanager: FileManager,
cancel_load_map: HashMap<ResourceId, Sender<()>>,
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),
}

View file

@ -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();

View file

@ -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");

View file

@ -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<Response>,
}
fn new_fetch_context(dc: Option<Sender<DevtoolsControlMsg>>) -> FetchContext<TestProvider> {
fn new_fetch_context(dc: Option<Sender<DevtoolsControlMsg>>) -> 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 {