Auto merge of #15098 - nox:load-fixes, r=jdm

Mark the page source as loaded only after parsing is done

<!-- 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/15098)
<!-- Reviewable:end -->

components/script/dom/document.rs

@@ -132,6 +132,7 @@ use style::restyle_hints::RestyleHint;
 use style::selector_parser::{RestyleDamage, Snapshot};
 use style::str::{split_html_space_chars, str_join};
 use style::stylesheets::Stylesheet;
+use task_source::TaskSource;
 use time;
 use url::percent_encoding::percent_decode;
 
@@ -1572,13 +1573,11 @@ impl Document {
         }
 
         if !self.loader.borrow().is_blocked() && !self.loader.borrow().events_inhibited() {
-            // Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
+            self.loader.borrow_mut().inhibit_events();
-            // NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
+            // Schedule a task to fire a "load" event.
             debug!("Document loads are complete.");
-            let win = self.window();
+            let handler = box DocumentProgressHandler::new(Trusted::new(self));
-            let msg = MainThreadScriptMsg::DocumentLoadsComplete(
+            self.window.dom_manipulation_task_source().queue(handler, self.window.upcast()).unwrap();
-                win.upcast::<GlobalScope>().pipeline_id());
-            win.main_thread_script_chan().send(msg).unwrap();
         }
     }
 
@@ -3314,6 +3313,9 @@ impl DocumentProgressHandler {
 
     fn dispatch_load(&self) {
         let document = self.addr.root();
+        if document.browsing_context().is_none() {
+            return;
+        }
         let window = document.window();
         let event = Event::new(window.upcast(),
                                atom!("load"),
@@ -3331,7 +3333,6 @@ impl DocumentProgressHandler {
         // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventEnd
         update_with_current_time_ms(&document.load_event_end);
 
-
         window.reflow(ReflowGoal::ForDisplay,
                       ReflowQueryType::NoQuery,
                       ReflowReason::DocumentLoaded);
@@ -3349,6 +3350,9 @@ impl Runnable for DocumentProgressHandler {
         if window.is_alive() {
             self.set_ready_state_complete();
             self.dispatch_load();
+            if let Some(fragment) = document.url().fragment() {
+                document.check_and_scroll_fragment(fragment);
+            }
         }
     }
 }

components/script/dom/servoparser/html.rs

@@ -93,6 +93,10 @@ impl Tokenizer {
         self.inner.end();
     }
 
+    pub fn url(&self) -> &ServoUrl {
+        &self.inner.sink().sink().base_url
+    }
+
     pub fn set_plaintext_state(&mut self) {
         self.inner.set_plaintext_state();
     }

components/script/dom/servoparser/mod.rs

@@ -110,7 +110,8 @@ impl ServoParser {
         let url = context_document.url();
 
         // Step 1.
-        let loader = DocumentLoader::new(&*context_document.loader());
+        let loader = DocumentLoader::new_with_threads(context_document.loader().resource_threads().clone(),
+                                                      Some(url.clone()));
         let document = Document::new(window, None, Some(url.clone()),
                                      context_document.origin().alias(),
                                      IsHTMLDocument::HTMLDocument,
@@ -351,14 +352,17 @@ impl ServoParser {
         self.document.set_current_parser(None);
 
         if self.pipeline.is_some() {
+            // Initial reflow.
             self.document.disarm_reflow_timeout();
             self.document.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
             let window = self.document.window();
             window.reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::FirstLoad);
         }
 
-        // Step 3.
+        // Steps 3-12 are in other castles, namely process_deferred_scripts and finish_load.
+        let url = self.tokenizer.borrow().url().clone();
         self.document.process_deferred_scripts();
+        self.document.finish_load(LoadType::PageSource(url));
     }
 }
 
@@ -401,6 +405,13 @@ impl Tokenizer {
         }
     }
 
+    fn url(&self) -> &ServoUrl {
+        match *self {
+            Tokenizer::Html(ref tokenizer) => tokenizer.url(),
+            Tokenizer::Xml(ref tokenizer) => tokenizer.url(),
+        }
+    }
+
     fn set_plaintext_state(&mut self) {
         match *self {
             Tokenizer::Html(ref mut tokenizer) => tokenizer.set_plaintext_state(),
@@ -558,9 +569,6 @@ impl FetchResponseListener for ParserContext {
             debug!("Failed to load page URL {}, error: {:?}", self.url, err);
         }
 
-        parser.document
-            .finish_load(LoadType::PageSource(self.url.clone()));
-
         parser.last_chunk_received.set(true);
         if !parser.suspended.get() {
             parser.parse_sync();

components/script/dom/servoparser/xml.rs

@@ -71,6 +71,10 @@ impl Tokenizer {
     pub fn end(&mut self) {
         self.inner.end()
     }
+
+    pub fn url(&self) -> &ServoUrl {
+        &self.inner.sink().sink().base_url
+    }
 }
 
 #[allow(unsafe_code)]

components/script/script_thread.rs

@@ -41,7 +41,7 @@ use dom::bindings::str::DOMString;
 use dom::bindings::trace::JSTraceable;
 use dom::bindings::utils::WRAP_CALLBACKS;
 use dom::browsingcontext::BrowsingContext;
-use dom::document::{Document, DocumentProgressHandler, DocumentSource, FocusType, IsHTMLDocument, TouchEventResult};
+use dom::document::{Document, DocumentSource, FocusType, IsHTMLDocument, TouchEventResult};
 use dom::element::Element;
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::globalscope::GlobalScope;
@@ -239,8 +239,6 @@ enum MixedMessage {
 pub enum MainThreadScriptMsg {
     /// Common variants associated with the script messages
     Common(CommonScriptMsg),
-    /// Notify a document that all pending loads are complete.
-    DocumentLoadsComplete(PipelineId),
     /// Notifies the script that a window associated with a particular pipeline
     /// should be closed (only dispatched to ScriptThread).
     ExitWindow(PipelineId),
@@ -1027,8 +1025,6 @@ impl ScriptThread {
                 self.handle_navigate(parent_pipeline_id, None, load_data, replace),
             MainThreadScriptMsg::ExitWindow(id) =>
                 self.handle_exit_window_msg(id),
-            MainThreadScriptMsg::DocumentLoadsComplete(id) =>
-                self.handle_loads_complete(id),
             MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg(_, runnable)) => {
                 // The category of the runnable is ignored by the pattern, however
                 // it is still respected by profiling (see categorize_msg).
@@ -1248,29 +1244,6 @@ impl ScriptThread {
         }
     }
 
-    fn handle_loads_complete(&self, pipeline: PipelineId) {
-        let doc = match { self.documents.borrow().find_document(pipeline) } {
-            Some(doc) => doc,
-            None => return warn!("Message sent to closed pipeline {}.", pipeline),
-        };
-        if doc.loader().is_blocked() {
-            debug!("Script thread got loads complete while loader is blocked.");
-            return;
-        }
-
-        doc.mut_loader().inhibit_events();
-
-        // https://html.spec.whatwg.org/multipage/#the-end step 7
-        // Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
-        // NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
-        let handler = box DocumentProgressHandler::new(Trusted::new(&doc));
-        self.dom_manipulation_task_source.queue(handler, doc.window().upcast()).unwrap();
-
-        if let Some(fragment) = doc.url().fragment() {
-            doc.check_and_scroll_fragment(fragment);
-        };
-    }
-
     fn collect_reports(&self, reports_chan: ReportsChan) {
         let mut path_seg = String::from("url(");
         let mut dom_tree_size = 0;
@@ -1771,7 +1744,7 @@ impl ScriptThread {
         });
 
         let loader = DocumentLoader::new_with_threads(self.resource_threads.clone(),
-                                                      Some(incomplete.url.clone()));
+                                                      Some(final_url.clone()));
 
         let is_html_document = match metadata.content_type {
             Some(Serde(ContentType(Mime(TopLevel::Application, SubLevel::Ext(ref sub_level), _))))

tests/wpt/metadata/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-invoke-pause-networkState.html.ini

@@ -0,0 +1,5 @@
+[resource-selection-invoke-pause-networkState.html]
+  type: testharness
+  [NOT invoking resource selection with pause() when networkState is not NETWORK_EMPTY]
+    expected: FAIL
+

tests/wpt/metadata/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-invoke-remove-from-document-networkState.html.ini

@@ -0,0 +1,5 @@
+[resource-selection-invoke-remove-from-document-networkState.html]
+  type: testharness
+  [NOT invoking resource selection with implicit pause() when networkState is not NETWORK_EMPTY]
+    expected: FAIL
+

tests/wpt/metadata/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-invoke-set-src-not-in-document.html.ini

@@ -0,0 +1,5 @@
+[resource-selection-invoke-set-src-not-in-document.html]
+  type: testharness
+  [invoking load by setting src on video not in a document]
+    expected: FAIL
+

tests/wpt/metadata/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-invoke-set-src.html.ini

@@ -0,0 +1,5 @@
+[resource-selection-invoke-set-src.html]
+  type: testharness
+  [invoking load by setting src]
+    expected: FAIL
+

tests/wpt/metadata/subresource-integrity/subresource-integrity.sub.html.ini

@@ -1,8 +1,6 @@
 [subresource-integrity.sub.html]
   type: testharness
-
   expected: OK
-
   [Style: Same-origin with correct sha256 and sha512 hash, rel='alternate stylesheet' enabled]
     expected: FAIL
 

tests/wpt/web-platform-tests/old-tests/submission/Opera/script_scheduling/078.html

@@ -1,43 +1,43 @@
 <!DOCTYPE html>
 <html>
-	<head>
+<head>
-		<title> adding several types of scripts through the DOM and removing some of them confuses scheduler (slow-loading scripts) </title>
+<title> adding several types of scripts through the DOM and removing some of them confuses scheduler (slow-loading scripts) </title>
-		<script src="/resources/testharness.js"></script>
+<script src="/resources/testharness.js"></script>
-        	<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testharnessreport.js"></script>
-	        <script src="testlib/testlib.js"></script>
+<script src="testlib/testlib.js"></script>
-		<script type="text/javascript">
+<script type="text/javascript">
-                  setup({explicit_done:true});
+    setup({ explicit_done: true });
-			var head=document.getElementsByTagName('head')[0];
+    var head = document.getElementsByTagName('head')[0];
-			function createScript(url, contents) {
+    function createScript(url, contents) {
-				props = {};
+        props = {};
-				if (url) {
+        if (url) {
-                                    props.src = url;
+            props.src = url;
-                                }
+        }
-				return testlib.addScript(contents, props, head, false);
+        return testlib.addScript(contents, props, head, false);
-			}
+    }
-			var t = async_test(undefined, {timeout:10000})
+    var t = async_test(undefined, { timeout: 10000 })
 
-			function test() {
+    function test() {
-                                document.getElementById("log").textContent = "Please wait..."
+        document.getElementById("log").textContent = "Please wait..."
-				var url = 'scripts/include-1.js?pipe=trickle(d1)';
+        var url = 'scripts/include-1.js?pipe=trickle(d1)';
-				var script = createScript(url);
+        var script = createScript(url);
-				var script2 = createScript('', 'log("Script #2 ran")');
+        var script2 = createScript('', 'log("Script #2 ran")');
-				head.removeChild(script2);
+        head.removeChild(script2);
-				var url = 'scripts/include-2.js?pipe=trickle(d2)';
+        var url = 'scripts/include-2.js?pipe=trickle(d2)';
-				var script3 = createScript(url);
+        var script3 = createScript(url);
-				head.removeChild(script3);
+        head.removeChild(script3);
 
-				setTimeout(t.step_func(function() {
+        setTimeout(t.step_func(function () {
-                                               done();
+            done();
-					       assert_array_equals(eventOrder, ['Script #2 ran', 'external script #1', 'external script #2']);
+            assert_array_equals(eventOrder, ['Script #2 ran', 'external script #1', 'external script #2']);
-					       t.done();
+            t.done();
-                                           }), 5500);
+        }), 5500);
 
-			};
+    };
-                        onload = t.step_func(test)
+    onload = t.step_func(test)
-		</script>
+</script>
-	</head>
+</head>
-	<body>
+<body>
-          <div id="log">FAILED (This TC requires JavaScript enabled)</div>
+<div id="log">FAILED (This TC requires JavaScript enabled)</div>
-	</body>
+</body>
-</html*>
+</html>

tests/wpt/web-platform-tests/old-tests/submission/Opera/script_scheduling/119.html

@@ -1,32 +1,31 @@
 <!DOCTYPE html>
 <html><head>
-	<title>scheduler: external defer script created with createContextualFragment</title>
+    <title>scheduler: external defer script created with createContextualFragment</title>
-	<script src="/resources/testharness.js"></script>
+    <script src="/resources/testharness.js"></script>
-	<script src="/resources/testharnessreport.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
-	<script src="testlib/testlib.js"></script>
+    <script src="testlib/testlib.js"></script>
 </head>
-        <div id="log"></div>
+<body>
-	<script>
+    <div id="log"></div>
-                log('inline script #1');
+    <script>
-                var t = async_test();
+        log('inline script #1');
+        var t = async_test();
 
-                t.step(function() {
+        t.step(function () {
-                  var range = document.createRange();
+            var range = document.createRange();
-                  var fragment = range.createContextualFragment("<script defer src='scripts/include-1.js?pipe=trickle(d1)'><\/script>");
+            var fragment = range.createContextualFragment("<script defer src='scripts/include-1.js?pipe=trickle(d1)'><\/script>");
-                  document.body.appendChild(fragment.firstChild);
+            document.body.appendChild(fragment.firstChild);
-                });
+        });
 
-                addEventListener("DOMContentLoaded", t.step_func(function() {
+        addEventListener("DOMContentLoaded", t.step_func(function () {
-			assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1']);
+            assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1']);
-			t.done();
+        }));
-	        }));
 
-                addEventListener("load", t.step_func(function() {
+        addEventListener("load", t.step_func_done(function () {
-			assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1', 'external script #1']);
+            assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1', 'external script #1']);
-			t.done();
+        }));
-	        }));
-              
-
-                log('end inline script #1');
-	</script>
 
+        log('end inline script #1');
+    </script>
+</body>
+</html>