From e6958d3947adedda7f3f52f26fae321f0b4d0ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 24 Jul 2016 04:26:54 -0700 Subject: [PATCH] script: Fix a few load related bugs. This is what was making me hit the new test failures. So turns out that when the DOMContentLoaded event is fired we fired no messages to the constellation, but we fired the DOMLoad message from the DocumentProgressHandler, effectively after having dispatched the Load message from script thread. This also fixes the possibility of a subframe navigation not blocking the load event of the parent document, for example. --- components/compositing/compositor.rs | 2 +- components/constellation/constellation.rs | 10 ++++++---- components/script/dom/document.rs | 22 +++++++++++++++------- components/script/script_thread.rs | 2 -- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index da9272d08b9..60b64a03047 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -2114,7 +2114,7 @@ impl IOCompositor { } // Check if there are any pending frames. If so, the image is not stable yet. - if self.pending_subpages.len() > 0 { + if !self.pending_subpages.is_empty() { return Err(NotReadyToPaint::PendingSubpages(self.pending_subpages.len())); } diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 7b1ae044921..f954fda7668 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1441,6 +1441,7 @@ impl Constellation let root = self.root_frame_id.is_none() || self.root_frame_id == Some(frame_id); self.compositor_proxy.send(ToCompositorMsg::LoadComplete(back, forward, root)); } + self.handle_subframe_loaded(pipeline_id); } fn handle_dom_load(&mut self, pipeline_id: PipelineId) { @@ -1455,8 +1456,6 @@ impl Constellation if webdriver_reset { self.webdriver.load_channel = None; } - - self.handle_subframe_loaded(pipeline_id); } fn handle_traverse_history_msg(&mut self, @@ -2073,7 +2072,7 @@ impl Constellation } // If there are pending loads, wait for those to complete. - if self.pending_frames.len() > 0 { + if !self.pending_frames.is_empty() { return ReadyToSave::PendingFrames; } @@ -2089,7 +2088,10 @@ impl Constellation let pipeline_id = frame.current.0; let pipeline = match self.pipelines.get(&pipeline_id) { - None => { warn!("Pipeline {:?} screenshot while closing.", pipeline_id); continue; }, + None => { + warn!("Pipeline {:?} screenshot while closing.", pipeline_id); + continue; + }, Some(pipeline) => pipeline, }; diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 28edce553ad..14e80de6346 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -376,6 +376,7 @@ impl Document { // that workable. match self.GetDocumentElement() { Some(root) => { + root.upcast::().is_dirty() || root.upcast::().has_dirty_descendants() || !self.modified_elements.borrow().is_empty() } @@ -1371,6 +1372,7 @@ impl Document { } pub fn finish_load(&self, load: LoadType) { + debug!("Document got finish_load: {:?}", load); // The parser might need the loader, so restrict the lifetime of the borrow. { let mut loader = self.loader.borrow_mut(); @@ -1396,9 +1398,9 @@ impl Document { // If we don't have a parser, and the reflow timer has been reset, explicitly // trigger a reflow. if let LoadType::Stylesheet(_) = load { - self.window().reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::StylesheetLoaded); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::StylesheetLoaded); } } @@ -1487,6 +1489,8 @@ impl Document { return; } self.domcontentloaded_dispatched.set(true); + assert!(self.ReadyState() != DocumentReadyState::Complete, + "Complete before DOMContentLoaded?"); update_with_current_time_ms(&self.dom_content_loaded_event_start); @@ -1497,14 +1501,17 @@ impl Document { ReflowQueryType::NoQuery, ReflowReason::DOMContentLoaded); + let pipeline_id = self.window.pipeline(); + let event = ConstellationMsg::DOMLoad(pipeline_id); + self.window.constellation_chan().send(event).unwrap(); + update_with_current_time_ms(&self.dom_content_loaded_event_end); } pub fn notify_constellation_load(&self) { let pipeline_id = self.window.pipeline(); - let event = ConstellationMsg::DOMLoad(pipeline_id); - self.window.constellation_chan().send(event).unwrap(); - + let load_event = ConstellationMsg::LoadComplete(pipeline_id); + self.window.constellation_chan().send(load_event).unwrap(); } pub fn set_current_parser(&self, script: Option) { @@ -2913,11 +2920,12 @@ impl DocumentProgressHandler { // http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventEnd update_with_current_time_ms(&document.load_event_end); - document.notify_constellation_load(); window.reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::DocumentLoaded); + + document.notify_constellation_load(); } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index cf0cebf5a65..fbfedae13e9 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -1193,8 +1193,6 @@ impl ScriptThread { // https://html.spec.whatwg.org/multipage/#the-end step 7 let handler = box DocumentProgressHandler::new(Trusted::new(doc)); self.dom_manipulation_task_source.queue(handler, GlobalRef::Window(doc.window())).unwrap(); - - self.constellation_chan.send(ConstellationMsg::LoadComplete(pipeline)).unwrap(); } fn collect_reports(&self, reports_chan: ReportsChan) {