From 30f0553ac744789705a3cf67ffce91f2ed5beb3e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 11 Jan 2017 14:13:15 +0100 Subject: [PATCH] Introduce PendingScript This moves scripts' loading results in Document, instead of maintaining them behind a DOMRefCell in each HTMLScriptElement. --- components/script/dom/bindings/cell.rs | 2 +- components/script/dom/document.rs | 203 ++++++++++++------ components/script/dom/htmlscriptelement.rs | 36 ++-- components/script/dom/servoparser/mod.rs | 6 +- components/script/lib.rs | 1 + tests/wpt/mozilla/meta/MANIFEST.json | 6 + .../tests/mozilla/nested_asap_script.html | 15 ++ .../tests/mozilla/nested_asap_script.js | 6 + 8 files changed, 193 insertions(+), 82 deletions(-) create mode 100644 tests/wpt/mozilla/tests/mozilla/nested_asap_script.html create mode 100644 tests/wpt/mozilla/tests/mozilla/nested_asap_script.js diff --git a/components/script/dom/bindings/cell.rs b/components/script/dom/bindings/cell.rs index fc26ca013c86..8e82de943818 100644 --- a/components/script/dom/bindings/cell.rs +++ b/components/script/dom/bindings/cell.rs @@ -11,7 +11,7 @@ use style::thread_state; /// /// This extends the API of `core::cell::RefCell` to allow unsafe access in /// certain situations, with dynamic checking in debug builds. -#[derive(Clone, PartialEq, Debug, HeapSizeOf)] +#[derive(Clone, Debug, Default, HeapSizeOf, PartialEq)] pub struct DOMRefCell { value: RefCell, } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 9c491e8618e4..fc3025615a71 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -60,7 +60,7 @@ use dom::htmlheadelement::HTMLHeadElement; use dom::htmlhtmlelement::HTMLHtmlElement; use dom::htmliframeelement::HTMLIFrameElement; use dom::htmlimageelement::HTMLImageElement; -use dom::htmlscriptelement::HTMLScriptElement; +use dom::htmlscriptelement::{HTMLScriptElement, ScriptResult}; use dom::htmltitleelement::HTMLTitleElement; use dom::keyboardevent::KeyboardEvent; use dom::location::Location; @@ -118,7 +118,7 @@ use servo_url::ServoUrl; use std::ascii::AsciiExt; use std::borrow::ToOwned; use std::cell::{Cell, Ref, RefMut}; -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::default::Default; use std::iter::once; @@ -220,15 +220,15 @@ pub struct Document { /// The script element that is currently executing. current_script: MutNullableJS, /// https://html.spec.whatwg.org/multipage/#pending-parsing-blocking-script - pending_parsing_blocking_script: MutNullableJS, + pending_parsing_blocking_script: DOMRefCell>, /// Number of stylesheets that block executing the next parser-inserted script script_blocking_stylesheets_count: Cell, /// https://html.spec.whatwg.org/multipage/#list-of-scripts-that-will-execute-when-the-document-has-finished-parsing - deferred_scripts: DOMRefCell>>, + deferred_scripts: PendingInOrderScriptVec, /// https://html.spec.whatwg.org/multipage/#list-of-scripts-that-will-execute-in-order-as-soon-as-possible - asap_in_order_scripts_list: DOMRefCell>>, + asap_in_order_scripts_list: PendingInOrderScriptVec, /// https://html.spec.whatwg.org/multipage/#set-of-scripts-that-will-execute-as-soon-as-possible - asap_scripts_set: DOMRefCell>>, + asap_scripts_set: DOMRefCell>, /// https://html.spec.whatwg.org/multipage/#concept-n-noscript /// True if scripting is enabled for all scripts in this document scripting_enabled: Cell, @@ -1450,25 +1450,27 @@ impl Document { changed } - pub fn set_pending_parsing_blocking_script(&self, script: &HTMLScriptElement) { + pub fn set_pending_parsing_blocking_script(&self, + script: &HTMLScriptElement, + load: Option) { assert!(!self.has_pending_parsing_blocking_script()); - self.pending_parsing_blocking_script.set(Some(script)); + *self.pending_parsing_blocking_script.borrow_mut() = Some(PendingScript::new_with_load(script, load)); } pub fn has_pending_parsing_blocking_script(&self) -> bool { - self.pending_parsing_blocking_script.get().is_some() + self.pending_parsing_blocking_script.borrow().is_some() } pub fn add_deferred_script(&self, script: &HTMLScriptElement) { - self.deferred_scripts.borrow_mut().push(JS::from_ref(script)); + self.deferred_scripts.push(script); } pub fn add_asap_script(&self, script: &HTMLScriptElement) { - self.asap_scripts_set.borrow_mut().push(JS::from_ref(script)); + self.asap_scripts_set.borrow_mut().push_back(PendingScript::new(script)); } pub fn push_asap_in_order_script(&self, script: &HTMLScriptElement) { - self.asap_in_order_scripts_list.borrow_mut().push(JS::from_ref(script)); + self.asap_in_order_scripts_list.push(script); } pub fn trigger_mozbrowser_event(&self, event: MozBrowserEvent) { @@ -1573,12 +1575,22 @@ impl Document { } if let Some(parser) = self.get_current_parser() { - if let Some(script) = self.pending_parsing_blocking_script.get() { - if self.script_blocking_stylesheets_count.get() > 0 || !script.is_ready_to_be_executed() { - return; - } - self.pending_parsing_blocking_script.set(None); - parser.resume_with_pending_parsing_blocking_script(&script); + let ready_to_be_executed = match self.pending_parsing_blocking_script.borrow_mut().as_mut() { + Some(pending) => { + if self.script_blocking_stylesheets_count.get() > 0 { + return; + } + if let Some(pair) = pending.take_result() { + Some(pair) + } else { + return; + } + }, + None => None, + }; + if let Some((element, result)) = ready_to_be_executed { + *self.pending_parsing_blocking_script.borrow_mut() = None; + parser.resume_with_pending_parsing_blocking_script(&element, result); } } else if self.reflow_timeout.get().is_none() { // If we don't have a parser, and the reflow timer has been reset, explicitly @@ -1601,58 +1613,66 @@ impl Document { } } - /// https://html.spec.whatwg.org/multipage/#the-end step 3 + /// https://html.spec.whatwg.org/multipage/#prepare-a-script step 22.d. + pub fn pending_parsing_blocking_script_loaded(&self, element: &HTMLScriptElement, result: ScriptResult) { + let mut blocking_script = self.pending_parsing_blocking_script.borrow_mut(); + let entry = blocking_script.as_mut().unwrap(); + assert!(&*entry.element == element); + entry.loaded(result); + } + + /// https://html.spec.whatwg.org/multipage/#prepare-a-script step 22.d. + pub fn deferred_script_loaded(&self, element: &HTMLScriptElement, result: ScriptResult) { + self.deferred_scripts.loaded(element, result); + } + + /// https://html.spec.whatwg.org/multipage/#the-end step 3. pub fn process_deferred_scripts(&self) { if self.ready_state.get() != DocumentReadyState::Interactive { return; } // Part of substep 1. - if self.script_blocking_stylesheets_count.get() > 0 { - return; - } - let mut deferred_scripts = self.deferred_scripts.borrow_mut(); - while !deferred_scripts.is_empty() { - { - let script = &*deferred_scripts[0]; - // Part of substep 1. - if !script.is_ready_to_be_executed() { - return; - } - // Substep 2. - script.execute(); + loop { + if self.script_blocking_stylesheets_count.get() > 0 { + return; } - // Substep 3. - deferred_scripts.remove(0); - // Substep 4 (implicit). - } - // https://html.spec.whatwg.org/multipage/#the-end step 4. - self.maybe_dispatch_dom_content_loaded(); - } - - /// https://html.spec.whatwg.org/multipage/#the-end step 5 and the latter parts of - /// https://html.spec.whatwg.org/multipage/#prepare-a-script 20.d and 20.e. - pub fn process_asap_scripts(&self) { - // Execute the first in-order asap-executed script if it's ready, repeat as required. - // Re-borrowing the list for each step because it can also be borrowed under execute. - while self.asap_in_order_scripts_list.borrow().len() > 0 { - let script = Root::from_ref(&*self.asap_in_order_scripts_list.borrow()[0]); - if !script.is_ready_to_be_executed() { + if let Some((element, result)) = self.deferred_scripts.take_next_ready_to_be_executed() { + element.execute(result); + } else { break; } - script.execute(); - self.asap_in_order_scripts_list.borrow_mut().remove(0); } + if self.deferred_scripts.is_empty() { + // https://html.spec.whatwg.org/multipage/#the-end step 4. + self.maybe_dispatch_dom_content_loaded(); + } + } - let mut idx = 0; - // Re-borrowing the set for each step because it can also be borrowed under execute. - while idx < self.asap_scripts_set.borrow().len() { - let script = Root::from_ref(&*self.asap_scripts_set.borrow()[idx]); - if !script.is_ready_to_be_executed() { - idx += 1; - continue; - } - script.execute(); - self.asap_scripts_set.borrow_mut().swap_remove(idx); + /// https://html.spec.whatwg.org/multipage/#the-end step 3. + /// https://html.spec.whatwg.org/multipage/#prepare-a-script step 22.d. + pub fn asap_script_loaded(&self, element: &HTMLScriptElement, result: ScriptResult) { + let mut scripts = self.asap_scripts_set.borrow_mut(); + let idx = scripts.iter().position(|entry| &*entry.element == element).unwrap(); + scripts.swap(0, idx); + scripts[0].loaded(result); + } + + /// https://html.spec.whatwg.org/multipage/#the-end step 3. + /// https://html.spec.whatwg.org/multipage/#prepare-a-script step 22.c. + pub fn asap_in_order_script_loaded(&self, + element: &HTMLScriptElement, + result: ScriptResult) { + self.asap_in_order_scripts_list.loaded(element, result); + } + + fn process_asap_scripts(&self) { + let pair = self.asap_scripts_set.borrow_mut().front_mut().and_then(PendingScript::take_result); + if let Some((element, result)) = pair { + self.asap_scripts_set.borrow_mut().pop_front(); + element.execute(result); + } + while let Some((element, result)) = self.asap_in_order_scripts_list.take_next_ready_to_be_executed() { + element.execute(result); } } @@ -1889,9 +1909,9 @@ impl Document { current_script: Default::default(), pending_parsing_blocking_script: Default::default(), script_blocking_stylesheets_count: Cell::new(0u32), - deferred_scripts: DOMRefCell::new(vec![]), - asap_in_order_scripts_list: DOMRefCell::new(vec![]), - asap_scripts_set: DOMRefCell::new(vec![]), + deferred_scripts: Default::default(), + asap_in_order_scripts_list: Default::default(), + asap_scripts_set: Default::default(), scripting_enabled: Cell::new(browsing_context.is_some()), animation_frame_ident: Cell::new(0), animation_frame_list: DOMRefCell::new(vec![]), @@ -3379,3 +3399,60 @@ impl AnimationFrameCallback { } } } + +#[derive(Default, HeapSizeOf, JSTraceable)] +#[must_root] +struct PendingInOrderScriptVec { + scripts: DOMRefCell>, +} + +impl PendingInOrderScriptVec { + fn is_empty(&self) -> bool { + self.scripts.borrow().is_empty() + } + fn push(&self, element: &HTMLScriptElement) { + self.scripts.borrow_mut().push_back(PendingScript::new(element)); + } + + fn loaded(&self, element: &HTMLScriptElement, result: ScriptResult) { + let mut scripts = self.scripts.borrow_mut(); + let mut entry = scripts.iter_mut().find(|entry| &*entry.element == element).unwrap(); + entry.loaded(result); + } + + fn take_next_ready_to_be_executed(&self) -> Option<(Root, ScriptResult)> { + let mut scripts = self.scripts.borrow_mut(); + let pair = scripts.front_mut().and_then(PendingScript::take_result); + if pair.is_none() { + return None; + } + scripts.pop_front(); + pair + } +} + +#[derive(HeapSizeOf, JSTraceable)] +#[must_root] +struct PendingScript { + element: JS, + load: Option, +} + +impl PendingScript { + fn new(element: &HTMLScriptElement) -> Self { + Self { element: JS::from_ref(element), load: None } + } + + fn new_with_load(element: &HTMLScriptElement, load: Option) -> Self { + Self { element: JS::from_ref(element), load } + } + + fn loaded(&mut self, result: ScriptResult) { + assert!(self.load.is_none()); + self.load = Some(result); + } + + fn take_result(&mut self) -> Option<(Root, ScriptResult)> { + self.load.take().map(|result| (Root::from_ref(&*self.element), result)) + } +} diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index ead0bb957589..8273aab31f0c 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -4,7 +4,6 @@ use document_loader::LoadType; use dom::attr::Attr; -use dom::bindings::cell::DOMRefCell; use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods; use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods; use dom::bindings::codegen::Bindings::HTMLScriptElementBinding; @@ -61,9 +60,6 @@ pub struct HTMLScriptElement { /// Document of the parser that created this element parser_document: JS, - - /// The source this script was loaded from - load: DOMRefCell>>, } impl HTMLScriptElement { @@ -77,7 +73,6 @@ impl HTMLScriptElement { non_blocking: Cell::new(creator != ElementCreator::ParserCreated), ready_to_be_parser_executed: Cell::new(false), parser_document: JS::from_ref(document), - load: DOMRefCell::new(None), } } @@ -137,10 +132,14 @@ impl ClassicScript { } } +pub type ScriptResult = Result; + /// The context required for asynchronously loading an external script source. struct ScriptContext { /// The element that initiated the request. elem: Trusted, + /// The kind of external script. + kind: ExternalScriptKind, /// The (fallback) character encoding argument to the "fetch a classic /// script" algorithm. character_encoding: EncodingRef, @@ -207,10 +206,16 @@ impl FetchResponseListener for ScriptContext { // https://html.spec.whatwg.org/multipage/#prepare-a-script // Step 18.6 (When the chosen algorithm asynchronously completes). let elem = self.elem.root(); - *elem.load.borrow_mut() = Some(load); elem.ready_to_be_parser_executed.set(true); - let document = document_from_node(&*elem); + + match self.kind { + ExternalScriptKind::Asap => document.asap_script_loaded(&elem, load), + ExternalScriptKind::AsapInOrder => document.asap_in_order_script_loaded(&elem, load), + ExternalScriptKind::Deferred => document.deferred_script_loaded(&elem, load), + ExternalScriptKind::ParsingBlocking => document.pending_parsing_blocking_script_loaded(&elem, load), + } + document.finish_load(LoadType::Script(self.url.clone())); } } @@ -219,6 +224,7 @@ impl PreInvoke for ScriptContext {} /// https://html.spec.whatwg.org/multipage/#fetch-a-classic-script fn fetch_a_classic_script(script: &HTMLScriptElement, + kind: ExternalScriptKind, url: ServoUrl, cors_setting: Option, integrity_metadata: String, @@ -254,6 +260,7 @@ fn fetch_a_classic_script(script: &HTMLScriptElement, let context = Arc::new(Mutex::new(ScriptContext { elem: Trusted::new(script), + kind: kind, character_encoding: character_encoding, data: vec!(), metadata: None, @@ -422,19 +429,19 @@ impl HTMLScriptElement { }; // Step 20.6. - fetch_a_classic_script(self, url, cors_setting, integrity_metadata.to_owned(), encoding); + fetch_a_classic_script(self, kind, url, cors_setting, integrity_metadata.to_owned(), encoding); // Step 22. match kind { ExternalScriptKind::Deferred => doc.add_deferred_script(self), - ExternalScriptKind::ParsingBlocking => doc.set_pending_parsing_blocking_script(self), + ExternalScriptKind::ParsingBlocking => doc.set_pending_parsing_blocking_script(self, None), ExternalScriptKind::AsapInOrder => doc.push_asap_in_order_script(self), ExternalScriptKind::Asap => doc.add_asap_script(self), } } else { // Step 21. assert!(!text.is_empty()); - *self.load.borrow_mut() = Some(Ok(ClassicScript::internal(text, base_url))); + let result = Ok(ClassicScript::internal(text, base_url)); self.ready_to_be_parser_executed.set(true); // Step 22. @@ -442,10 +449,10 @@ impl HTMLScriptElement { doc.get_current_parser().map_or(false, |parser| parser.script_nesting_level() <= 1) && doc.get_script_blocking_stylesheets_count() > 0 { // Step 22.e: classic, has no src, was parser-inserted, is blocked on stylesheet. - doc.set_pending_parsing_blocking_script(self); + doc.set_pending_parsing_blocking_script(self, Some(result)); } else { // Step 22.f: otherwise. - self.execute(); + self.execute(result); } } } @@ -455,7 +462,7 @@ impl HTMLScriptElement { } /// https://html.spec.whatwg.org/multipage/#execute-the-script-block - pub fn execute(&self) { + pub fn execute(&self, result: Result) { assert!(self.ready_to_be_parser_executed.get()); // Step 1. @@ -464,8 +471,7 @@ impl HTMLScriptElement { return; } - let load = self.load.borrow_mut().take().unwrap(); - let script = match load { + let script = match result { // Step 2. Err(e) => { warn!("error loading script {:?}", e); diff --git a/components/script/dom/servoparser/mod.rs b/components/script/dom/servoparser/mod.rs index 5ee23a997f41..a6c278042956 100644 --- a/components/script/dom/servoparser/mod.rs +++ b/components/script/dom/servoparser/mod.rs @@ -19,7 +19,7 @@ use dom::element::Element; use dom::globalscope::GlobalScope; use dom::htmlformelement::HTMLFormElement; use dom::htmlimageelement::HTMLImageElement; -use dom::htmlscriptelement::HTMLScriptElement; +use dom::htmlscriptelement::{HTMLScriptElement, ScriptResult}; use dom::node::{Node, NodeSiblingIterator}; use dom::text::Text; use encoding::all::UTF_8; @@ -171,7 +171,7 @@ impl ServoParser { /// ^ /// insertion point /// ``` - pub fn resume_with_pending_parsing_blocking_script(&self, script: &HTMLScriptElement) { + pub fn resume_with_pending_parsing_blocking_script(&self, script: &HTMLScriptElement, result: ScriptResult) { assert!(self.suspended.get()); self.suspended.set(false); @@ -184,7 +184,7 @@ impl ServoParser { assert_eq!(script_nesting_level, 0); self.script_nesting_level.set(script_nesting_level + 1); - script.execute(); + script.execute(result); self.script_nesting_level.set(script_nesting_level); if !self.suspended.get() { diff --git a/components/script/lib.rs b/components/script/lib.rs index 127194ebf659..f1fbe6f99c55 100644 --- a/components/script/lib.rs +++ b/components/script/lib.rs @@ -7,6 +7,7 @@ #![feature(const_fn)] #![feature(core_intrinsics)] #![feature(field_init_shorthand)] +#![feature(more_struct_aliases)] #![feature(mpsc_select)] #![feature(nonzero)] #![feature(on_unimplemented)] diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index b227f357a1ef..5b3faa75be7a 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -8660,6 +8660,12 @@ "url": "/_mozilla/mozilla/navigator.html" } ], + "mozilla/nested_asap_script.html": [ + { + "path": "mozilla/nested_asap_script.html", + "url": "/_mozilla/mozilla/nested_asap_script.html" + } + ], "mozilla/node_compareDocumentPosition.html": [ { "path": "mozilla/node_compareDocumentPosition.html", diff --git a/tests/wpt/mozilla/tests/mozilla/nested_asap_script.html b/tests/wpt/mozilla/tests/mozilla/nested_asap_script.html new file mode 100644 index 000000000000..1690fbcb9272 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/nested_asap_script.html @@ -0,0 +1,15 @@ + + + + +Nested ASAP scripts + + + + + + + + diff --git a/tests/wpt/mozilla/tests/mozilla/nested_asap_script.js b/tests/wpt/mozilla/tests/mozilla/nested_asap_script.js new file mode 100644 index 000000000000..59562a8c9c39 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/nested_asap_script.js @@ -0,0 +1,6 @@ +t.step(function () { + var s = document.createElement("script"); + s.setAttribute("async", ""); + s.src = "data:text/javascript,t.done()"; + document.body.appendChild(s); +});