Changed blob to use DataSlice with Arc in order to limit wasteful copying of byte vector

components/script/dom/blob.rs

@@ -8,104 +8,30 @@ use dom::bindings::error::Fallible;
 use dom::bindings::global::{GlobalField, GlobalRef};
 use dom::bindings::js::Root;
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
+use dom::bindings::trace::JSTraceable;
 use num::ToPrimitive;
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::cell::Cell;
 use std::cmp::{max, min};
-use std::sync::mpsc::Sender;
+use std::sync::Arc;
 use util::str::DOMString;
 
-// https://w3c.github.io/FileAPI/#blob
-#[dom_struct]
-pub struct Blob {
-    reflector_: Reflector,
-    bytes: Option<Vec<u8>>,
-    typeString: String,
-    global: GlobalField,
-    isClosed_: Cell<bool>,
+#[derive(Clone, JSTraceable)]
+pub struct DataSlice {
+    bytes: Arc<Vec<u8>>,
+    bytes_start: usize,
+    bytes_end: usize
 }
 
-fn is_ascii_printable(string: &str) -> bool {
-    // Step 5.1 in Sec 5.1 of File API spec
-    // https://w3c.github.io/FileAPI/#constructorBlob
-    string.chars().all(|c| c >= '\x20' && c <= '\x7E')
-}
-
-impl Blob {
-    pub fn new_inherited(global: GlobalRef, bytes: Option<Vec<u8>>, typeString: &str) -> Blob {
-        Blob {
-            reflector_: Reflector::new(),
-            bytes: bytes,
-            typeString: typeString.to_owned(),
-            global: GlobalField::from_rooted(&global),
-            isClosed_: Cell::new(false),
-        }
-    }
-
-    pub fn new(global: GlobalRef, bytes: Option<Vec<u8>>, typeString: &str) -> Root<Blob> {
-        reflect_dom_object(box Blob::new_inherited(global, bytes, typeString),
-                           global,
-                           BlobBinding::Wrap)
-    }
-
-    // https://w3c.github.io/FileAPI/#constructorBlob
-    pub fn Constructor(global: GlobalRef) -> Fallible<Root<Blob>> {
-        Ok(Blob::new(global, None, ""))
-    }
-
-    // https://w3c.github.io/FileAPI/#constructorBlob
-    pub fn Constructor_(global: GlobalRef,
-                        blobParts: DOMString,
-                        blobPropertyBag: &BlobBinding::BlobPropertyBag)
-                        -> Fallible<Root<Blob>> {
-        // TODO: accept other blobParts types - ArrayBuffer or ArrayBufferView or Blob
-        // FIXME(ajeffrey): convert directly from a DOMString to a Vec<u8>
-        let bytes: Option<Vec<u8>> = Some(String::from(blobParts).into_bytes());
-        let typeString = if is_ascii_printable(&blobPropertyBag.type_) {
-            &*blobPropertyBag.type_
-        } else {
-            ""
-        };
-        Ok(Blob::new(global, bytes, &typeString.to_ascii_lowercase()))
-    }
-
-    pub fn read_out_buffer(&self, send: Sender<Vec<u8>>) {
-        send.send(self.bytes.clone().unwrap_or(vec![])).unwrap();
-    }
-
-    // simpler to use version of read_out_buffer
-    pub fn clone_bytes(&self) -> Vec<u8> {
-        self.bytes.clone().unwrap_or(vec![])
-    }
-}
-
-impl BlobMethods for Blob {
-    // https://w3c.github.io/FileAPI/#dfn-size
-    fn Size(&self) -> u64 {
-        match self.bytes {
-            None => 0,
-            Some(ref bytes) => bytes.len() as u64,
-        }
-    }
-
-    // https://w3c.github.io/FileAPI/#dfn-type
-    fn Type(&self) -> DOMString {
-        DOMString::from(self.typeString.clone())
-    }
-
-    // https://w3c.github.io/FileAPI/#slice-method-algo
-    fn Slice(&self,
-             start: Option<i64>,
-             end: Option<i64>,
-             contentType: Option<DOMString>)
-             -> Root<Blob> {
-        let size: i64 = self.Size().to_i64().unwrap();
+impl DataSlice {
+    pub fn new(bytes: Arc<Vec<u8>>, start: Option<i64>, end: Option<i64>) -> DataSlice {
+        let size = bytes.len() as i64;
         let relativeStart: i64 = match start {
             None => 0,
             Some(start) => {
                 if start < 0 {
-                    max(size.to_i64().unwrap() + start, 0)
+                    max(size + start, 0)
                 } else {
                     min(start, size)
                 }
@@ -121,6 +47,119 @@ impl BlobMethods for Blob {
                 }
             }
         };
+
+        let span: i64 = max(relativeEnd - relativeStart, 0);
+        let start = relativeStart.to_usize().unwrap();
+        let end = (relativeStart + span).to_usize().unwrap();
+
+        DataSlice {
+            bytes: bytes,
+            bytes_start: start,
+            bytes_end: end
+        }
+    }
+
+    pub fn get_bytes(&self) -> &[u8] {
+        &self.bytes[self.bytes_start..self.bytes_end]
+    }
+
+    pub fn size(&self) -> u64 {
+        (self.bytes_end as u64) - (self.bytes_start as u64)
+    }
+}
+
+
+// https://w3c.github.io/FileAPI/#blob
+#[dom_struct]
+pub struct Blob {
+    reflector_: Reflector,
+    #[ignore_heap_size_of = "No clear owner"]
+    data: DataSlice,
+    typeString: String,
+    global: GlobalField,
+    isClosed_: Cell<bool>,
+}
+
+fn is_ascii_printable(string: &str) -> bool {
+    // Step 5.1 in Sec 5.1 of File API spec
+    // https://w3c.github.io/FileAPI/#constructorBlob
+    string.chars().all(|c| c >= '\x20' && c <= '\x7E')
+}
+
+impl Blob {
+    pub fn new_inherited(global: GlobalRef,
+                         bytes: Arc<Vec<u8>>,
+                         bytes_start: Option<i64>,
+                         bytes_end: Option<i64>,
+                         typeString: &str) -> Blob {
+        Blob {
+            reflector_: Reflector::new(),
+            data: DataSlice::new(bytes, bytes_start, bytes_end),
+            typeString: typeString.to_owned(),
+            global: GlobalField::from_rooted(&global),
+            isClosed_: Cell::new(false),
+        }
+    }
+
+    pub fn new(global: GlobalRef, bytes: Vec<u8>, typeString: &str) -> Root<Blob> {
+        let boxed_blob = box Blob::new_inherited(global, Arc::new(bytes), None, None, typeString);
+        reflect_dom_object(boxed_blob, global, BlobBinding::Wrap)
+    }
+
+    fn new_sliced(global: GlobalRef,
+                  bytes: Arc<Vec<u8>>,
+                  bytes_start: Option<i64>,
+                  bytes_end: Option<i64>,
+                  typeString: &str) -> Root<Blob> {
+
+      let boxed_blob = box Blob::new_inherited(global, bytes, bytes_start, bytes_end, typeString);
+      reflect_dom_object(boxed_blob, global, BlobBinding::Wrap)
+    }
+
+    // https://w3c.github.io/FileAPI/#constructorBlob
+    pub fn Constructor(global: GlobalRef) -> Fallible<Root<Blob>> {
+        Ok(Blob::new(global, Vec::new(), ""))
+    }
+
+    // https://w3c.github.io/FileAPI/#constructorBlob
+    pub fn Constructor_(global: GlobalRef,
+                        blobParts: DOMString,
+                        blobPropertyBag: &BlobBinding::BlobPropertyBag)
+                        -> Fallible<Root<Blob>> {
+        // TODO: accept other blobParts types - ArrayBuffer or ArrayBufferView or Blob
+        // FIXME(ajeffrey): convert directly from a DOMString to a Vec<u8>
+        let bytes: Vec<u8> = String::from(blobParts).into_bytes();
+        let typeString = if is_ascii_printable(&blobPropertyBag.type_) {
+            &*blobPropertyBag.type_
+        } else {
+            ""
+        };
+        Ok(Blob::new(global, bytes, &typeString.to_ascii_lowercase()))
+    }
+
+    pub fn get_data(&self) -> &DataSlice {
+        &self.data
+    }
+}
+
+impl BlobMethods for Blob {
+    // https://w3c.github.io/FileAPI/#dfn-size
+    fn Size(&self) -> u64 {
+        self.data.size()
+    }
+
+    // https://w3c.github.io/FileAPI/#dfn-type
+    fn Type(&self) -> DOMString {
+        DOMString::from(self.typeString.clone())
+    }
+
+    // https://w3c.github.io/FileAPI/#slice-method-algo
+    fn Slice(&self,
+             start: Option<i64>,
+             end: Option<i64>,
+             contentType: Option<DOMString>)
+             -> Root<Blob> {
+
         let relativeContentType = match contentType {
             None => DOMString::new(),
             Some(mut str) => {
@@ -132,18 +171,9 @@ impl BlobMethods for Blob {
                 }
             }
         };
-        let span: i64 = max(relativeEnd - relativeStart, 0);
         let global = self.global.root();
-        match self.bytes {
-            None => Blob::new(global.r(), None, &relativeContentType),
-            Some(ref vec) => {
-                let start = relativeStart.to_usize().unwrap();
-                let end = (relativeStart + span).to_usize().unwrap();
-                let mut bytes: Vec<u8> = Vec::new();
-                bytes.extend_from_slice(&vec[start..end]);
-                Blob::new(global.r(), Some(bytes), &relativeContentType)
-            }
-        }
+        let bytes = self.data.bytes.clone();
+        Blob::new_sliced(global.r(), bytes, start, end, &relativeContentType)
     }
 
     // https://w3c.github.io/FileAPI/#dfn-isClosed

components/script/dom/file.rs

@@ -8,6 +8,7 @@ use dom::bindings::global::GlobalRef;
 use dom::bindings::js::Root;
 use dom::bindings::reflector::reflect_dom_object;
 use dom::blob::Blob;
+use std::sync::Arc;
 use util::str::DOMString;
 
 #[dom_struct]
@@ -21,7 +22,7 @@ impl File {
                      _file_bits: &Blob, name: DOMString) -> File {
         File {
             //TODO: get type from the underlying filesystem instead of "".to_string()
-            blob: Blob::new_inherited(global, None, ""),
+            blob: Blob::new_inherited(global, Arc::new(Vec::new()), None, None, ""),
             name: name,
         }
         // XXXManishearth Once Blob is able to store data

components/script/dom/filereader.rs

@@ -12,7 +12,7 @@ use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, MutNullableHeap, Root};
 use dom::bindings::refcounted::Trusted;
 use dom::bindings::reflector::{Reflectable, reflect_dom_object};
-use dom::blob::Blob;
+use dom::blob::{Blob, DataSlice};
 use dom::domexception::{DOMErrorName, DOMException};
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::eventtarget::EventTarget;
@@ -23,10 +23,8 @@ use encoding::types::{DecoderTrap, EncodingRef};
 use hyper::mime::{Attr, Mime};
 use rustc_serialize::base64::{CharacterSet, Config, Newline, ToBase64};
 use script_task::ScriptTaskEventCategory::FileRead;
-use script_task::{CommonScriptMsg, Runnable, ScriptChan, ScriptPort};
+use script_task::{CommonScriptMsg, Runnable, ScriptChan};
 use std::cell::Cell;
-use std::sync::mpsc;
-use std::sync::mpsc::Receiver;
 use string_cache::Atom;
 use util::str::DOMString;
 use util::task::spawn_named;
@@ -163,7 +161,7 @@ impl FileReader {
 
     // https://w3c.github.io/FileAPI/#dfn-readAsText
     pub fn process_read_eof(filereader: TrustedFileReader, gen_id: GenerationId,
-                            data: ReadMetaData, blob_contents: Vec<u8>) {
+                            data: ReadMetaData, blob_contents: DataSlice) {
         let fr = filereader.root();
 
         macro_rules! return_on_abort(
@@ -178,11 +176,13 @@ impl FileReader {
         // Step 8.1
         fr.change_ready_state(FileReaderReadyState::Done);
         // Step 8.2
+
+        let bytes = blob_contents.get_bytes();
         let output = match data.function {
             FileReaderFunction::ReadAsDataUrl =>
-                FileReader::perform_readasdataurl(data, blob_contents),
+                FileReader::perform_readasdataurl(data, bytes),
             FileReaderFunction::ReadAsText =>
-                FileReader::perform_readastext(data, blob_contents),
+                FileReader::perform_readastext(data, bytes),
         };
 
         *fr.result.borrow_mut() = Some(output);
@@ -200,12 +200,11 @@ impl FileReader {
     }
 
     // https://w3c.github.io/FileAPI/#dfn-readAsText
-    fn perform_readastext(data: ReadMetaData, blob_contents: Vec<u8>)
+    fn perform_readastext(data: ReadMetaData, blob_bytes: &[u8])
         -> DOMString {
 
         let blob_label = &data.label;
         let blob_type = &data.blobtype;
-        let blob_bytes = &blob_contents[..];
 
         //https://w3c.github.io/FileAPI/#encoding-determination
         // Steps 1 & 2 & 3
@@ -233,7 +232,7 @@ impl FileReader {
     }
 
     //https://w3c.github.io/FileAPI/#dfn-readAsDataURL
-    fn perform_readasdataurl(data: ReadMetaData, blob_contents: Vec<u8>)
+    fn perform_readasdataurl(data: ReadMetaData, bytes: &[u8])
         -> DOMString {
         let config = Config {
             char_set: CharacterSet::UrlSafe,
@@ -241,7 +240,7 @@ impl FileReader {
             pad: true,
             line_length: None
         };
-        let base64 = blob_contents.to_base64(config);
+        let base64 = bytes.to_base64(config);
 
         let output = if data.blobtype.is_empty() {
             format!("data:base64,{}", base64)
@@ -355,8 +354,8 @@ impl FileReader {
         self.change_ready_state(FileReaderReadyState::Loading);
 
         // Step 4
-        let (send, bytes) = mpsc::channel();
-        blob.read_out_buffer(send);
+        let blob_contents = blob.get_data().clone();
+
         let type_ = blob.Type();
 
         let load_data = ReadMetaData::new(String::from(type_), label.map(String::from), function);
@@ -367,7 +366,7 @@ impl FileReader {
         let script_chan = global.file_reading_task_source();
 
         spawn_named("file reader async operation".to_owned(), move || {
-            perform_annotated_read_operation(gen_id, load_data, bytes, fr, script_chan)
+            perform_annotated_read_operation(gen_id, load_data, blob_contents, fr, script_chan)
         });
         Ok(())
     }
@@ -382,7 +381,7 @@ pub enum FileReaderEvent {
     ProcessRead(TrustedFileReader, GenerationId),
     ProcessReadData(TrustedFileReader, GenerationId),
     ProcessReadError(TrustedFileReader, GenerationId, DOMErrorName),
-    ProcessReadEOF(TrustedFileReader, GenerationId, ReadMetaData, Vec<u8>)
+    ProcessReadEOF(TrustedFileReader, GenerationId, ReadMetaData, DataSlice)
 }
 
 impl Runnable for FileReaderEvent {
@@ -406,7 +405,7 @@ impl Runnable for FileReaderEvent {
 }
 
 // https://w3c.github.io/FileAPI/#task-read-operation
-fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, blob_contents: Receiver<Vec<u8>>,
+fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, blob_contents: DataSlice,
     filereader: TrustedFileReader, script_chan: Box<ScriptChan + Send>) {
     let chan = &script_chan;
     // Step 4
@@ -416,16 +415,6 @@ fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, bl
     let task = box FileReaderEvent::ProcessReadData(filereader.clone(), gen_id);
     chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
 
-    let bytes = match blob_contents.recv() {
-        Ok(bytes) => bytes,
-        Err(_) => {
-            let task = box FileReaderEvent::ProcessReadError(filereader,
-                gen_id, DOMErrorName::NotFoundError);
-            chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
-            return;
-        }
-    };
-
-    let task = box FileReaderEvent::ProcessReadEOF(filereader, gen_id, data, bytes);
+    let task = box FileReaderEvent::ProcessReadEOF(filereader, gen_id, data, blob_contents);
     chan.send(CommonScriptMsg::RunnableMsg(FileRead, task)).unwrap();
 }

components/script/dom/testbinding.rs

@@ -85,7 +85,7 @@ impl TestBindingMethods for TestBinding {
     fn EnumAttribute(&self) -> TestEnum { TestEnum::_empty }
     fn SetEnumAttribute(&self, _: TestEnum) {}
     fn InterfaceAttribute(&self) -> Root<Blob> {
-        Blob::new(global_root_from_reflector(self).r(), None, "")
+        Blob::new(global_root_from_reflector(self).r(), Vec::new(), "")
     }
     fn SetInterfaceAttribute(&self, _: &Blob) {}
     fn UnionAttribute(&self) -> HTMLElementOrLong { HTMLElementOrLong::eLong(0) }
@@ -143,7 +143,7 @@ impl TestBindingMethods for TestBinding {
     fn SetAttr_to_automatically_rename(&self, _: DOMString) {}
     fn GetEnumAttributeNullable(&self) -> Option<TestEnum> { Some(TestEnum::_empty) }
     fn GetInterfaceAttributeNullable(&self) -> Option<Root<Blob>> {
-        Some(Blob::new(global_root_from_reflector(self).r(), None, ""))
+        Some(Blob::new(global_root_from_reflector(self).r(), Vec::new(), ""))
     }
     fn SetInterfaceAttributeNullable(&self, _: Option<&Blob>) {}
     fn GetInterfaceAttributeWeak(&self) -> Option<Root<URL>> {
@@ -182,7 +182,7 @@ impl TestBindingMethods for TestBinding {
     fn ReceiveByteString(&self) -> ByteString { ByteString::new(vec!()) }
     fn ReceiveEnum(&self) -> TestEnum { TestEnum::_empty }
     fn ReceiveInterface(&self) -> Root<Blob> {
-        Blob::new(global_root_from_reflector(self).r(), None, "")
+        Blob::new(global_root_from_reflector(self).r(), Vec::new(), "")
     }
     fn ReceiveAny(&self, _: *mut JSContext) -> JSVal { NullValue() }
     fn ReceiveObject(&self, _: *mut JSContext) -> *mut JSObject { panic!() }
@@ -207,7 +207,7 @@ impl TestBindingMethods for TestBinding {
     fn ReceiveNullableByteString(&self) -> Option<ByteString> { Some(ByteString::new(vec!())) }
     fn ReceiveNullableEnum(&self) -> Option<TestEnum> { Some(TestEnum::_empty) }
     fn ReceiveNullableInterface(&self) -> Option<Root<Blob>> {
-        Some(Blob::new(global_root_from_reflector(self).r(), None, ""))
+        Some(Blob::new(global_root_from_reflector(self).r(), Vec::new(), ""))
     }
     fn ReceiveNullableObject(&self, _: *mut JSContext) -> *mut JSObject { ptr::null_mut() }
     fn ReceiveNullableUnion(&self) -> Option<HTMLElementOrLong> {

components/script/dom/websocket.rs

@@ -381,19 +381,20 @@ impl WebSocketMethods for WebSocket {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-websocket-send
-    fn Send_(&self, data: &Blob) -> Fallible<()> {
+    fn Send_(&self, blob: &Blob) -> Fallible<()> {
 
         /* As per https://html.spec.whatwg.org/multipage/#websocket
            the buffered amount needs to be clamped to u32, even though Blob.Size() is u64
            If the buffer limit is reached in the first place, there are likely other major problems
         */
-        let data_byte_len = data.Size();
+        let data_byte_len = blob.Size();
         let send_data = try!(self.send_impl(data_byte_len));
 
         if send_data {
             let mut other_sender = self.sender.borrow_mut();
             let my_sender = other_sender.as_mut().unwrap();
-            let _ = my_sender.send(WebSocketDomAction::SendMessage(MessageData::Binary(data.clone_bytes())));
+            let bytes = blob.get_data().get_bytes().to_vec();
+            let _ = my_sender.send(WebSocketDomAction::SendMessage(MessageData::Binary(bytes)));
         }
 
         Ok(())
@@ -579,7 +580,7 @@ impl Runnable for MessageReceivedTask {
                 MessageData::Binary(data) => {
                     match ws.binary_type.get() {
                         BinaryType::Blob => {
-                            let blob = Blob::new(global.r(), Some(data), "");
+                            let blob = Blob::new(global.r(), data, "");
                             blob.to_jsval(cx, message.handle_mut());
                         }
                         BinaryType::Arraybuffer => {

tests/unit/script/dom/blob.rs

@@ -0,0 +1,34 @@
+/* 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 script::dom::blob::{DataSlice};
+use std::sync::Arc;
+
+#[test]
+fn test_data_slice_without_start_end_should_match_buffer_size() {
+    let bytes = Arc::new(vec![1u8, 2u8, 3u8]);
+    let data = DataSlice::new(bytes, None, None);
+    assert_eq!(data.size(), 3);
+}
+
+#[test]
+fn test_data_slice_should_prevent_reverse_bounds() {
+    let bytes = Arc::new(vec![1u8, 2, 3, 4, 5]);
+    let start = Some(3);
+    let end = Some(1);
+
+    let data = DataSlice::new(bytes, start, end);
+    assert_eq!(data.size(), 0);
+}
+
+#[test]
+fn test_data_slice_should_respect_correct_bounds() {
+    let bytes = Arc::new(vec![1u8, 2, 3, 4, 5]);
+    let start = Some(1);
+    let end = Some(3);
+
+    let data = DataSlice::new(bytes, start, end);
+    let expected = [2u8, 3];
+    assert_eq!(&expected, data.get_bytes());
+}

tests/unit/script/lib.rs

@@ -8,3 +8,6 @@ extern crate util;
 
 #[cfg(all(test, target_pointer_width = "64"))] mod size_of;
 #[cfg(test)] mod textinput;
+#[cfg(test)] mod dom {
+    mod blob;
+}