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.

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(),
@@ -978,6 +982,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();
                     },
@@ -1120,6 +1127,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);

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();

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();

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",

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};

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),

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