From f66935aba8c1003f7219f17a8e5a540e134f77d2 Mon Sep 17 00:00:00 2001 From: Zhen Zhang Date: Tue, 12 Jul 2016 22:00:08 -0700 Subject: [PATCH] servo: Merge #12406 - Refactor FileAPI implementation (from izgzhen:refactor-file); r=Manishearth 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 --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors Source-Repo: https://github.com/servo/servo Source-Revision: 665559556f5aeac5820e17684b14311aa3767c0c --- servo/components/net/blob_loader.rs | 17 +- servo/components/net/filemanager_thread.rs | 271 ++++++++++-------- servo/components/net_traits/blob_url_store.rs | 9 +- .../net_traits/filemanager_thread.rs | 25 +- servo/components/script/dom/bindings/trace.rs | 2 + servo/components/script/dom/blob.rs | 70 +++-- servo/components/script/dom/file.rs | 3 +- servo/tests/unit/net/filemanager_thread.rs | 5 +- 8 files changed, 224 insertions(+), 178 deletions(-) diff --git a/servo/components/net/blob_loader.rs b/servo/components/net/blob_loader.rs index e12cdab9804f..09a426fbc653 100644 --- a/servo/components/net/blob_loader.rs +++ b/servo/components/net/blob_loader.rs @@ -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, opt_filename: Option, - rel_pos: RelativePos, entry: BlobURLStoreEntry) { - let content_type: Mime = entry.type_string.parse().unwrap_or(mime!(Text / Plain)); + classifier: Arc, 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(()))); } } diff --git a/servo/components/net/filemanager_thread.rs b/servo/components/net/filemanager_thread.rs index 4be5b6a010df..3f19949f0e3b 100644 --- a/servo/components/net/filemanager_thread.rs +++ b/servo/components/net/filemanager_thread.rs @@ -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 { 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) -> Option; @@ -92,22 +95,38 @@ impl FileManagerThreadFactory for IpcSender FileManager { 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 FileManager { 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 FileManager { } } - 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 { entries: RwLock>, ui: &'static UI, @@ -358,11 +345,8 @@ impl FileManagerStore { 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 FileManagerStore { 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 FileManagerStore { } } - fn create_entry(&self, file_path: &Path, origin: &str) -> Option { - 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 { + 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 { + 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, 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 FileManagerStore { Ok(()) } - fn promote_memory(&self, entry: BlobURLStoreEntry, + fn promote_memory(&self, blob_buf: BlobBuf, sender: IpcSender>, 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), diff --git a/servo/components/net_traits/blob_url_store.rs b/servo/components/net_traits/blob_url_store.rs index aa68038606a4..fe9e83046fb3 100644 --- a/servo/components/net_traits/blob_url_store.rs +++ b/servo/components/net_traits/blob_url_store.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use url::Url; use uuid::Uuid; -/// Errors returns to BlobURLStoreMsg::Request +/// Errors returned to Blob URL Store request #[derive(Clone, Debug, Serialize, Deserialize)] pub enum BlobURLStoreError { /// Invalid File UUID @@ -16,12 +16,13 @@ pub enum BlobURLStoreError { /// Invalid entry content InvalidEntry, /// External error, from like file system, I/O etc. - External, + External(String), } -/// Blob URL store entry, a packaged form of Blob DOM object +/// Standalone blob buffer object #[derive(Clone, Serialize, Deserialize)] -pub struct BlobURLStoreEntry { +pub struct BlobBuf { + pub filename: Option, /// MIME type string pub type_string: String, /// Size of content in bytes diff --git a/servo/components/net_traits/filemanager_thread.rs b/servo/components/net_traits/filemanager_thread.rs index 6bbf8b59c9fe..455ed1fd2539 100644 --- a/servo/components/net_traits/filemanager_thread.rs +++ b/servo/components/net_traits/filemanager_thread.rs @@ -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 blob_url_store::{BlobURLStoreEntry, BlobURLStoreError}; +use blob_url_store::{BlobBuf, BlobURLStoreError}; use ipc_channel::ipc::IpcSender; use num_traits::ToPrimitive; use std::cmp::{max, min}; @@ -10,7 +10,8 @@ use std::ops::Range; use std::path::PathBuf; use super::{LoadConsumer, LoadData}; -// HACK: We should send Origin directly instead of this in future, blocked on #11722 +// HACK: Not really process-safe now, we should send Origin +// directly instead of this in future, blocked on #11722 /// File manager store entry's origin pub type FileOrigin = String; @@ -33,7 +34,7 @@ impl RelativePos { pub fn full_range() -> RelativePos { RelativePos { start: 0, - end: Some(0), + end: None, } } @@ -98,20 +99,24 @@ impl RelativePos { } } +// XXX: We should opt to Uuid once it implements `Deserialize` and `Serialize` +/// FileID used in inter-process message #[derive(Clone, Debug, Deserialize, Serialize)] pub struct SelectedFileId(pub String); +/// Response to file selection request #[derive(Debug, Deserialize, Serialize)] pub struct SelectedFile { pub id: SelectedFileId, pub filename: PathBuf, pub modified: u64, + pub size: u64, // https://w3c.github.io/FileAPI/#dfn-type pub type_string: String, } -/// Filter for file selection -/// the content is expected to be extension (e.g, "doc", without the prefixing ".") +/// Filter for file selection; +/// the `String` content is expected to be extension (e.g, "doc", without the prefixing ".") #[derive(Clone, Debug, Deserialize, Serialize)] pub struct FilterPattern(pub String); @@ -131,7 +136,7 @@ pub enum FileManagerThreadMsg { /// Add an entry as promoted memory-based blob and send back the associated FileID /// as part of a valid Blob URL - PromoteMemory(BlobURLStoreEntry, IpcSender>, FileOrigin), + PromoteMemory(BlobBuf, IpcSender>, FileOrigin), /// Add a sliced entry pointing to the parent FileID, and send back the associated FileID /// as part of a valid Blob URL @@ -161,8 +166,8 @@ pub enum FileManagerThreadError { InvalidSelection, /// The selection action is cancelled by user UserCancelled, - /// Failure to process file information such as file name, modified time etc. - FileInfoProcessingError, - /// Failure to read the file content - ReadFileError, + /// Errors returned from file system request + FileSystemError(String), + /// Blob URL Store error + BlobURLStoreError(BlobURLStoreError), } diff --git a/servo/components/script/dom/bindings/trace.rs b/servo/components/script/dom/bindings/trace.rs index e2c6e540a3d5..53912f635083 100644 --- a/servo/components/script/dom/bindings/trace.rs +++ b/servo/components/script/dom/bindings/trace.rs @@ -79,6 +79,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::hash::{BuildHasher, Hash}; use std::mem; use std::ops::{Deref, DerefMut}; +use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicUsize}; @@ -331,6 +332,7 @@ no_jsmanaged_fields!(SystemTime); no_jsmanaged_fields!(SelectedFileId); no_jsmanaged_fields!(RelativePos); no_jsmanaged_fields!(OpaqueStyleAndLayoutData); +no_jsmanaged_fields!(PathBuf); no_jsmanaged_fields!(CSSErrorReporter); no_jsmanaged_fields!(WebGLBufferId); no_jsmanaged_fields!(WebGLFramebufferId); diff --git a/servo/components/script/dom/blob.rs b/servo/components/script/dom/blob.rs index 1b114e906528..9a7ddc7e6b74 100644 --- a/servo/components/script/dom/blob.rs +++ b/servo/components/script/dom/blob.rs @@ -15,17 +15,29 @@ use encoding::all::UTF_8; use encoding::types::{EncoderTrap, Encoding}; use ipc_channel::ipc; use net_traits::IpcSend; -use net_traits::blob_url_store::BlobURLStoreEntry; +use net_traits::blob_url_store::BlobBuf; use net_traits::filemanager_thread::{FileManagerThreadMsg, SelectedFileId, RelativePos}; use std::ascii::AsciiExt; use std::cell::Cell; use std::ops::Index; +use std::path::PathBuf; +/// File-based blob +#[derive(JSTraceable)] +pub struct FileBlob { + id: SelectedFileId, + name: PathBuf, + cache: DOMRefCell>>, + size: u64, +} + + +/// Blob backend implementation #[must_root] #[derive(JSTraceable)] pub enum BlobImpl { - /// File-based blob, including id and possibly cached content - File(SelectedFileId, DOMRefCell>>), + /// File-based blob + File(FileBlob), /// Memory-based blob Memory(Vec), /// Sliced blob, including parent blob and @@ -42,8 +54,13 @@ impl BlobImpl { } /// Construct file-backed BlobImpl from File ID - pub fn new_from_file(file_id: SelectedFileId) -> BlobImpl { - BlobImpl::File(file_id, DOMRefCell::new(None)) + pub fn new_from_file(file_id: SelectedFileId, name: PathBuf, size: u64) -> BlobImpl { + BlobImpl::File(FileBlob { + id: file_id, + name: name, + cache: DOMRefCell::new(None), + size: size, + }) } } @@ -79,8 +96,8 @@ impl Blob { relativeContentType: DOMString) -> Root { let global = parent.global(); let blob_impl = match *parent.blob_impl.borrow() { - BlobImpl::File(ref id, _) => { - inc_ref_id(global.r(), id.clone()); + BlobImpl::File(ref f) => { + inc_ref_id(global.r(), f.id.clone()); // Create new parent node BlobImpl::Sliced(JS::from_ref(parent), rel_pos) @@ -93,8 +110,8 @@ impl Blob { // Adjust the slicing position, using same parent let new_rel_pos = old_rel_pos.slice_inner(&rel_pos); - if let BlobImpl::File(ref id, _) = *grandparent.blob_impl.borrow() { - inc_ref_id(global.r(), id.clone()); + if let BlobImpl::File(ref f) = *grandparent.blob_impl.borrow() { + inc_ref_id(global.r(), f.id.clone()); } BlobImpl::Sliced(grandparent.clone(), new_rel_pos) @@ -124,22 +141,22 @@ impl Blob { /// Get a slice to inner data, this might incur synchronous read and caching pub fn get_bytes(&self) -> Result, ()> { match *self.blob_impl.borrow() { - BlobImpl::File(ref id, ref cached) => { - let buffer = match *cached.borrow() { - Some(ref s) => Ok(s.clone()), + BlobImpl::File(ref f) => { + let (buffer, is_new_buffer) = match *f.cache.borrow() { + Some(ref bytes) => (bytes.clone(), false), None => { let global = self.global(); - let s = read_file(global.r(), id.clone())?; - Ok(s) + let bytes = read_file(global.r(), f.id.clone())?; + (bytes, true) } }; // Cache - if let Ok(buf) = buffer.clone() { - *cached.borrow_mut() = Some(buf); + if is_new_buffer { + *f.cache.borrow_mut() = Some(buffer.clone()); } - buffer + Ok(buffer) } BlobImpl::Memory(ref s) => Ok(s.clone()), BlobImpl::Sliced(ref parent, ref rel_pos) => { @@ -155,16 +172,16 @@ impl Blob { /// used by URL.createObjectURL pub fn get_blob_url_id(&self) -> SelectedFileId { match *self.blob_impl.borrow() { - BlobImpl::File(ref id, _) => { + BlobImpl::File(ref f) => { let global = self.global(); let origin = global.r().get_url().origin().unicode_serialization(); let filemanager = global.r().resource_threads().sender(); let (tx, rx) = ipc::channel().unwrap(); - let _ = filemanager.send(FileManagerThreadMsg::ActivateBlobURL(id.clone(), tx, origin.clone())); + let _ = filemanager.send(FileManagerThreadMsg::ActivateBlobURL(f.id.clone(), tx, origin.clone())); match rx.recv().unwrap() { - Ok(_) => id.clone(), + Ok(_) => f.id.clone(), Err(_) => SelectedFileId("".to_string()) // Return a dummy id on error } } @@ -176,8 +193,8 @@ impl Blob { // Return dummy id SelectedFileId("".to_string()) } - BlobImpl::File(ref parent_id, _) => - self.create_sliced_url_id(parent_id, rel_pos), + BlobImpl::File(ref f) => + self.create_sliced_url_id(&f.id, rel_pos), BlobImpl::Memory(ref bytes) => { let parent_id = parent.promote_to_file(bytes); *self.blob_impl.borrow_mut() = BlobImpl::Sliced(parent.clone(), rel_pos.clone()); @@ -195,14 +212,15 @@ impl Blob { let origin = global.r().get_url().origin().unicode_serialization(); let filemanager = global.r().resource_threads().sender(); - let entry = BlobURLStoreEntry { + let blob_buf = BlobBuf { + filename: None, type_string: self.typeString.clone(), size: self.Size(), bytes: bytes.to_vec(), }; let (tx, rx) = ipc::channel().unwrap(); - let _ = filemanager.send(FileManagerThreadMsg::PromoteMemory(entry, tx, origin.clone())); + let _ = filemanager.send(FileManagerThreadMsg::PromoteMemory(blob_buf, tx, origin.clone())); match rx.recv().unwrap() { Ok(new_id) => SelectedFileId(new_id.0), @@ -232,14 +250,14 @@ impl Blob { /// Cleanups at the time of destruction/closing fn clean_up_file_resource(&self) { - if let BlobImpl::File(ref id, _) = *self.blob_impl.borrow() { + if let BlobImpl::File(ref f) = *self.blob_impl.borrow() { let global = self.global(); let origin = global.r().get_url().origin().unicode_serialization(); let filemanager = global.r().resource_threads().sender(); let (tx, rx) = ipc::channel().unwrap(); - let msg = FileManagerThreadMsg::DecRef(id.clone(), origin, tx); + let msg = FileManagerThreadMsg::DecRef(f.id.clone(), origin, tx); let _ = filemanager.send(msg); let _ = rx.recv().unwrap(); } diff --git a/servo/components/script/dom/file.rs b/servo/components/script/dom/file.rs index 75f73aeda594..e5a41965bba5 100644 --- a/servo/components/script/dom/file.rs +++ b/servo/components/script/dom/file.rs @@ -54,7 +54,8 @@ impl File { let global = GlobalRef::Window(window); - File::new(global, BlobImpl::new_from_file(selected.id), name, Some(selected.modified as i64), "") + File::new(global, BlobImpl::new_from_file(selected.id, selected.filename, selected.size), + name, Some(selected.modified as i64), "") } // https://w3c.github.io/FileAPI/#file-constructor diff --git a/servo/tests/unit/net/filemanager_thread.rs b/servo/tests/unit/net/filemanager_thread.rs index 036fdc969c7d..7c203fec115f 100644 --- a/servo/tests/unit/net/filemanager_thread.rs +++ b/servo/tests/unit/net/filemanager_thread.rs @@ -4,6 +4,7 @@ use ipc_channel::ipc::{self, IpcSender}; use net::filemanager_thread::{FileManagerThreadFactory, UIProvider}; +use net_traits::blob_url_store::BlobURLStoreError; use net_traits::filemanager_thread::{FilterPattern, FileManagerThreadMsg, FileManagerThreadError}; use std::fs::File; use std::io::Read; @@ -56,7 +57,7 @@ fn test_filemanager() { let msg = rx2.recv().expect("Broken channel"); let vec = msg.expect("File manager reading failure is unexpected"); - assert!(test_file_content == vec, "Read content differs"); + assert_eq!(test_file_content, vec, "Read content differs"); } // Delete the id @@ -76,7 +77,7 @@ fn test_filemanager() { let msg = rx2.recv().expect("Broken channel"); match msg { - Err(FileManagerThreadError::ReadFileError) => {}, + Err(FileManagerThreadError::BlobURLStoreError(BlobURLStoreError::InvalidFileID)) => {}, other => { assert!(false, "Get unexpected response after deleting the id: {:?}", other); }