Auto merge of #9832 - metajack:suppress-reflows, r=mbrubeck

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.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9832)
<!-- Reviewable:end -->

9 files changed, 91 insertions, 78 deletions

diff --git a/components/layout/layout_thread.rs b/components/layout/layout_thread.rs
index a361e42fb15..1a8bdfb4a19 100644
--- a/components/layout/layout_thread.rs
+++ b/components/layout/layout_thread.rs
@@ -22,7 +22,7 @@ use euclid::size::Size2D;
 use flow::{self, Flow, ImmutableFlowUtils, MutableFlowUtils, MutableOwnedFlowUtils};
 use flow_ref::{self, FlowRef};
 use fnv::FnvHasher;
-use gfx::display_list::{ClippingRegion, DisplayList, LayerInfo};
+use gfx::display_list::{ClippingRegion, DisplayItemMetadata, DisplayList, LayerInfo};
 use gfx::display_list::{OpaqueNode, StackingContext, StackingContextId, StackingContextType};
 use gfx::font;
 use gfx::font_cache_thread::FontCacheThread;
@@ -114,6 +114,9 @@ pub struct LayoutThreadData {
     /// A queued response for the client {top, left, width, height} of a node in pixels.
     pub client_rect_response: Rect<i32>,
 
+    /// A queued response for the node at a given point
+    pub hit_test_response: (Option<DisplayItemMetadata>, bool),
+
     /// A queued response for the resolved style property of an element.
     pub resolved_style_response: Option<String>,
 
@@ -458,6 +461,7 @@ impl LayoutThread {
                     content_box_response: Rect::zero(),
                     content_boxes_response: Vec::new(),
                     client_rect_response: Rect::zero(),
+                    hit_test_response: (None, false),
                     resolved_style_response: None,
                     offset_parent_response: OffsetParentResponse::empty(),
                     margin_style_response: MarginStyleResponse::empty(),
@@ -977,6 +981,9 @@ impl LayoutThread {
                     ReflowQueryType::ContentBoxesQuery(_) => {
                         rw_data.content_boxes_response = Vec::new();
                     },
+                    ReflowQueryType::HitTestQuery(_, _) => {
+                        rw_data.hit_test_response = (None, false);
+                    },
                     ReflowQueryType::NodeGeometryQuery(_) => {
                         rw_data.client_rect_response = Rect::zero();
                     },
@@ -1119,6 +1126,18 @@ impl LayoutThread {
                     let node = unsafe { ServoLayoutNode::new(&node) };
                     rw_data.content_boxes_response = process_content_boxes_request(node, &mut root_flow);
                 },
+                ReflowQueryType::HitTestQuery(point, update_cursor) => {
+                    let point = Point2D::new(Au::from_f32_px(point.x), Au::from_f32_px(point.y));
+                    let result = match rw_data.display_list {
+                        None => panic!("Tried to hit test with no display list"),
+                        Some(ref dl) => dl.hit_test(point),
+                    };
+                    rw_data.hit_test_response = if result.len() > 0 {
+                        (Some(result[0]), update_cursor)
+                    } else {
+                        (None, update_cursor)
+                    };
+                },
                 ReflowQueryType::NodeGeometryQuery(node) => {
                     let node = unsafe { ServoLayoutNode::new(&node) };
                     rw_data.client_rect_response = process_node_geometry_request(node, &mut root_flow);
diff --git a/components/layout/query.rs b/components/layout/query.rs
index df45301106a..039de56f5c4 100644
--- a/components/layout/query.rs
+++ b/components/layout/query.rs
@@ -51,6 +51,25 @@ impl LayoutRPC for LayoutRPCImpl {
         ContentBoxesResponse(rw_data.content_boxes_response.clone())
     }
 
+    /// Requests the node containing the point of interest.
+    fn hit_test(&self) -> HitTestResponse {
+        let &LayoutRPCImpl(ref rw_data) = self;
+        let rw_data = rw_data.lock().unwrap();
+        let &(ref result, update_cursor) = &rw_data.hit_test_response;
+        if update_cursor {
+            // Compute the new cursor.
+            let cursor = match *result {
+                None => Cursor::DefaultCursor,
+                Some(dim) => dim.pointing.unwrap(),
+            };
+            let ConstellationChan(ref constellation_chan) = rw_data.constellation_chan;
+            constellation_chan.send(ConstellationMsg::SetCursor(cursor)).unwrap();
+        }
+        HitTestResponse {
+            node_address: result.map(|dim| dim.node.to_untrusted_node_address()),
+        }
+    }
+
     fn node_geometry(&self) -> NodeGeometryResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
@@ -66,33 +85,6 @@ impl LayoutRPC for LayoutRPCImpl {
         ResolvedStyleResponse(rw_data.resolved_style_response.clone())
     }
 
-    /// Requests the node containing the point of interest.
-    fn hit_test(&self, point: Point2D<f32>, update_cursor: bool) -> Result<HitTestResponse, ()> {
-        let point = Point2D::new(Au::from_f32_px(point.x), Au::from_f32_px(point.y));
-        let &LayoutRPCImpl(ref rw_data) = self;
-        let rw_data = rw_data.lock().unwrap();
-        let display_list = rw_data.display_list.as_ref().expect("Tried to hit test without a DisplayList!");
-
-        let result = display_list.hit_test(point);
-
-        if update_cursor {
-            let cursor = if !result.is_empty() {
-                result[0].pointing.unwrap()
-            } else {
-                Cursor::DefaultCursor
-            };
-
-            let ConstellationChan(ref constellation_chan) = rw_data.constellation_chan;
-            constellation_chan.send(ConstellationMsg::SetCursor(cursor)).unwrap();
-        };
-
-        if !result.is_empty() {
-            Ok(HitTestResponse(result[0].node.to_untrusted_node_address()))
-        } else {
-            Err(())
-        }
-    }
-
     fn offset_parent(&self) -> OffsetParentResponse {
         let &LayoutRPCImpl(ref rw_data) = self;
         let rw_data = rw_data.lock().unwrap();
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),
diff --git a/tests/wpt/metadata-css/cssom-view-1_dev/html/CaretPosition-001.htm.ini b/tests/wpt/metadata-css/cssom-view-1_dev/html/CaretPosition-001.htm.ini
deleted file mode 100644
index b9ce78c53e8..00000000000
--- a/tests/wpt/metadata-css/cssom-view-1_dev/html/CaretPosition-001.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[CaretPosition-001.htm]
-  type: testharness
-  [Element at (400, 100)]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPoint-001.htm.ini b/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPoint-001.htm.ini
deleted file mode 100644
index 3ca412a4b77..00000000000
--- a/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPoint-001.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[elementFromPoint-001.htm]
-  type: testharness
-  [CSSOM View - 5 - extensions to the Document interface]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPosition.htm.ini b/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPosition.htm.ini
index c91fed538c7..c772fa1f471 100644
--- a/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPosition.htm.ini
+++ b/tests/wpt/metadata-css/cssom-view-1_dev/html/elementFromPosition.htm.ini
@@ -1,20 +1,8 @@
 [elementFromPosition.htm]
   type: testharness
-  [test some point of the element: top left corner]
-    expected: FAIL
-
-  [test some point of the element: top line]
-    expected: FAIL
-
   [test some point of the element: top right corner]
     expected: FAIL
 
-  [test some point of the element: left line]
-    expected: FAIL
-
-  [test some point of the element: inside]
-    expected: FAIL
-
   [test some point of the element: right line]
     expected: FAIL