Auto merge of #12406 - izgzhen:refactor-file, r=Manishearth

Refactor FileAPI implementation

Most are simple refactoring, cleanups and improvements, but still involving two slightly notable changes:

+ In `filemanager`, now we read the file content based on requested `RelativePos` by `seek` and `read_exact` (rather than `read_to_end` then do slicing). This strategy might be again adjusted in future performance tuning but certainly better than nothing.
+ Also, I cached more file meta-info in both sides and left a block of comment on `filemanager`'s file reading mentioning the snapshot-state problem (not solved now though).

r? @Manishearth

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/12406)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-07-12 22:00:08 -07:00 committed by GitHub
commit 665559556f
8 changed files with 224 additions and 178 deletions

View file

@ -8,26 +8,23 @@ use hyper::http::RawStatus;
use mime::{Mime, Attr};
use mime_classifier::MimeClassifier;
use net_traits::ProgressMsg::Done;
use net_traits::blob_url_store::BlobURLStoreEntry;
use net_traits::filemanager_thread::RelativePos;
use net_traits::blob_url_store::BlobBuf;
use net_traits::response::HttpsState;
use net_traits::{LoadConsumer, LoadData, Metadata};
use resource_thread::start_sending_sniffed_opt;
use std::ops::Index;
use std::sync::Arc;
// TODO: Check on GET
// https://w3c.github.io/FileAPI/#requestResponseModel
pub fn load_blob(load_data: LoadData, start_chan: LoadConsumer,
classifier: Arc<MimeClassifier>, opt_filename: Option<String>,
rel_pos: RelativePos, entry: BlobURLStoreEntry) {
let content_type: Mime = entry.type_string.parse().unwrap_or(mime!(Text / Plain));
classifier: Arc<MimeClassifier>, blob_buf: BlobBuf) {
let content_type: Mime = blob_buf.type_string.parse().unwrap_or(mime!(Text / Plain));
let charset = content_type.get_param(Attr::Charset);
let mut headers = Headers::new();
if let Some(name) = opt_filename {
if let Some(name) = blob_buf.filename {
let charset = charset.and_then(|c| c.as_str().parse().ok());
headers.set(ContentDisposition {
disposition: DispositionType::Inline,
@ -38,10 +35,8 @@ pub fn load_blob(load_data: LoadData, start_chan: LoadConsumer,
});
}
let range = rel_pos.to_abs_range(entry.size as usize);
headers.set(ContentType(content_type.clone()));
headers.set(ContentLength(range.len() as u64));
headers.set(ContentLength(blob_buf.size as u64));
let metadata = Metadata {
final_url: load_data.url.clone(),
@ -55,7 +50,7 @@ pub fn load_blob(load_data: LoadData, start_chan: LoadConsumer,
if let Ok(chan) =
start_sending_sniffed_opt(start_chan, metadata, classifier,
&entry.bytes.index(range), load_data.context.clone()) {
&blob_buf.bytes, load_data.context.clone()) {
let _ = chan.send(Done(Ok(())));
}
}

View file

@ -6,14 +6,14 @@ use blob_loader::load_blob;
use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
use mime_classifier::MimeClassifier;
use mime_guess::guess_mime_type_opt;
use net_traits::blob_url_store::{BlobURLStoreEntry, BlobURLStoreError, parse_blob_url};
use net_traits::blob_url_store::{BlobBuf, BlobURLStoreError, parse_blob_url};
use net_traits::filemanager_thread::{FileManagerThreadMsg, FileManagerResult, FilterPattern, FileOrigin};
use net_traits::filemanager_thread::{SelectedFile, RelativePos, FileManagerThreadError, SelectedFileId};
use net_traits::{LoadConsumer, LoadData, NetworkError};
use resource_thread::send_error;
use std::collections::HashMap;
use std::fs::File;
use std::io::Read;
use std::io::{Read, Seek, SeekFrom};
use std::ops::Index;
use std::path::{Path, PathBuf};
use std::sync::atomic::{self, AtomicUsize, AtomicBool, Ordering};
@ -29,6 +29,9 @@ pub trait FileManagerThreadFactory<UI: 'static + UIProvider> {
fn new(&'static UI) -> Self;
}
/// Trait that provider of file-dialog UI should implement.
/// It will be used to initialize a generic FileManager.
/// For example, we can choose a dummy UI for testing purpose.
pub trait UIProvider where Self: Sync {
fn open_file_dialog(&self, path: &str, patterns: Vec<FilterPattern>) -> Option<String>;
@ -92,22 +95,38 @@ impl<UI: 'static + UIProvider> FileManagerThreadFactory<UI> for IpcSender<FileMa
}
}
/// FileManagerStore's entry
struct FileStoreEntry {
/// Origin of the entry's "creator"
origin: FileOrigin,
/// Backend implementation
file_impl: FileImpl,
/// Reference counting
/// Number of `FileImpl::Sliced` entries in `FileManagerStore`
/// that has a reference (FileID) to this entry
refs: AtomicUsize,
/// UUID key's validity as Blob URL
/// UUIDs only become valid blob URIs when explicitly requested
/// by the user with createObjectURL. Validity can be revoked as well.
/// (The UUID is the one that maps to this entry in `FileManagerStore`)
is_valid_url: AtomicBool
}
#[derive(Clone)]
struct FileMetaData {
path: PathBuf,
/// Modified time in UNIX Epoch format
modified: u64,
size: u64,
}
/// File backend implementation
#[derive(Clone)]
enum FileImpl {
PathOnly(PathBuf),
Memory(BlobURLStoreEntry),
/// Metadata of on-disk file
MetaDataOnly(FileMetaData),
/// In-memory Blob buffer object
Memory(BlobBuf),
/// A reference to parent entry in `FileManagerStore`,
/// representing a sliced version of the parent entry data
Sliced(Uuid, RelativePos),
}
@ -145,13 +164,15 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
spawn_named("read file".to_owned(), move || {
match store.try_read_file(id, origin) {
Ok(buffer) => { let _ = sender.send(Ok(buffer)); }
Err(_) => { let _ = sender.send(Err(FileManagerThreadError::ReadFileError)); }
Err(e) => {
let _ = sender.send(Err(FileManagerThreadError::BlobURLStoreError(e)));
}
}
})
}
FileManagerThreadMsg::PromoteMemory(entry, sender, origin) => {
FileManagerThreadMsg::PromoteMemory(blob_buf, sender, origin) => {
spawn_named("transfer memory".to_owned(), move || {
store.promote_memory(entry, sender, origin);
store.promote_memory(blob_buf, sender, origin);
})
}
FileManagerThreadMsg::AddSlicedURLEntry(id, rel_pos, sender, origin) =>{
@ -167,8 +188,7 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
send_error(load_data.url.clone(), format_err, consumer);
}
Some((id, _fragment)) => {
// check_url_validity is true since content is requested by this URL
self.process_request(load_data, consumer, RelativePos::full_range(), id, true);
self.process_request(load_data, consumer, id);
}
}
},
@ -214,55 +234,22 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
}
}
fn process_request(&self, load_data: LoadData, consumer: LoadConsumer,
rel_pos: RelativePos, id: Uuid, check_url_validity: bool) {
fn process_request(&self, load_data: LoadData, consumer: LoadConsumer, id: Uuid) {
let origin_in = load_data.url.origin().unicode_serialization();
match self.store.get_impl(&id, &origin_in, check_url_validity) {
Ok(file_impl) => {
match file_impl {
FileImpl::Memory(buffered) => {
let classifier = self.classifier.clone();
spawn_named("load blob".to_owned(), move ||
load_blob(load_data, consumer, classifier,
None, rel_pos, buffered));
}
FileImpl::PathOnly(filepath) => {
let opt_filename = filepath.file_name()
.and_then(|osstr| osstr.to_str())
.map(|s| s.to_string());
let mut bytes = vec![];
let mut handler = File::open(&filepath).unwrap();
let mime = guess_mime_type_opt(filepath);
let size = handler.read_to_end(&mut bytes).unwrap();
let entry = BlobURLStoreEntry {
type_string: match mime {
Some(x) => format!("{}", x),
None => "".to_string(),
},
size: size as u64,
bytes: bytes,
};
let classifier = self.classifier.clone();
spawn_named("load blob".to_owned(), move ||
load_blob(load_data, consumer, classifier,
opt_filename, rel_pos, entry));
},
FileImpl::Sliced(id, rel_pos) => {
// Next time we don't need to check validity since
// we have already done that for requesting URL
self.process_request(load_data, consumer, rel_pos, id, false);
}
}
}
Err(e) => {
send_error(load_data.url.clone(), NetworkError::Internal(format!("{:?}", e)), consumer);
// check_url_validity is true since content is requested by this URL
match self.store.get_blob_buf(&id, &origin_in, RelativePos::full_range(), true) {
Ok(blob_buf) => {
let classifier = self.classifier.clone();
spawn_named("load blob".to_owned(), move || load_blob(load_data, consumer, classifier, blob_buf));
}
Err(e) => send_error(load_data.url.clone(), NetworkError::Internal(format!("{:?}", e)), consumer),
}
}
}
/// 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> {
entries: RwLock<HashMap<Uuid, FileStoreEntry>>,
ui: &'static UI,
@ -358,11 +345,8 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
match opt_s {
Some(s) => {
let selected_path = Path::new(&s);
match self.create_entry(selected_path, &origin) {
Some(triple) => { let _ = sender.send(Ok(triple)); }
None => { let _ = sender.send(Err(FileManagerThreadError::InvalidSelection)); }
};
let result = self.create_entry(selected_path, &origin);
let _ = sender.send(result);
}
None => {
let _ = sender.send(Err(FileManagerThreadError::UserCancelled));
@ -395,8 +379,11 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
for path in selected_paths {
match self.create_entry(path, &origin) {
Some(triple) => replies.push(triple),
None => { let _ = sender.send(Err(FileManagerThreadError::InvalidSelection)); }
Ok(triple) => replies.push(triple),
Err(e) => {
let _ = sender.send(Err(e));
return;
}
};
}
@ -409,78 +396,114 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
}
}
fn create_entry(&self, file_path: &Path, origin: &str) -> Option<SelectedFile> {
match File::open(file_path) {
Ok(handler) => {
let id = Uuid::new_v4();
let file_impl = FileImpl::PathOnly(file_path.to_path_buf());
fn create_entry(&self, file_path: &Path, origin: &str) -> Result<SelectedFile, FileManagerThreadError> {
use net_traits::filemanager_thread::FileManagerThreadError::FileSystemError;
self.insert(id, FileStoreEntry {
origin: origin.to_string(),
file_impl: file_impl,
refs: AtomicUsize::new(1),
// Invalid here since create_entry is called by file selection
is_valid_url: AtomicBool::new(false),
});
let handler = try!(File::open(file_path).map_err(|e| FileSystemError(e.to_string())));
let metadata = try!(handler.metadata().map_err(|e| FileSystemError(e.to_string())));
let modified = try!(metadata.modified().map_err(|e| FileSystemError(e.to_string())));
let elapsed = try!(modified.elapsed().map_err(|e| FileSystemError(e.to_string())));
// Unix Epoch: https://doc.servo.org/std/time/constant.UNIX_EPOCH.html
let modified_epoch = elapsed.as_secs() * 1000 + elapsed.subsec_nanos() as u64 / 1000000;
let file_size = metadata.len();
let file_name = try!(file_path.file_name().ok_or(FileSystemError("Invalid filepath".to_string())));
// Unix Epoch: https://doc.servo.org/std/time/constant.UNIX_EPOCH.html
let epoch = handler.metadata().and_then(|metadata| metadata.modified()).map_err(|_| ())
.and_then(|systime| systime.elapsed().map_err(|_| ()))
.and_then(|elapsed| {
let secs = elapsed.as_secs();
let nsecs = elapsed.subsec_nanos();
let msecs = secs * 1000 + nsecs as u64 / 1000000;
Ok(msecs)
});
let file_impl = FileImpl::MetaDataOnly(FileMetaData {
path: file_path.to_path_buf(),
modified: modified_epoch,
size: file_size,
});
let filename = file_path.file_name();
let id = Uuid::new_v4();
match (epoch, filename) {
(Ok(epoch), Some(filename)) => {
let filename_path = Path::new(filename);
let mime = guess_mime_type_opt(filename_path);
Some(SelectedFile {
id: SelectedFileId(id.simple().to_string()),
filename: filename_path.to_path_buf(),
modified: epoch,
type_string: match mime {
Some(x) => format!("{}", x),
None => "".to_string(),
},
})
}
_ => None
self.insert(id, FileStoreEntry {
origin: origin.to_string(),
file_impl: file_impl,
refs: AtomicUsize::new(1),
// Invalid here since create_entry is called by file selection
is_valid_url: AtomicBool::new(false),
});
let filename_path = Path::new(file_name);
let type_string = match guess_mime_type_opt(filename_path) {
Some(x) => format!("{}", x),
None => "".to_string(),
};
Ok(SelectedFile {
id: SelectedFileId(id.simple().to_string()),
filename: filename_path.to_path_buf(),
modified: modified_epoch,
size: file_size,
type_string: type_string,
})
}
fn get_blob_buf(&self, id: &Uuid, origin_in: &FileOrigin, rel_pos: RelativePos,
check_url_validity: bool) -> Result<BlobBuf, BlobURLStoreError> {
let file_impl = try!(self.get_impl(id, origin_in, check_url_validity));
match file_impl {
FileImpl::Memory(buf) => {
let range = rel_pos.to_abs_range(buf.size as usize);
Ok(BlobBuf {
filename: None,
type_string: buf.type_string,
size: range.len() as u64,
bytes: buf.bytes.index(range).to_vec(),
})
}
FileImpl::MetaDataOnly(metadata) => {
/* XXX: Snapshot state check (optional) https://w3c.github.io/FileAPI/#snapshot-state.
Concretely, here we create another handler, and this handler might not
has the same underlying file state (meta-info plus content) as the time
create_entry is called.
*/
let opt_filename = metadata.path.file_name()
.and_then(|osstr| osstr.to_str())
.map(|s| s.to_string());
let mime = guess_mime_type_opt(metadata.path.clone());
let range = rel_pos.to_abs_range(metadata.size as usize);
let mut handler = try!(File::open(&metadata.path)
.map_err(|e| BlobURLStoreError::External(e.to_string())));
let seeked_start = try!(handler.seek(SeekFrom::Start(range.start as u64))
.map_err(|e| BlobURLStoreError::External(e.to_string())));
if seeked_start == (range.start as u64) {
let mut bytes = vec![0; range.len()];
try!(handler.read_exact(&mut bytes)
.map_err(|e| BlobURLStoreError::External(e.to_string())));
Ok(BlobBuf {
filename: opt_filename,
type_string: match mime {
Some(x) => format!("{}", x),
None => "".to_string(),
},
size: range.len() as u64,
bytes: bytes,
})
} else {
Err(BlobURLStoreError::InvalidEntry)
}
},
Err(_) => None
}
FileImpl::Sliced(parent_id, inner_rel_pos) => {
// Next time we don't need to check validity since
// we have already done that for requesting URL if necessary
self.get_blob_buf(&parent_id, origin_in, rel_pos.slice_inner(&inner_rel_pos), false)
}
}
}
fn try_read_file(&self, id: SelectedFileId, origin_in: FileOrigin) -> Result<Vec<u8>, BlobURLStoreError> {
let id = try!(Uuid::parse_str(&id.0).map_err(|_| BlobURLStoreError::InvalidFileID));
match self.get_impl(&id, &origin_in, false) {
Ok(file_impl) => {
match file_impl {
FileImpl::PathOnly(filepath) => {
let mut buffer = vec![];
let mut handler = try!(File::open(filepath)
.map_err(|_| BlobURLStoreError::InvalidEntry));
try!(handler.read_to_end(&mut buffer)
.map_err(|_| BlobURLStoreError::External));
Ok(buffer)
},
FileImpl::Memory(buffered) => {
Ok(buffered.bytes)
},
FileImpl::Sliced(id, rel_pos) => {
self.try_read_file(SelectedFileId(id.simple().to_string()), origin_in)
.map(|bytes| bytes.index(rel_pos.to_abs_range(bytes.len())).to_vec())
}
}
},
Err(e) => Err(e),
}
// No need to check URL validity in reading a file by FileReader
let blob_buf = try!(self.get_blob_buf(&id, &origin_in, RelativePos::full_range(), false));
Ok(blob_buf.bytes)
}
fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin,
@ -525,14 +548,14 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
Ok(())
}
fn promote_memory(&self, entry: BlobURLStoreEntry,
fn promote_memory(&self, blob_buf: BlobBuf,
sender: IpcSender<Result<SelectedFileId, BlobURLStoreError>>, origin: FileOrigin) {
match Url::parse(&origin) { // parse to check sanity
Ok(_) => {
let id = Uuid::new_v4();
self.insert(id, FileStoreEntry {
origin: origin.clone(),
file_impl: FileImpl::Memory(entry),
file_impl: FileImpl::Memory(blob_buf),
refs: AtomicUsize::new(1),
// Valid here since PromoteMemory implies URL creation
is_valid_url: AtomicBool::new(true),