Auto merge of #12258 - izgzhen:remove-data-slice, r=Manishearth

Remove DataSlice, fix #12249

r? @Manishearth

---
<!-- 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
- [x] These changes fix #12249

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12258)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-07-05 06:24:46 -07:00 committed by GitHub
commit 87c7772527
12 changed files with 61 additions and 163 deletions

View file

@ -135,8 +135,8 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
Err(_) => { let _ = sender.send(Err(FileManagerThreadError::ReadFileError)); }
}
}
FileManagerThreadMsg::TransferMemory(entry, rel_pos, sender, origin) =>
self.transfer_memory(entry, rel_pos, sender, origin),
FileManagerThreadMsg::TransferMemory(entry, sender, origin) =>
self.transfer_memory(entry, sender, origin),
FileManagerThreadMsg::AddSlicedEntry(id, rel_pos, sender, origin) =>
self.add_sliced_entry(id, rel_pos, sender, origin),
FileManagerThreadMsg::LoadBlob(load_data, consumer) => {
@ -418,8 +418,9 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
}
}
fn transfer_memory(&mut self, entry: BlobURLStoreEntry, rel_pos: RelativePos,
sender: IpcSender<Result<SelectedFileId, BlobURLStoreError>>, origin: FileOrigin) {
fn transfer_memory(&mut self, entry: BlobURLStoreEntry,
sender: IpcSender<Result<SelectedFileId, BlobURLStoreError>>,
origin: FileOrigin) {
match Url::parse(&origin) { // parse to check sanity
Ok(_) => {
let id = Uuid::new_v4();
@ -428,9 +429,8 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
file_impl: FileImpl::Memory(entry),
refs: Cell::new(1),
});
let sliced_id = SelectedFileId(id.simple().to_string());
self.add_sliced_entry(sliced_id, rel_pos, sender, origin);
let _ = sender.send(Ok(SelectedFileId(id.simple().to_string())));
}
Err(_) => {
let _ = sender.send(Err(BlobURLStoreError::InvalidOrigin));

View file

@ -130,7 +130,7 @@ pub enum FileManagerThreadMsg {
LoadBlob(LoadData, LoadConsumer),
/// Add an entry and send back the associated uuid
TransferMemory(BlobURLStoreEntry, RelativePos, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
TransferMemory(BlobURLStoreEntry, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
/// Add a sliced entry pointing to the parent id with a relative slicing positing
AddSlicedEntry(SelectedFileId, RelativePos, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),

View file

@ -19,71 +19,15 @@ use net_traits::blob_url_store::BlobURLStoreEntry;
use net_traits::filemanager_thread::{FileManagerThreadMsg, SelectedFileId, RelativePos};
use std::ascii::AsciiExt;
use std::cell::Cell;
use std::ops::Range;
use std::sync::Arc;
#[derive(Clone, JSTraceable)]
pub struct DataSlice {
bytes: Arc<Vec<u8>>,
bytes_start: usize,
bytes_end: usize
}
impl DataSlice {
/// Construct DataSlice from reference counted bytes
pub fn new(bytes: Arc<Vec<u8>>, start: Option<i64>, end: Option<i64>) -> DataSlice {
let range = RelativePos::from_opts(start, end).to_abs_range(bytes.len());
DataSlice {
bytes: bytes,
bytes_start: range.start,
bytes_end: range.end,
}
}
/// Construct data slice from a vector of bytes
pub fn from_bytes(bytes: Vec<u8>) -> DataSlice {
DataSlice::new(Arc::new(bytes), None, None)
}
/// Construct an empty data slice
pub fn empty() -> DataSlice {
DataSlice {
bytes: Arc::new(Vec::new()),
bytes_start: 0,
bytes_end: 0,
}
}
/// Get sliced bytes
pub fn get_bytes(&self) -> &[u8] {
&self.bytes[self.bytes_start..self.bytes_end]
}
/// Get length of sliced bytes
pub fn size(&self) -> u64 {
(self.bytes_end as u64) - (self.bytes_start as u64)
}
/// Further adjust the slice range based on passed-in relative positions
pub fn slice(&self, pos: &RelativePos) -> DataSlice {
let old_size = self.size();
let range = pos.to_abs_range(old_size as usize);
DataSlice {
bytes: self.bytes.clone(),
bytes_start: self.bytes_start + range.start,
bytes_end: self.bytes_start + range.end,
}
}
}
use std::ops::Index;
#[must_root]
#[derive(JSTraceable)]
pub enum BlobImpl {
/// File-based blob, including id and possibly cached content
File(SelectedFileId, DOMRefCell<Option<DataSlice>>),
File(SelectedFileId, DOMRefCell<Option<Vec<u8>>>),
/// Memory-based blob
Memory(DataSlice),
Memory(Vec<u8>),
/// Sliced blob, including parent blob and
/// relative positions representing current slicing range,
/// it is leaf of a two-layer fat tree
@ -91,20 +35,16 @@ pub enum BlobImpl {
}
impl BlobImpl {
/// Construct memory-backed BlobImpl from DataSlice
pub fn new_from_slice(slice: DataSlice) -> BlobImpl {
BlobImpl::Memory(slice)
/// 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))
}
/// Construct empty, memory-backed BlobImpl
pub fn new_from_empty_slice() -> BlobImpl {
BlobImpl::new_from_slice(DataSlice::empty())
}
}
// https://w3c.github.io/FileAPI/#blob
@ -178,12 +118,11 @@ impl Blob {
}
};
let slice = DataSlice::from_bytes(bytes);
Ok(Blob::new(global, BlobImpl::new_from_slice(slice), blobPropertyBag.get_typestring()))
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_slice(&self) -> Result<DataSlice, ()> {
pub fn get_bytes(&self) -> Result<Vec<u8>, ()> {
match *self.blob_impl.borrow() {
BlobImpl::File(ref id, ref cached) => {
let buffer = match *cached.borrow() {
@ -204,17 +143,14 @@ impl Blob {
}
BlobImpl::Memory(ref s) => Ok(s.clone()),
BlobImpl::Sliced(ref parent, ref rel_pos) => {
let dataslice = parent.get_slice_or_empty();
Ok(dataslice.slice(rel_pos))
parent.get_bytes().map(|v| {
let range = rel_pos.to_abs_range(v.len());
v.index(range).to_vec()
})
}
}
}
/// Try to get a slice, and if any exception happens, return the empty slice
pub fn get_slice_or_empty(&self) -> DataSlice {
self.get_slice().unwrap_or(DataSlice::empty())
}
pub fn get_id(&self) -> SelectedFileId {
match *self.blob_impl.borrow() {
BlobImpl::File(ref id, _) => id.clone(),
@ -228,8 +164,8 @@ impl Blob {
}
BlobImpl::File(ref parent_id, _) =>
self.create_sliced_id(parent_id, rel_pos),
BlobImpl::Memory(ref parent_slice) => {
let parent_id = parent.promote_to_file(parent_slice);
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_id(&parent_id, rel_pos)
}
@ -240,15 +176,10 @@ 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, self_slice: &DataSlice) -> SelectedFileId {
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 bytes = self_slice.get_bytes();
let rel_pos = RelativePos::from_abs_range(Range {
start: self_slice.bytes_start,
end: self_slice.bytes_end,
}, self_slice.bytes.len());
let entry = BlobURLStoreEntry {
type_string: self.typeString.clone(),
@ -257,7 +188,7 @@ impl Blob {
};
let (tx, rx) = ipc::channel().unwrap();
let _ = filemanager.send(FileManagerThreadMsg::TransferMemory(entry, rel_pos, tx, origin.clone()));
let _ = filemanager.send(FileManagerThreadMsg::TransferMemory(entry, tx, origin.clone()));
match rx.recv().unwrap() {
Ok(new_id) => SelectedFileId(new_id.0),
@ -285,7 +216,7 @@ impl Blob {
}
}
fn read_file(global: GlobalRef, id: SelectedFileId) -> Result<DataSlice, ()> {
fn read_file(global: GlobalRef, id: SelectedFileId) -> Result<Vec<u8>, ()> {
let file_manager = global.filemanager_thread();
let (chan, recv) = ipc::channel().map_err(|_|())?;
let origin = global.get_url().origin().unicode_serialization();
@ -300,8 +231,7 @@ fn read_file(global: GlobalRef, id: SelectedFileId) -> Result<DataSlice, ()> {
}
};
let bytes = result.map_err(|_|())?;
Ok(DataSlice::from_bytes(bytes))
result.map_err(|_|())
}
/// Extract bytes from BlobParts, used by Blob and File constructor
@ -316,7 +246,8 @@ pub fn blob_parts_to_bytes(blobparts: Vec<BlobOrString>) -> Result<Vec<u8>, ()>
ret.append(&mut bytes);
},
&BlobOrString::Blob(ref b) => {
ret.append(&mut b.get_slice_or_empty().bytes.to_vec());
let mut bytes = b.get_bytes().unwrap_or(vec![]);
ret.append(&mut bytes);
},
}
}
@ -327,7 +258,11 @@ pub fn blob_parts_to_bytes(blobparts: Vec<BlobOrString>) -> Result<Vec<u8>, ()>
impl BlobMethods for Blob {
// https://w3c.github.io/FileAPI/#dfn-size
fn Size(&self) -> u64 {
self.get_slice_or_empty().size()
// XXX: This will incur reading if file-based
match self.get_bytes() {
Ok(s) => s.len() as u64,
_ => 0,
}
}
// https://w3c.github.io/FileAPI/#dfn-type

View file

@ -10,7 +10,7 @@ use dom::bindings::global::GlobalRef;
use dom::bindings::js::Root;
use dom::bindings::reflector::reflect_dom_object;
use dom::bindings::str::DOMString;
use dom::blob::{Blob, BlobImpl, DataSlice, blob_parts_to_bytes};
use dom::blob::{Blob, BlobImpl, blob_parts_to_bytes};
use dom::window::Window;
use net_traits::filemanager_thread::SelectedFile;
use time;
@ -71,9 +71,8 @@ impl File {
let ref blobPropertyBag = filePropertyBag.parent;
let typeString = blobPropertyBag.get_typestring();
let slice = DataSlice::from_bytes(bytes);
let modified = filePropertyBag.lastModified;
Ok(File::new(global, BlobImpl::new_from_slice(slice), filename, modified, &typeString))
Ok(File::new(global, BlobImpl::new_from_bytes(bytes), filename, modified, &typeString))
}
pub fn name(&self) -> &DOMString {

View file

@ -13,7 +13,7 @@ use dom::bindings::js::{JS, MutNullableHeap, Root};
use dom::bindings::refcounted::Trusted;
use dom::bindings::reflector::{Reflectable, reflect_dom_object};
use dom::bindings::str::DOMString;
use dom::blob::{Blob, DataSlice};
use dom::blob::Blob;
use dom::domexception::{DOMErrorName, DOMException};
use dom::event::{Event, EventBubbles, EventCancelable};
use dom::eventtarget::EventTarget;
@ -27,6 +27,7 @@ use script_runtime::ScriptThreadEventCategory::FileRead;
use script_runtime::{ScriptChan, CommonScriptMsg};
use script_thread::Runnable;
use std::cell::Cell;
use std::sync::Arc;
use string_cache::Atom;
use util::thread::spawn_named;
@ -160,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: DataSlice) {
data: ReadMetaData, blob_contents: Arc<Vec<u8>>) {
let fr = filereader.root();
macro_rules! return_on_abort(
@ -176,12 +177,11 @@ impl FileReader {
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, bytes),
FileReader::perform_readasdataurl(data, &blob_contents),
FileReaderFunction::ReadAsText =>
FileReader::perform_readastext(data, bytes),
FileReader::perform_readastext(data, &blob_contents),
};
*fr.result.borrow_mut() = Some(output);
@ -349,7 +349,7 @@ impl FileReader {
self.change_ready_state(FileReaderReadyState::Loading);
// Step 4
let blob_contents = blob.get_slice_or_empty();
let blob_contents = Arc::new(blob.get_bytes().unwrap_or(vec![]));
let type_ = blob.Type();
@ -376,7 +376,7 @@ pub enum FileReaderEvent {
ProcessRead(TrustedFileReader, GenerationId),
ProcessReadData(TrustedFileReader, GenerationId),
ProcessReadError(TrustedFileReader, GenerationId, DOMErrorName),
ProcessReadEOF(TrustedFileReader, GenerationId, ReadMetaData, DataSlice)
ProcessReadEOF(TrustedFileReader, GenerationId, ReadMetaData, Arc<Vec<u8>>)
}
impl Runnable for FileReaderEvent {
@ -400,7 +400,7 @@ impl Runnable for FileReaderEvent {
}
// https://w3c.github.io/FileAPI/#thread-read-operation
fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, blob_contents: DataSlice,
fn perform_annotated_read_operation(gen_id: GenerationId, data: ReadMetaData, blob_contents: Arc<Vec<u8>>,
filereader: TrustedFileReader, script_chan: Box<ScriptChan + Send>) {
let chan = &script_chan;
// Step 4

View file

@ -126,8 +126,9 @@ impl FormData {
Some(fname) => {
let global = self.global();
let name = DOMString::from(fname.0);
let slice = blob.get_slice_or_empty();
Root::upcast(File::new(global.r(), BlobImpl::new_from_slice(slice), name, None, ""))
let bytes = blob.get_bytes().unwrap_or(vec![]);
Root::upcast(File::new(global.r(), BlobImpl::new_from_bytes(bytes), name, None, ""))
}
None => Root::from_ref(blob)
}

View file

@ -324,9 +324,9 @@ impl HTMLFormElement {
content_disposition,
content_type));
let slice = f.upcast::<Blob>().get_slice_or_empty();
let bytes = &f.upcast::<Blob>().get_bytes().unwrap_or(vec![])[..];
let decoded = encoding.decode(&slice.get_bytes(), DecoderTrap::Replace)
let decoded = encoding.decode(bytes, DecoderTrap::Replace)
.expect("Invalid encoding in file");
result.push_str(&decoded);
}

View file

@ -101,7 +101,7 @@ impl TestBindingMethods for TestBinding {
fn EnumAttribute(&self) -> TestEnum { TestEnum::_empty }
fn SetEnumAttribute(&self, _: TestEnum) {}
fn InterfaceAttribute(&self) -> Root<Blob> {
Blob::new(self.global().r(), BlobImpl::new_from_empty_slice(), "".to_owned())
Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned())
}
fn SetInterfaceAttribute(&self, _: &Blob) {}
fn UnionAttribute(&self) -> HTMLElementOrLong { HTMLElementOrLong::Long(0) }
@ -179,7 +179,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(self.global().r(), BlobImpl::new_from_empty_slice(), "".to_owned()))
Some(Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned()))
}
fn SetInterfaceAttributeNullable(&self, _: Option<&Blob>) {}
fn GetInterfaceAttributeWeak(&self) -> Option<Root<URL>> {
@ -230,7 +230,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(self.global().r(), BlobImpl::new_from_empty_slice(), "".to_owned())
Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned())
}
fn ReceiveAny(&self, _: *mut JSContext) -> JSVal { NullValue() }
fn ReceiveObject(&self, _: *mut JSContext) -> *mut JSObject { panic!() }
@ -247,7 +247,7 @@ impl TestBindingMethods for TestBinding {
}
fn ReceiveSequence(&self) -> Vec<i32> { vec![1] }
fn ReceiveInterfaceSequence(&self) -> Vec<Root<Blob>> {
vec![Blob::new(self.global().r(), BlobImpl::new_from_empty_slice(), "".to_owned())]
vec![Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned())]
}
fn ReceiveNullableBoolean(&self) -> Option<bool> { Some(false) }
@ -268,7 +268,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(self.global().r(), BlobImpl::new_from_empty_slice(), "".to_owned()))
Some(Blob::new(self.global().r(), BlobImpl::new_from_bytes(vec![]), "".to_owned()))
}
fn ReceiveNullableObject(&self, _: *mut JSContext) -> *mut JSObject { ptr::null_mut() }
fn ReceiveNullableUnion(&self) -> Option<HTMLElementOrLong> {

View file

@ -16,7 +16,7 @@ use dom::bindings::js::Root;
use dom::bindings::refcounted::Trusted;
use dom::bindings::reflector::{Reflectable, reflect_dom_object};
use dom::bindings::str::{DOMString, USVString, is_token};
use dom::blob::{Blob, BlobImpl, DataSlice};
use dom::blob::{Blob, BlobImpl};
use dom::closeevent::CloseEvent;
use dom::event::{Event, EventBubbles, EventCancelable};
use dom::eventtarget::EventTarget;
@ -405,7 +405,7 @@ impl WebSocketMethods for WebSocket {
if send_data {
let mut other_sender = self.sender.borrow_mut();
let my_sender = other_sender.as_mut().unwrap();
let bytes = blob.get_slice_or_empty().get_bytes().to_vec();
let bytes = blob.get_bytes().unwrap_or(vec![]);
let _ = my_sender.send(WebSocketDomAction::SendMessage(MessageData::Binary(bytes)));
}
@ -591,8 +591,7 @@ impl Runnable for MessageReceivedTask {
MessageData::Binary(data) => {
match ws.binary_type.get() {
BinaryType::Blob => {
let slice = DataSlice::from_bytes(data);
let blob = Blob::new(global.r(), BlobImpl::new_from_slice(slice), "".to_owned());
let blob = Blob::new(global.r(), BlobImpl::new_from_bytes(data), "".to_owned());
blob.to_jsval(cx, message.handle_mut());
}
BinaryType::Arraybuffer => {

View file

@ -20,7 +20,7 @@ use dom::bindings::js::{Root, RootedReference};
use dom::bindings::refcounted::Trusted;
use dom::bindings::reflector::{Reflectable, reflect_dom_object};
use dom::bindings::str::{ByteString, DOMString, USVString, is_token};
use dom::blob::{Blob, DataSlice, BlobImpl};
use dom::blob::{Blob, BlobImpl};
use dom::document::DocumentSource;
use dom::document::{Document, IsHTMLDocument};
use dom::event::{Event, EventBubbles, EventCancelable};
@ -1105,8 +1105,8 @@ impl XMLHttpRequest {
let mime = self.final_mime_type().as_ref().map(Mime::to_string).unwrap_or("".to_owned());
// Step 3, 4
let slice = DataSlice::from_bytes(self.response.borrow().to_vec());
let blob = Blob::new(self.global().r(), BlobImpl::new_from_slice(slice), mime);
let bytes = self.response.borrow().to_vec();
let blob = Blob::new(self.global().r(), BlobImpl::new_from_bytes(bytes), mime);
self.response_blob.set(Some(blob.r()));
blob
}
@ -1375,7 +1375,8 @@ impl Extractable for BodyInit {
} else {
Some(b.Type())
};
(b.get_slice_or_empty().get_bytes().to_vec(), content_type)
let bytes = b.get_bytes().unwrap_or(vec![]);
(bytes, content_type)
},
}
}

View file

@ -1,34 +0,0 @@
/* 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());
}

View file

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