Refactor FileAPI implementation

2 files changed, 155 insertions, 137 deletions

diff --git a/components/net/blob_loader.rs b/components/net/blob_loader.rs
index e12cdab9804..09a426fbc65 100644
--- a/components/net/blob_loader.rs
+++ b/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(())));
     }
 }
diff --git a/components/net/filemanager_thread.rs b/components/net/filemanager_thread.rs
index 4be5b6a010d..3f19949f0e3 100644
--- a/components/net/filemanager_thread.rs
+++ b/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<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;
+
+        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,
+        });
 
-                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 id = Uuid::new_v4();
 
-                // 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 filename = file_path.file_name();
-
-                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),