Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the dependency of fetch on filemanager_thread::UIProvider. #14275

Merged
merged 5 commits into from Nov 22, 2016
Prev

Pass the UIProvider to FileManager::handle() as needed.

  • Loading branch information
Ms2ger committed Nov 21, 2016
commit ae1340bf506eeef71c026e3da10b010eca59d379
@@ -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;
@@ -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,10 +33,9 @@ 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()) {
@@ -122,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),
@@ -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());
@@ -112,26 +112,15 @@ enum FileImpl {
Sliced(Uuid, RelativePos),
}

pub struct FileManager<UI: 'static + UIProvider> {
#[derive(Clone)]

This comment has been minimized.

@jdm

jdm Nov 21, 2016

Member

Is this necessary?

pub struct FileManager {
store: Arc<FileManagerStore>,
ui: &'static UI,
}

// Not derived to avoid an unnecessary `UI: Clone` bound.
impl<UI: 'static + UIProvider> Clone for FileManager<UI> {
fn clone(&self) -> Self {
FileManager {
store: self.store.clone(),
ui: self.ui,
}
}
}

impl<UI: 'static + UIProvider> FileManager<UI> {
pub fn new(ui: &'static UI) -> FileManager<UI> {
impl FileManager {
pub fn new() -> FileManager {
FileManager {
store: Arc::new(FileManagerStore::new()),
ui: ui,
}
}

@@ -162,18 +151,21 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
}

/// Message handler
pub fn handle(&self, msg: FileManagerThreadMsg, cancel_listener: Option<CancellationListener>) {
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();
let ui = self.ui;
spawn_named("select file".to_owned(), move || {
store.select_file(filter, sender, origin, opt_test_path, ui);
});
}
FileManagerThreadMsg::SelectFiles(filter, sender, origin, opt_test_paths) => {
let store = self.store.clone();
let ui = self.ui;
spawn_named("select files".to_owned(), move || {
store.select_files(filter, sender, origin, opt_test_paths, ui);
})
@@ -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),
}
@@ -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");

@@ -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 {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.