Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Use IndexSet for storing descendants
Fixes intermittent failures in `/html/semantics/scripting-1/the-script-element/module/choice-of-error-1.html`
  • Loading branch information
Manishearth committed Jan 6, 2020
1 parent e7bc0fa commit 9460b43
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 12 deletions.
9 changes: 9 additions & 0 deletions components/script/dom/bindings/trace.rs
Expand Up @@ -311,6 +311,15 @@ unsafe impl<T: JSTraceable> JSTraceable for VecDeque<T> {
}
}

unsafe impl<T: JSTraceable + Eq + Hash> JSTraceable for indexmap::IndexSet<T> {
#[inline]
unsafe fn trace(&self, trc: *mut JSTracer) {
for e in self.iter() {
e.trace(trc);
}
}
}

unsafe impl<A, B, C, D> JSTraceable for (A, B, C, D)
where
A: JSTraceable,
Expand Down
34 changes: 22 additions & 12 deletions components/script/script_module.rs
Expand Up @@ -70,6 +70,8 @@ use std::rc::Rc;
use std::sync::{Arc, Mutex};
use url::ParseError as UrlParseError;

use indexmap::IndexSet;

pub fn get_source_text(source: &[u16]) -> SourceText<u16> {
SourceText {
units_: source.as_ptr() as *const _,
Expand Down Expand Up @@ -162,8 +164,16 @@ pub struct ModuleTree {
text: DomRefCell<DOMString>,
record: DomRefCell<Option<ModuleObject>>,
status: DomRefCell<ModuleStatus>,
parent_urls: DomRefCell<HashSet<ServoUrl>>,
descendant_urls: DomRefCell<HashSet<ServoUrl>>,
// The spec maintains load order for descendants, so we use an indexset for descendants and
// parents. This isn't actually necessary for parents however the IndexSet APIs don't
// interop with HashSet, and IndexSet isn't very expensive
// (https://github.com/bluss/indexmap/issues/110)
//
// By default all maps in web specs are ordered maps
// (https://infra.spec.whatwg.org/#ordered-map), however we can usually get away with using
// stdlib maps and sets because we rarely iterate over them.
parent_urls: DomRefCell<IndexSet<ServoUrl>>,
descendant_urls: DomRefCell<IndexSet<ServoUrl>>,
visited_urls: DomRefCell<HashSet<ServoUrl>>,
error: DomRefCell<Option<ModuleError>>,
promise: DomRefCell<Option<Rc<Promise>>>,
Expand All @@ -176,8 +186,8 @@ impl ModuleTree {
text: DomRefCell::new(DOMString::new()),
record: DomRefCell::new(None),
status: DomRefCell::new(ModuleStatus::Initial),
parent_urls: DomRefCell::new(HashSet::new()),
descendant_urls: DomRefCell::new(HashSet::new()),
parent_urls: DomRefCell::new(IndexSet::new()),
descendant_urls: DomRefCell::new(IndexSet::new()),
visited_urls: DomRefCell::new(HashSet::new()),
error: DomRefCell::new(None),
promise: DomRefCell::new(None),
Expand Down Expand Up @@ -224,23 +234,23 @@ impl ModuleTree {
*self.text.borrow_mut() = module_text;
}

pub fn get_parent_urls(&self) -> &DomRefCell<HashSet<ServoUrl>> {
pub fn get_parent_urls(&self) -> &DomRefCell<IndexSet<ServoUrl>> {
&self.parent_urls
}

pub fn insert_parent_url(&self, parent_url: ServoUrl) {
self.parent_urls.borrow_mut().insert(parent_url);
}

pub fn append_parent_urls(&self, parent_urls: HashSet<ServoUrl>) {
pub fn append_parent_urls(&self, parent_urls: IndexSet<ServoUrl>) {
self.parent_urls.borrow_mut().extend(parent_urls);
}

pub fn get_descendant_urls(&self) -> &DomRefCell<HashSet<ServoUrl>> {
pub fn get_descendant_urls(&self) -> &DomRefCell<IndexSet<ServoUrl>> {
&self.descendant_urls
}

pub fn append_descendant_urls(&self, descendant_urls: HashSet<ServoUrl>) {
pub fn append_descendant_urls(&self, descendant_urls: IndexSet<ServoUrl>) {
self.descendant_urls.borrow_mut().extend(descendant_urls);
}

Expand Down Expand Up @@ -474,7 +484,7 @@ impl ModuleTree {
pub fn resolve_requested_modules(
&self,
global: &GlobalScope,
) -> Result<HashSet<ServoUrl>, ModuleError> {
) -> Result<IndexSet<ServoUrl>, ModuleError> {
let status = self.get_status();

assert_ne!(status, ModuleStatus::Initial);
Expand Down Expand Up @@ -503,7 +513,7 @@ impl ModuleTree {
None
}
})
.collect::<HashSet<ServoUrl>>()
.collect::<IndexSet<ServoUrl>>()
});
}

Expand All @@ -516,10 +526,10 @@ impl ModuleTree {
global: &GlobalScope,
module_object: HandleObject,
base_url: ServoUrl,
) -> Result<HashSet<ServoUrl>, ModuleError> {
) -> Result<IndexSet<ServoUrl>, ModuleError> {
let _ac = JSAutoRealm::new(*global.get_cx(), *global.reflector().get_jsobject());

let mut specifier_urls = HashSet::new();
let mut specifier_urls = IndexSet::new();

unsafe {
rooted!(in(*global.get_cx()) let requested_modules = GetRequestedModules(*global.get_cx(), module_object));
Expand Down

0 comments on commit 9460b43

Please sign in to comment.