Skip to content

Commit

Permalink
Move checks for document completion to the end of the event loop.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jdm committed Mar 7, 2017
1 parent 2bd02fe commit dc5335a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 6 deletions.
17 changes: 13 additions & 4 deletions components/script/dom/document.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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));
Expand Down
26 changes: 25 additions & 1 deletion components/script/script_thread.rs
Expand Up @@ -483,6 +483,10 @@ pub struct ScriptThread {

/// A handle to the webvr thread, if available
webvr_thread: Option<IpcSender<WebVRMsg>>,

/// 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<HashSet<JS<Document>>>,
}

/// In the event of thread panic, all data on the stack runs its destructor. However, there
Expand Down Expand Up @@ -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() };
Expand Down Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion components/script_plugins/unrooted_must_root.rs
Expand Up @@ -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 {
Expand Down

0 comments on commit dc5335a

Please sign in to comment.