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

@@ -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<T> {
     value: RefCell<T>,
 }

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<HTMLScriptElement>,
     /// https://html.spec.whatwg.org/multipage/#pending-parsing-blocking-script
-    pending_parsing_blocking_script: MutNullableJS<HTMLScriptElement>,
+    pending_parsing_blocking_script: DOMRefCell<Option<PendingScript>>,
     /// Number of stylesheets that block executing the next parser-inserted script
     script_blocking_stylesheets_count: Cell<u32>,
     /// https://html.spec.whatwg.org/multipage/#list-of-scripts-that-will-execute-when-the-document-has-finished-parsing
-    deferred_scripts: DOMRefCell<Vec<JS<HTMLScriptElement>>>,
+    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<Vec<JS<HTMLScriptElement>>>,
+    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<Vec<JS<HTMLScriptElement>>>,
+    asap_scripts_set: DOMRefCell<VecDeque<PendingScript>>,
     /// https://html.spec.whatwg.org/multipage/#concept-n-noscript
     /// True if scripting is enabled for all scripts in this document
     scripting_enabled: Cell<bool>,
@@ -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<ScriptResult>) {
         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() {
+            let ready_to_be_executed = match self.pending_parsing_blocking_script.borrow_mut().as_mut() {
-                if self.script_blocking_stylesheets_count.get() > 0 || !script.is_ready_to_be_executed() {
+                Some(pending) => {
+                    if self.script_blocking_stylesheets_count.get() > 0 {
                         return;
                     }
-                self.pending_parsing_blocking_script.set(None);
+                    if let Some(pair) = pending.take_result() {
-                parser.resume_with_pending_parsing_blocking_script(&script);
+                        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.
+        loop {
             if self.script_blocking_stylesheets_count.get() > 0 {
                 return;
             }
-        let mut deferred_scripts = self.deferred_scripts.borrow_mut();
+            if let Some((element, result)) = self.deferred_scripts.take_next_ready_to_be_executed() {
-        while !deferred_scripts.is_empty() {
+                element.execute(result);
-            {
+            } else {
-                let script = &*deferred_scripts[0];
+                break;
-                // Part of substep 1.
-                if !script.is_ready_to_be_executed() {
-                    return;
             }
-                // Substep 2.
-                script.execute();
-            }
-            // Substep 3.
-            deferred_scripts.remove(0);
-            // Substep 4 (implicit).
         }
+        if self.deferred_scripts.is_empty() {
             // 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() {
-                break;
-            }
-            script.execute();
-            self.asap_in_order_scripts_list.borrow_mut().remove(0);
     }
 
-        let mut idx = 0;
+    /// https://html.spec.whatwg.org/multipage/#the-end step 3.
-        // Re-borrowing the set for each step because it can also be borrowed under execute.
+    /// https://html.spec.whatwg.org/multipage/#prepare-a-script step 22.d.
-        while idx < self.asap_scripts_set.borrow().len() {
+    pub fn asap_script_loaded(&self, element: &HTMLScriptElement, result: ScriptResult) {
-            let script = Root::from_ref(&*self.asap_scripts_set.borrow()[idx]);
+        let mut scripts = self.asap_scripts_set.borrow_mut();
-            if !script.is_ready_to_be_executed() {
+        let idx = scripts.iter().position(|entry| &*entry.element == element).unwrap();
-                idx += 1;
+        scripts.swap(0, idx);
-                continue;
+        scripts[0].loaded(result);
     }
-            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.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![]),
+            deferred_scripts: Default::default(),
-            asap_in_order_scripts_list: DOMRefCell::new(vec![]),
+            asap_in_order_scripts_list: Default::default(),
-            asap_scripts_set: DOMRefCell::new(vec![]),
+            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<VecDeque<PendingScript>>,
+}
+
+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<HTMLScriptElement>, 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<HTMLScriptElement>,
+    load: Option<ScriptResult>,
+}
+
+impl PendingScript {
+    fn new(element: &HTMLScriptElement) -> Self {
+        Self { element: JS::from_ref(element), load: None }
+    }
+
+    fn new_with_load(element: &HTMLScriptElement, load: Option<ScriptResult>) -> 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<HTMLScriptElement>, ScriptResult)> {
+        self.load.take().map(|result| (Root::from_ref(&*self.element), result))
+    }
+}

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<Document>,
-
-    /// The source this script was loaded from
-    load: DOMRefCell<Option<Result<ClassicScript, NetworkError>>>,
 }
 
 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<ClassicScript, NetworkError>;
+
 /// The context required for asynchronously loading an external script source.
 struct ScriptContext {
     /// The element that initiated the request.
     elem: Trusted<HTMLScriptElement>,
+    /// 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<CorsSettings>,
                           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<ClassicScript, NetworkError>) {
         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 result {
-        let script = match load {
             // Step 2.
             Err(e) => {
                 warn!("error loading script {:?}", e);

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() {

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)]

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",

tests/wpt/mozilla/tests/mozilla/nested_asap_script.html

@@ -0,0 +1,15 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Nested ASAP scripts</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+<script>
+var t = async_test("Nested ASAP scripts");
+</script>
+<script async src="nested_asap_script.js"></script>
+</body>
+</html>

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);
+});