Refactor FileAPI implementation

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<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(())));
     }
 }

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<String>,
     /// MIME type string
     pub type_string: String,
     /// Size of content in bytes

components/net_traits/filemanager_thread.rs

@@ -2,15 +2,16 @@
  * 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;
 
@@ -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<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
@@ -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),
 }

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);

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<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
@@ -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<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)
@@ -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<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) => {
@@ -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();
         }

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

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);
                 }