From dc5335a21eb70f5a9751ef2f6929ed3731b4ad5e Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Tue, 28 Feb 2017 10:30:13 -0500 Subject: [PATCH] Move checks for document completion to the end of the event loop. This better reflects the text of the specification - rather than queuing a task to dispatch the load evnet as soon as the document loader is unblocked, we want to "spin the event loop until there is nothing that delays the load event in the Document." Spinning the event loop is a concept that requires running tasks completely, hence we check the condition before returning to the start of the event loop. --- components/script/dom/document.rs | 17 +++++++++--- components/script/script_thread.rs | 26 ++++++++++++++++++- .../script_plugins/unrooted_must_root.rs | 3 ++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index c836a46f1da0..6c79bb385789 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -107,7 +107,7 @@ use net_traits::response::HttpsState; use num_traits::ToPrimitive; use script_layout_interface::message::{Msg, ReflowQueryType}; use script_runtime::{CommonScriptMsg, ScriptThreadEventCategory}; -use script_thread::{MainThreadScriptMsg, Runnable}; +use script_thread::{MainThreadScriptMsg, Runnable, ScriptThread}; use script_traits::{AnimationState, CompositorEvent, DocumentActivity}; use script_traits::{MouseButton, MouseEventType, MozBrowserEvent}; use script_traits::{ScriptMsg as ConstellationMsg, TouchpadPressurePhase}; @@ -1599,17 +1599,26 @@ impl Document { // Step 5 can be found in asap_script_loaded and // asap_in_order_script_loaded. - if self.loader.borrow().is_blocked() { + let loader = self.loader.borrow(); + if loader.is_blocked() || loader.events_inhibited() { // Step 6. return; } - // The rest will ever run only once per document. - if self.loader.borrow().events_inhibited() { + ScriptThread::mark_document_with_no_blocked_loads(self); + } + + // https://html.spec.whatwg.org/multipage/#the-end + pub fn maybe_queue_document_completion(&self) { + if self.loader.borrow().is_blocked() { + // Step 6. return; } + + assert!(!self.loader.borrow().events_inhibited()); self.loader.borrow_mut().inhibit_events(); + // The rest will ever run only once per document. // Step 7. debug!("Document loads are complete."); let handler = box DocumentProgressHandler::new(Trusted::new(self)); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 192923294a66..e877e904cebb 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -483,6 +483,10 @@ pub struct ScriptThread { /// A handle to the webvr thread, if available webvr_thread: Option>, + + /// A list of pipelines containing documents that finished loading all their blocking + /// resources during a turn of the event loop. + docs_with_no_blocking_loads: DOMRefCell>>, } /// In the event of thread panic, all data on the stack runs its destructor. However, there @@ -567,6 +571,15 @@ impl ScriptThreadFactory for ScriptThread { } impl ScriptThread { + pub fn mark_document_with_no_blocked_loads(doc: &Document) { + SCRIPT_THREAD_ROOT.with(|root| { + let script_thread = unsafe { &*root.get().unwrap() }; + script_thread.docs_with_no_blocking_loads + .borrow_mut() + .insert(JS::from_ref(doc)); + }) + } + pub fn invoke_perform_a_microtask_checkpoint() { SCRIPT_THREAD_ROOT.with(|root| { let script_thread = unsafe { &*root.get().unwrap() }; @@ -704,7 +717,9 @@ impl ScriptThread { layout_to_constellation_chan: state.layout_to_constellation_chan, - webvr_thread: state.webvr_thread + webvr_thread: state.webvr_thread, + + docs_with_no_blocking_loads: Default::default(), } } @@ -885,6 +900,15 @@ impl ScriptThread { } } + { + // https://html.spec.whatwg.org/multipage/#the-end step 6 + let mut docs = self.docs_with_no_blocking_loads.borrow_mut(); + for document in docs.iter() { + document.maybe_queue_document_completion(); + } + docs.clear(); + } + // https://html.spec.whatwg.org/multipage/#event-loop-processing-model step 7.12 // Issue batched reflows on any pages that require it (e.g. if images loaded) diff --git a/components/script_plugins/unrooted_must_root.rs b/components/script_plugins/unrooted_must_root.rs index ad74f6c4b8fe..c62796b7a90f 100644 --- a/components/script_plugins/unrooted_must_root.rs +++ b/components/script_plugins/unrooted_must_root.rs @@ -54,7 +54,8 @@ fn is_unrooted_ty(cx: &LateContext, ty: &ty::TyS, in_new_function: bool) -> bool || match_def_path(cx, did.did, &["core", "slice", "Iter"]) || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "Entry"]) || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "OccupiedEntry"]) - || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "VacantEntry"]) { + || match_def_path(cx, did.did, &["std", "collections", "hash", "map", "VacantEntry"]) + || match_def_path(cx, did.did, &["std", "collections", "hash", "set", "Iter"]) { // Structures which are semantically similar to an &ptr. false } else if did.is_box() && in_new_function {