diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index b1ad01b81e0..65c173fce3b 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -737,7 +737,7 @@ impl RangeRequestBounds { RangeRequestBounds::Final(pos) => { if let Some(len) = len { if pos.start <= len as i64 { - return Ok(pos.clone()); + return Ok(*pos); } } Err("Tried to process RangeRequestBounds::Final without len") diff --git a/components/script/dom/blob.rs b/components/script/dom/blob.rs index 27aa382c3fc..c5c5c480707 100644 --- a/components/script/dom/blob.rs +++ b/components/script/dom/blob.rs @@ -7,7 +7,7 @@ use std::ptr; use std::rc::Rc; use base::id::{BlobId, BlobIndex}; -use constellation_traits::BlobImpl; +use constellation_traits::{BlobData, BlobImpl}; use dom_struct::dom_struct; use encoding_rs::UTF_8; use js::jsapi::JSObject; @@ -33,7 +33,7 @@ use crate::dom::readablestream::ReadableStream; use crate::realms::{AlreadyInRealm, InRealm}; use crate::script_runtime::CanGc; -// https://w3c.github.io/FileAPI/#blob +/// #[dom_struct] pub(crate) struct Blob { reflector_: Reflector, @@ -198,7 +198,7 @@ impl BlobMethods for Blob { self.get_stream(can_gc) } - // https://w3c.github.io/FileAPI/#slice-method-algo + /// fn Slice( &self, start: Option, @@ -206,11 +206,24 @@ impl BlobMethods for Blob { content_type: Option, can_gc: CanGc, ) -> DomRoot { - let type_string = - normalize_type_string(content_type.unwrap_or(DOMString::from("")).as_ref()); - let rel_pos = RelativePos::from_opts(start, end); - let blob_impl = BlobImpl::new_sliced(rel_pos, self.blob_id, type_string); - Blob::new(&self.global(), blob_impl, can_gc) + let global = self.global(); + let type_string = normalize_type_string(&content_type.unwrap_or_default()); + + // If our parent is already a sliced blob then we reference the data from the grandparent instead, + // to keep the blob ancestry chain short. + let (parent, range) = match *global.get_blob_data(&self.blob_id) { + BlobData::Sliced(grandparent, parent_range) => { + let range = RelativePos { + start: parent_range.start + start.unwrap_or_default(), + end: end.map(|end| end + parent_range.start).or(parent_range.end), + }; + (grandparent, range) + }, + _ => (self.blob_id, RelativePos::from_opts(start, end)), + }; + + let blob_impl = BlobImpl::new_sliced(range, parent, type_string); + Blob::new(&global, blob_impl, can_gc) } // https://w3c.github.io/FileAPI/#text-method-algo diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 527d03eed4e..98c4c3ed53d 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::cell::{Cell, OnceCell}; +use std::cell::{Cell, OnceCell, Ref}; use std::collections::hash_map::Entry; use std::collections::{HashMap, VecDeque}; use std::ops::Index; @@ -1821,12 +1821,8 @@ impl GlobalScope { /// In the case of a File-backed blob, this might incur synchronous read and caching. pub(crate) fn get_blob_bytes(&self, blob_id: &BlobId) -> Result, ()> { let parent = { - let blob_state = self.blob_state.borrow(); - let blob_info = blob_state - .get(blob_id) - .expect("get_blob_bytes for an unknown blob."); - match blob_info.blob_impl.blob_data() { - BlobData::Sliced(parent, rel_pos) => Some((*parent, rel_pos.clone())), + match *self.get_blob_data(blob_id) { + BlobData::Sliced(parent, rel_pos) => Some((parent, rel_pos)), _ => None, } }; @@ -1840,14 +1836,24 @@ impl GlobalScope { } } + /// Retrieve information about a specific blob from the blob store + /// + /// # Panics + /// This function panics if there is no blob with the given ID. + pub(crate) fn get_blob_data<'a>(&'a self, blob_id: &BlobId) -> Ref<'a, BlobData> { + Ref::map(self.blob_state.borrow(), |blob_state| { + blob_state + .get(blob_id) + .expect("get_blob_impl called for a unknown blob") + .blob_impl + .blob_data() + }) + } + /// Get bytes from a non-sliced blob fn get_blob_bytes_non_sliced(&self, blob_id: &BlobId) -> Result, ()> { - let blob_state = self.blob_state.borrow(); - let blob_info = blob_state - .get(blob_id) - .expect("get_blob_bytes_non_sliced called for a unknown blob."); - match blob_info.blob_impl.blob_data() { - BlobData::File(f) => { + match *self.get_blob_data(blob_id) { + BlobData::File(ref f) => { let (buffer, is_new_buffer) = match f.get_cache() { Some(bytes) => (bytes, false), None => { @@ -1863,7 +1869,7 @@ impl GlobalScope { Ok(buffer) }, - BlobData::Memory(s) => Ok(s.clone()), + BlobData::Memory(ref s) => Ok(s.clone()), BlobData::Sliced(_, _) => panic!("This blob doesn't have a parent."), } } @@ -1876,12 +1882,8 @@ impl GlobalScope { /// TODO: merge with `get_blob_bytes` by way of broader integration with blob streams. fn get_blob_bytes_or_file_id(&self, blob_id: &BlobId) -> BlobResult { let parent = { - let blob_state = self.blob_state.borrow(); - let blob_info = blob_state - .get(blob_id) - .expect("get_blob_bytes_or_file_id for an unknown blob."); - match blob_info.blob_impl.blob_data() { - BlobData::Sliced(parent, rel_pos) => Some((*parent, rel_pos.clone())), + match *self.get_blob_data(blob_id) { + BlobData::Sliced(parent, rel_pos) => Some((parent, rel_pos)), _ => None, } }; @@ -1906,16 +1908,12 @@ impl GlobalScope { /// tweaked for integration with streams. /// TODO: merge with `get_blob_bytes` by way of broader integration with blob streams. fn get_blob_bytes_non_sliced_or_file_id(&self, blob_id: &BlobId) -> BlobResult { - let blob_state = self.blob_state.borrow(); - let blob_info = blob_state - .get(blob_id) - .expect("get_blob_bytes_non_sliced_or_file_id called for a unknown blob."); - match blob_info.blob_impl.blob_data() { - BlobData::File(f) => match f.get_cache() { + match *self.get_blob_data(blob_id) { + BlobData::File(ref f) => match f.get_cache() { Some(bytes) => BlobResult::Bytes(bytes.clone()), None => BlobResult::File(f.get_id(), f.get_size() as usize), }, - BlobData::Memory(s) => BlobResult::Bytes(s.clone()), + BlobData::Memory(ref s) => BlobResult::Bytes(s.clone()), BlobData::Sliced(_, _) => panic!("This blob doesn't have a parent."), } } @@ -1931,39 +1929,27 @@ impl GlobalScope { /// pub(crate) fn get_blob_size(&self, blob_id: &BlobId) -> u64 { - let blob_state = self.blob_state.borrow(); let parent = { - let blob_info = blob_state - .get(blob_id) - .expect("get_blob_size called for a unknown blob."); - match blob_info.blob_impl.blob_data() { - BlobData::Sliced(parent, rel_pos) => Some((*parent, rel_pos.clone())), + match *self.get_blob_data(blob_id) { + BlobData::Sliced(parent, rel_pos) => Some((parent, rel_pos)), _ => None, } }; match parent { Some((parent_id, rel_pos)) => { - let parent_info = blob_state - .get(&parent_id) - .expect("Parent of blob whose size is unknown."); - let parent_size = match parent_info.blob_impl.blob_data() { - BlobData::File(f) => f.get_size(), - BlobData::Memory(v) => v.len() as u64, + let parent_size = match *self.get_blob_data(&parent_id) { + BlobData::File(ref f) => f.get_size(), + BlobData::Memory(ref v) => v.len() as u64, BlobData::Sliced(_, _) => panic!("Blob ancestry should be only one level."), }; rel_pos.to_abs_range(parent_size as usize).len() as u64 }, - None => { - let blob_info = blob_state - .get(blob_id) - .expect("Blob whose size is unknown."); - match blob_info.blob_impl.blob_data() { - BlobData::File(f) => f.get_size(), - BlobData::Memory(v) => v.len() as u64, - BlobData::Sliced(_, _) => { - panic!("It was previously checked that this blob does not have a parent.") - }, - } + None => match *self.get_blob_data(blob_id) { + BlobData::File(ref f) => f.get_size(), + BlobData::Memory(ref v) => v.len() as u64, + BlobData::Sliced(_, _) => { + panic!("It was previously checked that this blob does not have a parent.") + }, }, } } @@ -1979,7 +1965,7 @@ impl GlobalScope { blob_info.has_url = true; match blob_info.blob_impl.blob_data() { - BlobData::Sliced(parent, rel_pos) => Some((*parent, rel_pos.clone())), + BlobData::Sliced(parent, rel_pos) => Some((*parent, *rel_pos)), _ => None, } }; @@ -2020,12 +2006,8 @@ impl GlobalScope { let origin = get_blob_origin(&self.get_url()); let (tx, rx) = profile_ipc::channel(self.time_profiler_chan().clone()).unwrap(); - let msg = FileManagerThreadMsg::AddSlicedURLEntry( - *parent_file_id, - rel_pos.clone(), - tx, - origin.clone(), - ); + let msg = + FileManagerThreadMsg::AddSlicedURLEntry(*parent_file_id, *rel_pos, tx, origin.clone()); self.send_to_file_manager(msg); match rx.recv().expect("File manager thread is down.") { Ok(new_id) => { diff --git a/components/shared/constellation/structured_data/serializable.rs b/components/shared/constellation/structured_data/serializable.rs index abc05ad5758..22370087665 100644 --- a/components/shared/constellation/structured_data/serializable.rs +++ b/components/shared/constellation/structured_data/serializable.rs @@ -221,9 +221,9 @@ impl BlobImpl { } /// Construct a BlobImpl from a slice of a parent. - pub fn new_sliced(rel_pos: RelativePos, parent: BlobId, type_string: String) -> BlobImpl { + pub fn new_sliced(range: RelativePos, parent: BlobId, type_string: String) -> BlobImpl { let blob_id = BlobId::new(); - let blob_data = BlobData::Sliced(parent, rel_pos); + let blob_data = BlobData::Sliced(parent, range); BlobImpl { blob_id, type_string, diff --git a/components/shared/net/filemanager_thread.rs b/components/shared/net/filemanager_thread.rs index e55fbefcefc..0e9014e1670 100644 --- a/components/shared/net/filemanager_thread.rs +++ b/components/shared/net/filemanager_thread.rs @@ -39,7 +39,7 @@ pub enum FileTokenCheck { /// Relative slice positions of a sequence, /// whose semantic should be consistent with (start, end) parameters in /// -#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)] +#[derive(Clone, Copy, Debug, Deserialize, MallocSizeOf, Serialize)] pub struct RelativePos { /// Relative to first byte if non-negative, /// relative to one past last byte if negative, diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 78b1ca5e8b3..7035ae424dc 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -60,6 +60,13 @@ {} ] ], + "slice-blob-twice-crash.html": [ + "94227dcc74d80dae19b39df025708ffc24ee78c5", + [ + null, + {} + ] + ], "test-wait-crash.html": [ "2419da6af0c278a17b9ff974d4418f9e386ef3e0", [ diff --git a/tests/wpt/mozilla/tests/mozilla/slice-blob-twice-crash.html b/tests/wpt/mozilla/tests/mozilla/slice-blob-twice-crash.html new file mode 100644 index 00000000000..94227dcc74d --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/slice-blob-twice-crash.html @@ -0,0 +1,5 @@ + + +