mirror of
https://github.com/servo/servo.git
synced 2025-06-06 16:45:39 +00:00
Auto merge of #12579 - izgzhen:fix-fileapi-ref, r=Manishearth
Fix FileAPI's refcount implementation Revise several intricate parts of FileAPI's internal refcounting-related implementation. Goal: Get it done right once and for all. r? @Manishearth <!-- Please describe your changes on the following line: --> --- <!-- 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 do not require tests because it is internal logic change <!-- 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="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12579) <!-- Reviewable:end -->
This commit is contained in:
commit
93b130f3f7
3 changed files with 119 additions and 90 deletions
|
@ -176,7 +176,7 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
|
|||
if let Ok(id) = Uuid::parse_str(&id.0) {
|
||||
spawn_named("revoke blob url".to_owned(), move || {
|
||||
// Since it is revocation, unset_url_validity is true
|
||||
let _ = sender.send(store.dec_ref(&id, &origin, true));
|
||||
let _ = sender.send(store.set_blob_url_validity(false, &id, &origin));
|
||||
})
|
||||
} else {
|
||||
let _ = sender.send(Err(BlobURLStoreError::InvalidFileID));
|
||||
|
@ -187,23 +187,16 @@ impl<UI: 'static + UIProvider> FileManager<UI> {
|
|||
spawn_named("dec ref".to_owned(), move || {
|
||||
// Since it is simple DecRef (possibly caused by close/drop),
|
||||
// unset_url_validity is false
|
||||
let _ = sender.send(store.dec_ref(&id, &origin, false));
|
||||
let _ = sender.send(store.dec_ref(&id, &origin));
|
||||
})
|
||||
} else {
|
||||
let _ = sender.send(Err(BlobURLStoreError::InvalidFileID));
|
||||
}
|
||||
}
|
||||
FileManagerThreadMsg::IncRef(id, origin) => {
|
||||
if let Ok(id) = Uuid::parse_str(&id.0) {
|
||||
spawn_named("inc ref".to_owned(), move || {
|
||||
let _ = store.inc_ref(&id, &origin);
|
||||
})
|
||||
}
|
||||
}
|
||||
FileManagerThreadMsg::ActivateBlobURL(id, sender, origin) => {
|
||||
if let Ok(id) = Uuid::parse_str(&id.0) {
|
||||
spawn_named("activate blob url".to_owned(), move || {
|
||||
let _ = sender.send(store.activate_blob_url(&id, &origin));
|
||||
let _ = sender.send(store.set_blob_url_validity(true, &id, &origin));
|
||||
});
|
||||
} else {
|
||||
let _ = sender.send(Err(BlobURLStoreError::InvalidFileID));
|
||||
|
@ -285,9 +278,11 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
|
|||
file_impl: FileImpl::Sliced(parent_id, rel_pos),
|
||||
refs: AtomicUsize::new(1),
|
||||
// Valid here since AddSlicedURLEntry implies URL creation
|
||||
// from a BlobImpl::Sliced
|
||||
is_valid_url: AtomicBool::new(true),
|
||||
});
|
||||
|
||||
// We assume that the returned id will be held by BlobImpl::File
|
||||
let _ = sender.send(Ok(SelectedFileId(new_id.simple().to_string())));
|
||||
}
|
||||
Err(e) => {
|
||||
|
@ -474,17 +469,12 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
|
|||
self.get_blob_buf(sender, &id, &origin_in, RelativePos::full_range(), check_url_validity)
|
||||
}
|
||||
|
||||
fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin,
|
||||
unset_url_validity: bool) -> Result<(), BlobURLStoreError> {
|
||||
fn dec_ref(&self, id: &Uuid, origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> {
|
||||
let (do_remove, opt_parent_id) = match self.entries.read().unwrap().get(id) {
|
||||
Some(entry) => {
|
||||
if *entry.origin == *origin_in {
|
||||
let old_refs = entry.refs.fetch_sub(1, Ordering::Release);
|
||||
|
||||
if unset_url_validity {
|
||||
entry.is_valid_url.store(false, Ordering::Release);
|
||||
}
|
||||
|
||||
if old_refs > 1 {
|
||||
// not the last reference, no need to touch parent
|
||||
(false, None)
|
||||
|
@ -514,7 +504,7 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
|
|||
if let Some(parent_id) = opt_parent_id {
|
||||
// unset_url_validity for parent is false since we only need
|
||||
// to unset the initial requesting URL
|
||||
return self.dec_ref(&parent_id, origin_in, false);
|
||||
return self.dec_ref(&parent_id, origin_in);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -542,22 +532,46 @@ impl <UI: 'static + UIProvider> FileManagerStore<UI> {
|
|||
}
|
||||
}
|
||||
|
||||
fn activate_blob_url(&self, id: &Uuid, origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> {
|
||||
match self.entries.read().unwrap().get(id) {
|
||||
fn set_blob_url_validity(&self, validity: bool, id: &Uuid,
|
||||
origin_in: &FileOrigin) -> Result<(), BlobURLStoreError> {
|
||||
let (do_remove, opt_parent_id, res) = match self.entries.read().unwrap().get(id) {
|
||||
Some(entry) => {
|
||||
if *entry.origin == *origin_in {
|
||||
entry.is_valid_url.store(true, Ordering::Release);
|
||||
Ok(())
|
||||
entry.is_valid_url.store(validity, Ordering::Release);
|
||||
|
||||
if !validity {
|
||||
// Check if it is the last possible reference
|
||||
// since refs only accounts for blob id holders
|
||||
// and store entry id holders
|
||||
let zero_refs = entry.refs.load(Ordering::Acquire) == 0;
|
||||
|
||||
if let FileImpl::Sliced(ref parent_id, _) = entry.file_impl {
|
||||
(zero_refs, Some(parent_id.clone()), Ok(()))
|
||||
} else {
|
||||
(zero_refs, None, Ok(()))
|
||||
}
|
||||
} else {
|
||||
(false, None, Ok(()))
|
||||
}
|
||||
} else {
|
||||
Err(BlobURLStoreError::InvalidOrigin)
|
||||
(false, None, Err(BlobURLStoreError::InvalidOrigin))
|
||||
}
|
||||
}
|
||||
None => Err(BlobURLStoreError::InvalidFileID)
|
||||
None => (false, None, Err(BlobURLStoreError::InvalidFileID))
|
||||
};
|
||||
|
||||
if do_remove {
|
||||
atomic::fence(Ordering::Acquire);
|
||||
self.remove(id);
|
||||
|
||||
if let Some(parent_id) = opt_parent_id {
|
||||
return self.dec_ref(&parent_id, origin_in);
|
||||
}
|
||||
}
|
||||
res
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
fn select_files_pref_enabled() -> bool {
|
||||
PREFS.get("dom.testing.htmlinputelement.select_files.enabled")
|
||||
.as_boolean().unwrap_or(false)
|
||||
|
|
|
@ -138,18 +138,15 @@ pub enum FileManagerThreadMsg {
|
|||
/// as part of a valid Blob URL
|
||||
AddSlicedURLEntry(SelectedFileId, RelativePos, IpcSender<Result<SelectedFileId, BlobURLStoreError>>, FileOrigin),
|
||||
|
||||
/// Revoke Blob URL and send back the acknowledgement
|
||||
RevokeBlobURL(SelectedFileId, FileOrigin, IpcSender<Result<(), BlobURLStoreError>>),
|
||||
|
||||
/// Decrease reference count and send back the acknowledgement
|
||||
DecRef(SelectedFileId, FileOrigin, IpcSender<Result<(), BlobURLStoreError>>),
|
||||
|
||||
/// Increase reference count
|
||||
IncRef(SelectedFileId, FileOrigin),
|
||||
|
||||
/// Activate an internal FileID so it becomes valid as part of a Blob URL
|
||||
ActivateBlobURL(SelectedFileId, IpcSender<Result<(), BlobURLStoreError>>, FileOrigin),
|
||||
|
||||
/// Revoke Blob URL and send back the acknowledgement
|
||||
RevokeBlobURL(SelectedFileId, FileOrigin, IpcSender<Result<(), BlobURLStoreError>>),
|
||||
|
||||
/// Shut down this thread
|
||||
Exit,
|
||||
}
|
||||
|
|
|
@ -18,6 +18,7 @@ use net_traits::IpcSend;
|
|||
use net_traits::blob_url_store::{BlobBuf, get_blob_origin};
|
||||
use net_traits::filemanager_thread::{FileManagerThreadMsg, SelectedFileId, RelativePos, ReadFileProgress};
|
||||
use std::cell::Cell;
|
||||
use std::mem;
|
||||
use std::ops::Index;
|
||||
use std::path::PathBuf;
|
||||
use uuid::Uuid;
|
||||
|
@ -26,7 +27,7 @@ use uuid::Uuid;
|
|||
#[derive(JSTraceable)]
|
||||
pub struct FileBlob {
|
||||
id: SelectedFileId,
|
||||
name: PathBuf,
|
||||
name: Option<PathBuf>,
|
||||
cache: DOMRefCell<Option<Vec<u8>>>,
|
||||
size: u64,
|
||||
}
|
||||
|
@ -57,7 +58,7 @@ impl BlobImpl {
|
|||
pub fn new_from_file(file_id: SelectedFileId, name: PathBuf, size: u64) -> BlobImpl {
|
||||
BlobImpl::File(FileBlob {
|
||||
id: file_id,
|
||||
name: name,
|
||||
name: Some(name),
|
||||
cache: DOMRefCell::new(None),
|
||||
size: size,
|
||||
})
|
||||
|
@ -98,9 +99,7 @@ impl Blob {
|
|||
relativeContentType: DOMString) -> Root<Blob> {
|
||||
let global = parent.global();
|
||||
let blob_impl = match *parent.blob_impl.borrow() {
|
||||
BlobImpl::File(ref f) => {
|
||||
inc_ref_id(global.r(), f.id.clone());
|
||||
|
||||
BlobImpl::File(_) => {
|
||||
// Create new parent node
|
||||
BlobImpl::Sliced(JS::from_ref(parent), rel_pos)
|
||||
}
|
||||
|
@ -110,13 +109,7 @@ impl Blob {
|
|||
}
|
||||
BlobImpl::Sliced(ref grandparent, ref old_rel_pos) => {
|
||||
// Adjust the slicing position, using same parent
|
||||
let new_rel_pos = old_rel_pos.slice_inner(&rel_pos);
|
||||
|
||||
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)
|
||||
BlobImpl::Sliced(grandparent.clone(), old_rel_pos.slice_inner(&rel_pos))
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -173,47 +166,56 @@ impl Blob {
|
|||
/// Get a FileID representing the Blob content,
|
||||
/// used by URL.createObjectURL
|
||||
pub fn get_blob_url_id(&self) -> SelectedFileId {
|
||||
match *self.blob_impl.borrow() {
|
||||
BlobImpl::File(ref f) => {
|
||||
let global = self.global();
|
||||
let origin = get_blob_origin(&global.r().get_url());
|
||||
let filemanager = global.r().resource_threads().sender();
|
||||
let (tx, rx) = ipc::channel().unwrap();
|
||||
|
||||
let _ = filemanager.send(FileManagerThreadMsg::ActivateBlobURL(f.id.clone(), tx, origin.clone()));
|
||||
|
||||
match rx.recv().unwrap() {
|
||||
Ok(_) => f.id.clone(),
|
||||
Err(_) => SelectedFileId(Uuid::new_v4().simple().to_string()) // Return a dummy id on error
|
||||
}
|
||||
}
|
||||
BlobImpl::Memory(ref slice) => {
|
||||
self.promote(slice, true)
|
||||
}
|
||||
let opt_sliced_parent = match *self.blob_impl.borrow() {
|
||||
BlobImpl::Sliced(ref parent, ref rel_pos) => {
|
||||
match *parent.blob_impl.borrow() {
|
||||
BlobImpl::Sliced(_, _) => {
|
||||
debug!("Sliced can't have a sliced parent");
|
||||
// Return dummy id
|
||||
SelectedFileId(Uuid::new_v4().simple().to_string())
|
||||
}
|
||||
BlobImpl::File(ref f) =>
|
||||
self.create_sliced_url_id(&f.id, rel_pos),
|
||||
BlobImpl::Memory(ref bytes) => {
|
||||
let parent_id = parent.promote(bytes, false);
|
||||
*self.blob_impl.borrow_mut() = BlobImpl::Sliced(parent.clone(), rel_pos.clone());
|
||||
self.create_sliced_url_id(&parent_id, rel_pos)
|
||||
}
|
||||
}
|
||||
Some((parent.promote(/* set_valid is */ false), rel_pos.clone(), parent.Size()))
|
||||
}
|
||||
_ => None
|
||||
};
|
||||
|
||||
match opt_sliced_parent {
|
||||
Some((parent_id, rel_pos, size)) => self.create_sliced_url_id(&parent_id, &rel_pos, size),
|
||||
None => self.promote(/* set_valid is */ true),
|
||||
}
|
||||
}
|
||||
|
||||
/// Promite memory-based Blob to file-based,
|
||||
/// The bytes in data slice will be transferred to file manager thread.
|
||||
/// Promote non-Slice blob:
|
||||
/// 1. Memory-based: The bytes in data slice will be transferred to file manager thread.
|
||||
/// 2. File-based: Activation
|
||||
/// Depending on set_valid, the returned FileID can be part of
|
||||
/// valid or invalid Blob URL.
|
||||
fn promote(&self, bytes: &[u8], set_valid: bool) -> SelectedFileId {
|
||||
fn promote(&self, set_valid: bool) -> SelectedFileId {
|
||||
let mut bytes = vec![];
|
||||
|
||||
match *self.blob_impl.borrow_mut() {
|
||||
BlobImpl::Sliced(_, _) => {
|
||||
debug!("Sliced can't have a sliced parent");
|
||||
// Return dummy id
|
||||
return SelectedFileId(Uuid::new_v4().simple().to_string());
|
||||
}
|
||||
BlobImpl::File(ref f) => {
|
||||
if set_valid {
|
||||
let global = self.global();
|
||||
let origin = get_blob_origin(&global.r().get_url());
|
||||
let filemanager = global.r().resource_threads().sender();
|
||||
let (tx, rx) = ipc::channel().unwrap();
|
||||
|
||||
let msg = FileManagerThreadMsg::ActivateBlobURL(f.id.clone(), tx, origin.clone());
|
||||
let _ = filemanager.send(msg);
|
||||
|
||||
match rx.recv().unwrap() {
|
||||
Ok(_) => return f.id.clone(),
|
||||
// Return a dummy id on error
|
||||
Err(_) => return SelectedFileId(Uuid::new_v4().simple().to_string())
|
||||
}
|
||||
} else {
|
||||
// no need to activate
|
||||
return f.id.clone();
|
||||
}
|
||||
}
|
||||
BlobImpl::Memory(ref mut bytes_in) => mem::swap(bytes_in, &mut bytes),
|
||||
};
|
||||
|
||||
let global = self.global();
|
||||
let origin = get_blob_origin(&global.r().get_url());
|
||||
let filemanager = global.r().resource_threads().sender();
|
||||
|
@ -221,7 +223,7 @@ impl Blob {
|
|||
let blob_buf = BlobBuf {
|
||||
filename: None,
|
||||
type_string: self.typeString.clone(),
|
||||
size: self.Size(),
|
||||
size: bytes.len() as u64,
|
||||
bytes: bytes.to_vec(),
|
||||
};
|
||||
|
||||
|
@ -229,7 +231,16 @@ impl Blob {
|
|||
let _ = filemanager.send(FileManagerThreadMsg::PromoteMemory(blob_buf, set_valid, tx, origin.clone()));
|
||||
|
||||
match rx.recv().unwrap() {
|
||||
Ok(new_id) => SelectedFileId(new_id.0),
|
||||
Ok(id) => {
|
||||
let id = SelectedFileId(id.0);
|
||||
*self.blob_impl.borrow_mut() = BlobImpl::File(FileBlob {
|
||||
id: id.clone(),
|
||||
name: None,
|
||||
cache: DOMRefCell::new(Some(bytes.to_vec())),
|
||||
size: bytes.len() as u64,
|
||||
});
|
||||
id
|
||||
}
|
||||
// Dummy id
|
||||
Err(_) => SelectedFileId(Uuid::new_v4().simple().to_string()),
|
||||
}
|
||||
|
@ -237,7 +248,7 @@ impl Blob {
|
|||
|
||||
/// Get a FileID representing sliced parent-blob content
|
||||
fn create_sliced_url_id(&self, parent_id: &SelectedFileId,
|
||||
rel_pos: &RelativePos) -> SelectedFileId {
|
||||
rel_pos: &RelativePos, parent_len: u64) -> SelectedFileId {
|
||||
let global = self.global();
|
||||
|
||||
let origin = get_blob_origin(&global.r().get_url());
|
||||
|
@ -248,10 +259,25 @@ impl Blob {
|
|||
rel_pos.clone(),
|
||||
tx, origin.clone());
|
||||
let _ = filemanager.send(msg);
|
||||
let new_id = rx.recv().unwrap().unwrap();
|
||||
match rx.recv().expect("File manager thread is down") {
|
||||
Ok(new_id) => {
|
||||
let new_id = SelectedFileId(new_id.0);
|
||||
|
||||
// Return the indirect id reference
|
||||
SelectedFileId(new_id.0)
|
||||
*self.blob_impl.borrow_mut() = BlobImpl::File(FileBlob {
|
||||
id: new_id.clone(),
|
||||
name: None,
|
||||
cache: DOMRefCell::new(None),
|
||||
size: rel_pos.to_abs_range(parent_len as usize).len() as u64,
|
||||
});
|
||||
|
||||
// Return the indirect id reference
|
||||
new_id
|
||||
}
|
||||
Err(_) => {
|
||||
// Return dummy id
|
||||
SelectedFileId(Uuid::new_v4().simple().to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Cleanups at the time of destruction/closing
|
||||
|
@ -392,11 +418,3 @@ fn is_ascii_printable(string: &str) -> bool {
|
|||
// https://w3c.github.io/FileAPI/#constructorBlob
|
||||
string.chars().all(|c| c >= '\x20' && c <= '\x7E')
|
||||
}
|
||||
|
||||
/// Bump the reference counter in file manager thread
|
||||
fn inc_ref_id(global: GlobalRef, id: SelectedFileId) {
|
||||
let file_manager = global.filemanager_thread();
|
||||
let origin = get_blob_origin(&global.get_url());
|
||||
let msg = FileManagerThreadMsg::IncRef(id, origin);
|
||||
let _ = file_manager.send(msg);
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue