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

Read file URL in chunks #21560

Merged
merged 3 commits into from Sep 7, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Next

Read file in chunks and send chunks to FetchTaskTarget instead of rea…

…d_to_end call
  • Loading branch information
InquisitivePenguin committed Aug 28, 2018
commit f04c965a9bab46ff6943416baab338032bc526c1
@@ -25,14 +25,16 @@ use servo_url::ServoUrl;
use std::borrow::Cow;
use std::fmt;
use std::fs::File;
use std::io::Read;
use std::io::{BufReader, BufRead};
use std::mem;
use std::str;
use std::sync::{Arc, Mutex};
use std::sync::atomic::Ordering;
use std::sync::mpsc::{Sender, Receiver};
use subresource_integrity::is_response_integrity_valid;

const FILE_CHUNK_SIZE: usize = 32768; //32 KB

pub type Target<'a> = &'a mut (FetchTaskTarget + Send);

pub enum Data {
@@ -486,8 +488,20 @@ fn scheme_fetch(request: &mut Request,
Ok(file_path) => {
match File::open(file_path.clone()) {
Ok(mut file) => {
let mut bytes = vec![];
let _ = file.read_to_end(&mut bytes);
let mut reader = BufReader::with_capacity(FILE_CHUNK_SIZE, file);
let mut bytes = Vec::new();
loop {
let length = {
let mut buffer = reader.fill_buf().unwrap().to_vec();
let buffer_len = buffer.len();
bytes.append(&mut buffer);
target.process_response_chunk(buffer);

This comment has been minimized.

@jdm

jdm Aug 31, 2018

Member

We actually have a system in place that requires less manual work, so let's make use of it. wait_for_response handles both synchronous and asynchronous responses; we can make this one asynchronous by:

  1. spawning a new thread before we open the file
  2. creating a new channel and storing it in done_chan (
    // We're about to spawn a thread to be waited on here
    let (done_sender, done_receiver) = channel();
    *done_chan = Some((done_sender.clone(), done_receiver));
    )
  3. setting the body to ResponseBody::Receiving
  4. sending the file chunks across the channel
  5. setting the body to ResponseBody::Done once the file is completely read (
    if cancellation_listener.lock().unwrap().cancelled() {
    *res_body.lock().unwrap() = ResponseBody::Done(vec![]);
    let _ = done_sender.send(Data::Cancelled);
    return;
    }
    match read_block(&mut res) {
    Ok(Data::Payload(chunk)) => {
    if let ResponseBody::Receiving(ref mut body) = *res_body.lock().unwrap() {
    body.extend_from_slice(&chunk);
    let _ = done_sender.send(Data::Payload(chunk));
    }
    },
    Ok(Data::Done) | Err(_) => {
    let mut body = res_body.lock().unwrap();
    let completed_body = match *body {
    ResponseBody::Receiving(ref mut body) => {
    mem::replace(body, vec![])
    },
    _ => vec![],
    };
    *body = ResponseBody::Done(completed_body);
    let _ = done_sender.send(Data::Done);
    break;
    }
    Ok(Data::Cancelled) => unreachable!() // read_block doesn't return Data::Cancelled
    )

To ensure everything still works, try loading a page from your local machine (not from a web server) that loads an image >32kb using a relative URL.

This comment has been minimized.

@InquisitivePenguin

InquisitivePenguin Sep 1, 2018

Author Contributor

Would thread::spawn work for making a new thread?

This comment has been minimized.

@jdm

jdm Sep 2, 2018

Member

Yes.

This comment has been minimized.

@InquisitivePenguin

InquisitivePenguin Sep 3, 2018

Author Contributor

Should I still call target.process_response_chunk?

This comment has been minimized.

@jdm

jdm Sep 3, 2018

Member

That should not be necessary in the model that I described.

This comment has been minimized.

@InquisitivePenguin

InquisitivePenguin Sep 4, 2018

Author Contributor

I had some time to work on this today, I'm building it to see if I can load files properly. I'll push the commit if it works.

buffer_len
};
if length == 0 { break; }
reader.consume(length);
}

let mime = guess_mime_type(file_path);

let mut response = Response::new(url);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.