servo: Merge #12406 - Refactor FileAPI implementation (from izgzhen:refactor-file); r=Manishearth
authorZhen Zhang <izgzhen@gmail.com>
Tue, 12 Jul 2016 22:00:08 -0700
changeset 339278 1c10c2c3ec8dc542be86929e10364590e11989e1
parent 339277 6ad159ec3b5998745e905573a79b25c2cfd30bc9
child 339279 413d60265ae9b7cfdf33bb81161edaecff59c6b2
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersManishearth
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 <!-- 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. --> Source-Repo: https://github.com/servo/servo Source-Revision: 665559556f5aeac5820e17684b14311aa3767c0c
servo/components/net/blob_loader.rs
servo/components/net/filemanager_thread.rs
servo/components/net_traits/blob_url_store.rs
servo/components/net_traits/filemanager_thread.rs
servo/components/script/dom/bindings/trace.rs
servo/components/script/dom/blob.rs
servo/components/script/dom/file.rs
servo/tests/unit/net/filemanager_thread.rs
--- a/servo/components/net/blob_loader.rs
+++ b/servo/components/net/blob_loader.rs
@@ -3,59 +3,54 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use hyper::header::{DispositionType, ContentDisposition, DispositionParam};
 use hyper::header::{Headers, ContentType, ContentLength, Charset};
 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,
             parameters: vec![
                 DispositionParam::Filename(charset.unwrap_or(Charset::Us_Ascii),
                                            None, name.as_bytes().to_vec())
             ]
         });
     }
 
-    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(),
         content_type: Some(ContentType(content_type.clone())),
         charset: charset.map(|c| c.as_str().to_string()),
         headers: Some(headers),
         // https://w3c.github.io/FileAPI/#TwoHundredOK
         status: Some(RawStatus(200, "OK".into())),
         https_state: HttpsState::None,
     };
 
     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(())));
     }
 }
--- a/servo/components/net/filemanager_thread.rs
+++ b/servo/components/net/filemanager_thread.rs
@@ -1,39 +1,42 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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_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};
 use std::sync::{Arc, RwLock};
 #[cfg(any(target_os = "macos", target_os = "linux"))]
 use tinyfiledialogs;
 use url::Url;
 use util::prefs::PREFS;
 use util::thread::spawn_named;
 use uuid::Uuid;
 
 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>;
 
     fn open_file_dialog_multi(&self, path: &str, patterns: Vec<FilterPattern>) -> Option<Vec<String>>;
 }
 
 pub struct TFDProvider;
 
@@ -87,32 +90,48 @@ impl<UI: 'static + UIProvider> FileManag
         spawn_named("FileManager".to_owned(), move || {
             FileManager::new(recv, ui).start();
         });
 
         chan
     }
 }
 
+/// 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),
 }
 
 struct FileManager<UI: 'static + UIProvider> {
     receiver: IpcReceiver<FileManagerThreadMsg>,
     store: Arc<FileManagerStore<UI>>,
     classifier: Arc<MimeClassifier>,
 }
@@ -140,40 +159,41 @@ impl<UI: 'static + UIProvider> FileManag
                     spawn_named("select files".to_owned(), move || {
                         store.select_files(filter, sender, origin, opt_test_paths);
                     })
                 }
                 FileManagerThreadMsg::ReadFile(sender, id, origin) => {
                     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) =>{
                     spawn_named("add sliced URL entry".to_owned(), move || {
                         store.add_sliced_url_entry(id, rel_pos, sender, origin);
                     })
                 }
                 FileManagerThreadMsg::LoadBlob(load_data, consumer) => {
                     match parse_blob_url(&load_data.url.clone()) {
                         None => {
                             let e = format!("Invalid blob URL format {:?}", load_data.url);
                             let format_err = NetworkError::Internal(e);
                             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);
                         }
                     }
                 },
                 FileManagerThreadMsg::RevokeBlobURL(id, origin, sender) => {
                     if let Ok(id) = Uuid::parse_str(&id.0) {
                         spawn_named("revoke blob url".to_owned(), move || {
                             // Since it is revocation, unset_url_validity is true
                             let _ = sender.send(store.dec_ref(&id, &origin, true));
@@ -209,65 +229,32 @@ impl<UI: 'static + UIProvider> FileManag
                         let _ = sender.send(Err(BlobURLStoreError::InvalidFileID));
                     }
                 }
                 FileManagerThreadMsg::Exit => break,
             };
         }
     }
 
-    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);
-                    }
-                }
+        // 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);
-            }
+            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,
 }
 
 impl <UI: 'static + UIProvider> FileManagerStore<UI> {
     fn new(ui: &'static UI) -> Self {
         FileManagerStore {
@@ -353,21 +340,18 @@ impl <UI: 'static + UIProvider> FileMana
             opt_test_path
         } else {
             self.ui.open_file_dialog("", patterns)
         };
 
         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));
                 return;
             }
         }
     }
 
@@ -390,102 +374,141 @@ impl <UI: 'static + UIProvider> FileMana
                 for s in &v {
                     selected_paths.push(Path::new(s));
                 }
 
                 let mut replies = vec![];
 
                 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;
+                        }
                     };
                 }
 
                 let _ = sender.send(Ok(replies));
             }
             None => {
                 let _ = sender.send(Err(FileManagerThreadError::UserCancelled));
                 return;
             }
         }
     }
 
-    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;
+
+        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())));
+
+        let file_impl = FileImpl::MetaDataOnly(FileMetaData {
+            path: file_path.to_path_buf(),
+            modified: modified_epoch,
+            size: file_size,
+        });
+
+        let id = Uuid::new_v4();
 
-                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),
-                });
+        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,
+        })
+    }
 
-                // 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)
-                                                });
+    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 filename = file_path.file_name();
+                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);
 
-                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
+                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,
                unset_url_validity: bool) -> Result<(), BlobURLStoreError> {
         let (is_last_ref, opt_parent_id) = match self.entries.read().unwrap().get(id) {
             Some(entry) => {
                 if *entry.origin == *origin_in {
                     let old_refs = entry.refs.fetch_sub(1, Ordering::Release);
@@ -520,24 +543,24 @@ impl <UI: 'static + UIProvider> FileMana
                 // to unset the initial requesting URL
                 return self.dec_ref(&parent_id, origin_in, false);
             }
         }
 
         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),
                 });
 
                 let _ = sender.send(Ok(SelectedFileId(id.simple().to_string())));
             }
             Err(_) => {
--- a/servo/components/net_traits/blob_url_store.rs
+++ b/servo/components/net_traits/blob_url_store.rs
@@ -1,32 +1,33 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 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
     InvalidFileID,
     /// Invalid URL origin
     InvalidOrigin,
     /// 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<String>,
     /// MIME type string
     pub type_string: String,
     /// Size of content in bytes
     pub size: u64,
     /// Content of blob
     pub bytes: Vec<u8>,
 }
 
--- a/servo/components/net_traits/filemanager_thread.rs
+++ b/servo/components/net_traits/filemanager_thread.rs
@@ -1,21 +1,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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};
 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;
 
 /// Relative slice positions of a sequence,
 /// whose semantic should be consistent with (start, end) parameters in
 /// https://w3c.github.io/FileAPI/#dfn-slice
 #[derive(Clone, Deserialize, Serialize)]
 pub struct RelativePos {
@@ -28,17 +29,17 @@ pub struct RelativePos {
     pub end: Option<i64>,
 }
 
 impl RelativePos {
     /// Full range from start to end
     pub fn full_range() -> RelativePos {
         RelativePos {
             start: 0,
-            end: Some(0),
+            end: None,
         }
     }
 
     /// Instantiate optional slice position parameters
     pub fn from_opts(start: Option<i64>, end: Option<i64>) -> RelativePos {
         RelativePos {
             start: start.unwrap_or(0),
             end: end,
@@ -93,30 +94,34 @@ impl RelativePos {
     pub fn from_abs_range(range: Range<usize>, size: usize) -> RelativePos {
         RelativePos {
             start: range.start as i64,
             end: Some(size as i64 - range.end as i64),
         }
     }
 }
 
+// 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);
 
 #[derive(Deserialize, Serialize)]
 pub enum FileManagerThreadMsg {
     /// Select a single file, return triple (FileID, FileName, lastModified)
     SelectFile(Vec<FilterPattern>, IpcSender<FileManagerResult<SelectedFile>>, FileOrigin, Option<String>),
 
@@ -126,17 +131,17 @@ pub enum FileManagerThreadMsg {
     /// Read file, return the bytes
     ReadFile(IpcSender<FileManagerResult<Vec<u8>>>, SelectedFileId, FileOrigin),
 
     /// Load resource by Blob URL
     LoadBlob(LoadData, LoadConsumer),
 
     /// Add an entry as promoted memory-based blob and send back the associated FileID
     /// as part of a valid Blob URL
-    PromoteMemory(BlobURLStoreEntry, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
+    PromoteMemory(BlobBuf, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
 
     /// Add a sliced entry pointing to the parent FileID, and send back the associated FileID
     /// as part of a valid Blob URL
     AddSlicedURLEntry(SelectedFileId, RelativePos, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
 
     /// Revoke Blob URL and send back the acknowledgement
     RevokeBlobURL(SelectedFileId, FileOrigin, IpcSender<Result<(), BlobURLStoreError>>),
 
@@ -156,13 +161,13 @@ pub enum FileManagerThreadMsg {
 pub type FileManagerResult<T> = Result<T, FileManagerThreadError>;
 
 #[derive(Debug, Deserialize, Serialize)]
 pub enum FileManagerThreadError {
     /// The selection action is invalid due to exceptional reason
     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),
 }
--- a/servo/components/script/dom/bindings/trace.rs
+++ b/servo/components/script/dom/bindings/trace.rs
@@ -74,16 +74,17 @@ use script_traits::{TimerEventId, TimerS
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
 use std::boxed::FnBox;
 use std::cell::{Cell, UnsafeCell};
 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};
 use std::sync::mpsc::{Receiver, Sender};
 use std::time::SystemTime;
 use string_cache::{Atom, Namespace, QualName};
 use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto};
 use style::element_state::*;
@@ -326,16 +327,17 @@ no_jsmanaged_fields!(SharedRt);
 no_jsmanaged_fields!(TouchpadPressurePhase);
 no_jsmanaged_fields!(USVString);
 no_jsmanaged_fields!(ReferrerPolicy);
 no_jsmanaged_fields!(ResourceThreads);
 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);
 no_jsmanaged_fields!(WebGLProgramId);
 no_jsmanaged_fields!(WebGLRenderbufferId);
 no_jsmanaged_fields!(WebGLShaderId);
 no_jsmanaged_fields!(WebGLTextureId);
 
--- a/servo/components/script/dom/blob.rs
+++ b/servo/components/script/dom/blob.rs
@@ -10,45 +10,62 @@ use dom::bindings::error::{Error, Fallib
 use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::{Reflectable, Reflector, reflect_dom_object};
 use dom::bindings::str::DOMString;
 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<Option<Vec<u8>>>,
+    size: u64,
+}
+
+
+/// Blob backend implementation
 #[must_root]
 #[derive(JSTraceable)]
 pub enum BlobImpl {
-    /// File-based blob, including id and possibly cached content
-    File(SelectedFileId, DOMRefCell<Option<Vec<u8>>>),
+    /// File-based blob
+    File(FileBlob),
     /// Memory-based blob
     Memory(Vec<u8>),
     /// Sliced blob, including parent blob and
     /// relative positions representing current slicing range,
     /// it is leaf of a two-layer fat tree
     Sliced(JS<Blob>, RelativePos),
 }
 
 impl BlobImpl {
     /// Construct memory-backed BlobImpl
     #[allow(unrooted_must_root)]
     pub fn new_from_bytes(bytes: Vec<u8>) -> BlobImpl {
         BlobImpl::Memory(bytes)
     }
 
     /// 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,
+        })
     }
 }
 
 // https://w3c.github.io/FileAPI/#blob
 #[dom_struct]
 pub struct Blob {
     reflector_: Reflector,
     #[ignore_heap_size_of = "No clear owner"]
@@ -74,32 +91,32 @@ impl Blob {
         }
     }
 
     #[allow(unrooted_must_root)]
     fn new_sliced(parent: &Blob, rel_pos: RelativePos,
                   relativeContentType: DOMString) -> Root<Blob> {
         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)
             }
             BlobImpl::Memory(_) => {
                 // Create new parent node
                 BlobImpl::Sliced(JS::from_ref(parent), rel_pos)
             }
             BlobImpl::Sliced(ref grandparent, ref old_rel_pos) => {
                 // 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)
             }
         };
 
         Blob::new(global.r(), blob_impl, relativeContentType.into())
     }
@@ -119,70 +136,70 @@ impl Blob {
         };
 
         Ok(Blob::new(global, BlobImpl::new_from_bytes(bytes), blobPropertyBag.get_typestring()))
     }
 
     /// Get a slice to inner data, this might incur synchronous read and caching
     pub fn get_bytes(&self) -> Result<Vec<u8>, ()> {
         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) => {
                 parent.get_bytes().map(|v| {
                     let range = rel_pos.to_abs_range(v.len());
                     v.index(range).to_vec()
                 })
             }
         }
     }
 
     /// Get a FileID representing the Blob content,
     /// 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
                 }
             }
             BlobImpl::Memory(ref slice) => self.promote_to_file(slice),
             BlobImpl::Sliced(ref parent, ref rel_pos) => {
                 match *parent.blob_impl.borrow() {
                     BlobImpl::Sliced(_, _) => {
                         debug!("Sliced can't have a sliced parent");
                         // 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());
                         self.create_sliced_url_id(&parent_id, rel_pos)
                     }
                 }
             }
         }
@@ -190,24 +207,25 @@ impl Blob {
 
     /// Promite memory-based Blob to file-based,
     /// The bytes in data slice will be transferred to file manager thread
     fn promote_to_file(&self, bytes: &[u8]) -> SelectedFileId {
         let global = self.global();
         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),
             // Dummy id
             Err(_) => SelectedFileId("".to_string()),
         }
     }
 
@@ -227,24 +245,24 @@ impl Blob {
         let new_id = rx.recv().unwrap().unwrap();
 
         // Return the indirect id reference
         SelectedFileId(new_id.0)
     }
 
     /// 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();
         }
     }
 }
 
 impl Drop for Blob {
     fn drop(&mut self) {
--- a/servo/components/script/dom/file.rs
+++ b/servo/components/script/dom/file.rs
@@ -49,17 +49,18 @@ impl File {
     }
 
     // Construct from selected file message from file manager thread
     pub fn new_from_selected(window: &Window, selected: SelectedFile) -> Root<File> {
         let name = DOMString::from(selected.filename.to_str().expect("File name encoding error"));
 
         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
     pub fn Constructor(global: GlobalRef,
                        fileBits: Vec<BlobOrString>,
                        filename: DOMString,
                        filePropertyBag: &FileBinding::FilePropertyBag)
                        -> Fallible<Root<File>> {
--- a/servo/tests/unit/net/filemanager_thread.rs
+++ b/servo/tests/unit/net/filemanager_thread.rs
@@ -1,14 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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 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;
 use std::path::PathBuf;
 
 const TEST_PROVIDER: &'static TestProvider = &TestProvider;
 
 struct TestProvider;
@@ -51,17 +52,17 @@ fn test_filemanager() {
         // Test by reading, expecting same content
         {
             let (tx2, rx2) = ipc::channel().unwrap();
             chan.send(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), origin.clone())).unwrap();
 
             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
         {
             let (tx2, rx2) = ipc::channel().unwrap();
             chan.send(FileManagerThreadMsg::DecRef(selected.id.clone(), origin.clone(), tx2)).unwrap();
 
             let ret = rx2.recv().expect("Broken channel");
@@ -71,17 +72,17 @@ fn test_filemanager() {
         // Test by reading again, expecting read error because we invalidated the id
         {
             let (tx2, rx2) = ipc::channel().unwrap();
             chan.send(FileManagerThreadMsg::ReadFile(tx2, selected.id.clone(), origin.clone())).unwrap();
 
             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);
                 }
             }
         }
     }
 
     let _ = chan.send(FileManagerThreadMsg::Exit);