Suppress reflows before RefreshTick or FirstLoad

This fixes a bug where partially loaded content is displayed to the user
before it should be, usually before stylesheets have loaded. This commit
supresses reflows until either FirstLoad or RefreshTick, whichever comes
first.

Unfortunately, hit_test and mouse_over did not do reflows if they were
necessary, and so by suppressing the initial spurious reflows, these
methods started to panic without a display list to query. This patch
also transforms these into queries similar to the other existing
queries.

4 files changed, 52 insertions, 28 deletions

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index ec0e78fafc2..7d0a3be8b14 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -80,7 +80,6 @@ use html5ever::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
 use ipc_channel::ipc::{self, IpcSender};
 use js::jsapi::JS_GetRuntime;
 use js::jsapi::{JSContext, JSObject, JSRuntime};
-use layout_interface::HitTestResponse;
 use layout_interface::{LayoutChan, Msg, ReflowQueryType};
 use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
 use msg::constellation_msg::{ConstellationChan, Key, KeyModifiers, KeyState};
@@ -93,7 +92,7 @@ use num::ToPrimitive;
 use script_thread::{MainThreadScriptMsg, Runnable};
 use script_traits::{AnimationState, MouseButton, MouseEventType, MozBrowserEvent};
 use script_traits::{ScriptMsg as ConstellationMsg, ScriptToCompositorMsg};
-use script_traits::{TouchEventType, TouchId, UntrustedNodeAddress};
+use script_traits::{TouchEventType, TouchId};
 use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::boxed::FnBox;
@@ -563,17 +562,6 @@ impl Document {
                 .map(Root::upcast)
     }
 
-    pub fn hit_test(&self, page_point: &Point2D<f32>, update_cursor: bool) -> Option<UntrustedNodeAddress> {
-        assert!(self.GetDocumentElement().is_some());
-        match self.window.layout().hit_test(*page_point, update_cursor) {
-            Ok(HitTestResponse(node_address)) => Some(node_address),
-            Err(()) => {
-                debug!("layout query error");
-                None
-            }
-        }
-    }
-
     // https://html.spec.whatwg.org/multipage/#current-document-readiness
     pub fn set_ready_state(&self, state: DocumentReadyState) {
         match state {
@@ -685,7 +673,7 @@ impl Document {
 
         let page_point = Point2D::new(client_point.x + self.window.PageXOffset() as f32,
                                       client_point.y + self.window.PageYOffset() as f32);
-        let node = match self.hit_test(&page_point, false) {
+        let node = match self.window.hit_test_query(page_point, false) {
             Some(node_address) => {
                 debug!("node address is {:?}", node_address);
                 node::from_untrusted_node_address(js_runtime, node_address)
@@ -814,7 +802,7 @@ impl Document {
 
         let client_point = client_point.unwrap();
 
-        let maybe_new_target = self.hit_test(&page_point, true).and_then(|address| {
+        let maybe_new_target = self.window.hit_test_query(page_point, true).and_then(|address| {
             let node = node::from_untrusted_node_address(js_runtime, address);
             node.inclusive_ancestors()
                 .filter_map(Root::downcast::<Element>)
@@ -908,7 +896,7 @@ impl Document {
             TouchEventType::Cancel => "touchcancel",
         };
 
-        let node = match self.hit_test(&point, false) {
+        let node = match self.window.hit_test_query(point, false) {
             Some(node_address) => node::from_untrusted_node_address(js_runtime, node_address),
             None => return false,
         };
@@ -2575,7 +2563,7 @@ impl DocumentMethods for Document {
 
         let js_runtime = unsafe { JS_GetRuntime(window.get_cx()) };
 
-        match self.hit_test(point, false)  {
+        match self.window.hit_test_query(*point, false) {
             Some(untrusted_node_address) => {
                 let node = node::from_untrusted_node_address(js_runtime, untrusted_node_address);
                 let parent_node = node.GetParentNode().unwrap();
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 0869423c55b..ed50d073175 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -56,7 +56,7 @@ use rustc_serialize::base64::{FromBase64, STANDARD, ToBase64};
 use script_thread::{DOMManipulationTaskSource, UserInteractionTaskSource, NetworkingTaskSource};
 use script_thread::{HistoryTraversalTaskSource, FileReadingTaskSource, SendableMainThreadScriptChan};
 use script_thread::{ScriptChan, ScriptPort, MainThreadScriptChan, MainThreadScriptMsg, RunnableWrapper};
-use script_traits::ConstellationControlMsg;
+use script_traits::{ConstellationControlMsg, UntrustedNodeAddress};
 use script_traits::{DocumentState, MsDuration, ScriptToCompositorMsg, TimerEvent, TimerEventId};
 use script_traits::{MozBrowserEvent, ScriptMsg as ConstellationMsg, TimerEventRequest, TimerSource};
 use std::ascii::AsciiExt;
@@ -219,6 +219,11 @@ pub struct Window {
     /// to prevent creating display list items for content that is far away from the viewport.
     page_clip_rect: Cell<Rect<Au>>,
 
+    /// Flag to suppress reflows. The first reflow will come either with
+    /// RefreshTick or with FirstLoad. Until those first reflows, we want to
+    /// suppress others like MissingExplicitReflow.
+    suppress_reflow: Cell<bool>,
+
     /// A counter of the number of pending reflows for this window.
     pending_reflow_count: Cell<u32>,
 
@@ -933,17 +938,34 @@ impl Window {
         recv.recv().unwrap_or((Size2D::zero(), Point2D::zero()))
     }
 
-    /// Reflows the page unconditionally. This method will wait for the layout thread to complete
-    /// (but see the `TODO` below). If there is no window size yet, the page is presumed invisible
-    /// and no reflow is performed.
+    /// Reflows the page unconditionally if possible and not suppressed. This
+    /// method will wait for the layout thread to complete (but see the `TODO`
+    /// below). If there is no window size yet, the page is presumed invisible
+    /// and no reflow is performed. If reflow is suppressed, no reflow will be
+    /// performed for ForDisplay goals.
     ///
     /// TODO(pcwalton): Only wait for style recalc, since we have off-main-thread layout.
     pub fn force_reflow(&self, goal: ReflowGoal, query_type: ReflowQueryType, reason: ReflowReason) {
+        // Check if we need to unsuppress reflow. Note that this needs to be
+        // *before* any early bailouts, or reflow might never be unsuppresed!
+        match reason {
+            ReflowReason::FirstLoad | ReflowReason::RefreshTick => self.suppress_reflow.set(false),
+            _ => (),
+        }
+
+        // If there is no window size, we have nothing to do.
         let window_size = match self.window_size.get() {
             Some(window_size) => window_size,
             None => return,
         };
 
+        let for_display = query_type == ReflowQueryType::NoQuery;
+        if for_display && self.suppress_reflow.get() {
+            debug!("Suppressing reflow pipeline {} for goal {:?} reason {:?} before FirstLoad or RefreshTick",
+                   self.id, goal, reason);
+            return;
+        }
+
         debug!("script: performing reflow for goal {:?} reason {:?}", goal, reason);
 
         let marker = if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) {
@@ -957,7 +979,7 @@ impl Window {
 
         // On debug mode, print the reflow event information.
         if opts::get().relayout_event {
-            debug_reflow_events(&goal, &query_type, &reason);
+            debug_reflow_events(self.id, &goal, &query_type, &reason);
         }
 
         let document = self.Document();
@@ -1015,7 +1037,9 @@ impl Window {
 
             // If window_size is `None`, we don't reflow, so the document stays dirty.
             // Otherwise, we shouldn't need a reflow immediately after a reflow.
-            assert!(!self.Document().needs_reflow() || self.window_size.get().is_none());
+            assert!(!self.Document().needs_reflow() ||
+                    self.window_size.get().is_none() ||
+                    self.suppress_reflow.get());
         } else {
             debug!("Document doesn't need reflow - skipping it (reason {:?})", reason);
         }
@@ -1074,6 +1098,14 @@ impl Window {
         self.layout_rpc.node_geometry().client_rect
     }
 
+    pub fn hit_test_query(&self, hit_test_request: Point2D<f32>, update_cursor: bool)
+                          -> Option<UntrustedNodeAddress> {
+        self.reflow(ReflowGoal::ForDisplay,
+                    ReflowQueryType::HitTestQuery(hit_test_request, update_cursor),
+                    ReflowReason::Query);
+        self.layout_rpc.hit_test().node_address
+    }
+
     pub fn resolved_style_query(&self,
                             element: TrustedNodeAddress,
                             pseudo: Option<PseudoElement>,
@@ -1377,6 +1409,7 @@ impl Window {
             layout_rpc: layout_rpc,
             window_size: Cell::new(window_size),
             current_viewport: Cell::new(Rect::zero()),
+            suppress_reflow: Cell::new(true),
             pending_reflow_count: Cell::new(0),
             current_state: Cell::new(WindowState::Alive),
 
@@ -1413,8 +1446,8 @@ fn should_move_clip_rect(clip_rect: Rect<Au>, new_viewport: Rect<f32>) -> bool {
     (clip_rect.max_y() - new_viewport.max_y()).abs() <= viewport_scroll_margin.height
 }
 
-fn debug_reflow_events(goal: &ReflowGoal, query_type: &ReflowQueryType, reason: &ReflowReason) {
-    let mut debug_msg = "****".to_owned();
+fn debug_reflow_events(id: PipelineId, goal: &ReflowGoal, query_type: &ReflowQueryType, reason: &ReflowReason) {
+    let mut debug_msg = format!("**** pipeline={}", id);
     debug_msg.push_str(match *goal {
         ReflowGoal::ForDisplay => "\tForDisplay",
         ReflowGoal::ForScriptQuery => "\tForScriptQuery",
@@ -1424,6 +1457,7 @@ fn debug_reflow_events(goal: &ReflowGoal, query_type: &ReflowQueryType, reason:
         ReflowQueryType::NoQuery => "\tNoQuery",
         ReflowQueryType::ContentBoxQuery(_n) => "\tContentBoxQuery",
         ReflowQueryType::ContentBoxesQuery(_n) => "\tContentBoxesQuery",
+        ReflowQueryType::HitTestQuery(_n, _o) => "\tHitTestQuery",
         ReflowQueryType::NodeGeometryQuery(_n) => "\tNodeGeometryQuery",
         ReflowQueryType::ResolvedStyleQuery(_, _, _) => "\tResolvedStyleQuery",
         ReflowQueryType::OffsetParentQuery(_n) => "\tOffsetParentQuery",
diff --git a/components/script/dom/xmldocument.rs b/components/script/dom/xmldocument.rs
index e8cf284c7d1..6fc1ebfd267 100644
--- a/components/script/dom/xmldocument.rs
+++ b/components/script/dom/xmldocument.rs
@@ -6,7 +6,6 @@ use document_loader::DocumentLoader;
 use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::codegen::Bindings::XMLDocumentBinding::{self, XMLDocumentMethods};
-use dom::bindings::error::Fallible;
 use dom::bindings::global::GlobalRef;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{Root, RootedReference};
diff --git a/components/script/layout_interface.rs b/components/script/layout_interface.rs
index 3976f0a8d20..9a8cddf97e1 100644
--- a/components/script/layout_interface.rs
+++ b/components/script/layout_interface.rs
@@ -105,7 +105,7 @@ pub trait LayoutRPC {
     /// Requests the geometry of this node. Used by APIs such as `clientTop`.
     fn node_geometry(&self) -> NodeGeometryResponse;
     /// Requests the node containing the point of interest
-    fn hit_test(&self, point: Point2D<f32>, update_cursor: bool) -> Result<HitTestResponse, ()>;
+    fn hit_test(&self) -> HitTestResponse;
     /// Query layout for the resolved value of a given CSS property
     fn resolved_style(&self) -> ResolvedStyleResponse;
     fn offset_parent(&self) -> OffsetParentResponse;
@@ -134,10 +134,12 @@ impl MarginStyleResponse {
 
 pub struct ContentBoxResponse(pub Rect<Au>);
 pub struct ContentBoxesResponse(pub Vec<Rect<Au>>);
+pub struct HitTestResponse {
+    pub node_address: Option<UntrustedNodeAddress>,
+}
 pub struct NodeGeometryResponse {
     pub client_rect: Rect<i32>,
 }
-pub struct HitTestResponse(pub UntrustedNodeAddress);
 pub struct ResolvedStyleResponse(pub Option<String>);
 
 #[derive(Clone)]
@@ -161,6 +163,7 @@ pub enum ReflowQueryType {
     NoQuery,
     ContentBoxQuery(TrustedNodeAddress),
     ContentBoxesQuery(TrustedNodeAddress),
+    HitTestQuery(Point2D<f32>, bool),
     NodeGeometryQuery(TrustedNodeAddress),
     ResolvedStyleQuery(TrustedNodeAddress, Option<PseudoElement>, Atom),
     OffsetParentQuery(TrustedNodeAddress),