layout: Keep track of whether we've deferred the painting of the document due to

a script query.

This will, rather unfortunately, mean that we might repaint two times if we've
deferred a paint, then get an out-of-band reflow. Still seemed better than not
suppressing paints at all.

Fixes #13131

6 files changed, 111 insertions, 57 deletions

diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 4cf5a435da7..d3be538e2b3 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -85,7 +85,7 @@ use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheResult, ImageCach
 use profile_traits::mem::{self, Report, ReportKind, ReportsChan};
 use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType};
 use profile_traits::time::{self, TimerMetadata, profile};
-use script::layout_wrapper::ServoLayoutNode;
+use script::layout_wrapper::{ServoLayoutDocument, ServoLayoutNode};
 use script_layout_interface::message::{Msg, NewLayoutThreadInfo, Reflow, ReflowQueryType, ScriptReflow};
 use script_layout_interface::reporter::CSSErrorReporter;
 use script_layout_interface::restyle_damage::{REPAINT, STORE_OVERFLOW, REFLOW_OUT_OF_FLOW, REFLOW};
@@ -629,6 +629,7 @@ impl LayoutThread {
                                                                   reflow_info.goal);
 
         self.perform_post_style_recalc_layout_passes(&reflow_info,
+                                                     None,
                                                      &mut *rw_data,
                                                      &mut layout_context);
 
@@ -901,6 +902,7 @@ impl LayoutThread {
 
     fn compute_abs_pos_and_build_display_list(&mut self,
                                               data: &Reflow,
+                                              document: Option<&ServoLayoutDocument>,
                                               layout_root: &mut Flow,
                                               shared_layout_context: &mut SharedLayoutContext,
                                               rw_data: &mut LayoutThreadData) {
@@ -962,59 +964,69 @@ impl LayoutThread {
                     Some(Arc::new(DisplayList::new(root_stacking_context, display_list_entries)))
             }
 
-            if data.goal == ReflowGoal::ForDisplay {
-                let display_list = (*rw_data.display_list.as_ref().unwrap()).clone();
+            if data.goal != ReflowGoal::ForDisplay {
+                // Defer the paint step until the next ForDisplay.
+                //
+                // We need to tell the document about this so it doesn't
+                // incorrectly suppress reflows. See #13131.
+                document.expect("No document in a non-display reflow?")
+                        .needs_paint_from_layout();
+                return;
+            }
+            if let Some(document) = document {
+                document.will_paint();
+            }
+            let display_list = (*rw_data.display_list.as_ref().unwrap()).clone();
 
-                if opts::get().dump_display_list {
-                    display_list.print();
-                }
-                if opts::get().dump_display_list_json {
-                    println!("{}", serde_json::to_string_pretty(&display_list).unwrap());
-                }
+            if opts::get().dump_display_list {
+                display_list.print();
+            }
+            if opts::get().dump_display_list_json {
+                println!("{}", serde_json::to_string_pretty(&display_list).unwrap());
+            }
 
-                debug!("Layout done!");
-
-                self.epoch.next();
-
-                if let Some(ref mut webrender_api) = self.webrender_api {
-                    // TODO: Avoid the temporary conversion and build webrender sc/dl directly!
-                    let Epoch(epoch_number) = self.epoch;
-                    let epoch = webrender_traits::Epoch(epoch_number);
-                    let pipeline_id = self.id.to_webrender();
-
-                    // TODO(gw) For now only create a root scrolling layer!
-                    let mut frame_builder = WebRenderFrameBuilder::new(pipeline_id);
-                    let root_scroll_layer_id = frame_builder.next_scroll_layer_id();
-                    let sc_id = rw_data.display_list.as_ref().unwrap().convert_to_webrender(
-                        webrender_api,
-                        pipeline_id,
-                        epoch,
-                        Some(root_scroll_layer_id),
-                        &mut frame_builder);
-                    let root_background_color = get_root_flow_background_color(layout_root);
-                    let root_background_color =
-                        webrender_traits::ColorF::new(root_background_color.r,
-                                                      root_background_color.g,
-                                                      root_background_color.b,
-                                                      root_background_color.a);
-
-                    let viewport_size = Size2D::new(self.viewport_size.width.to_f32_px(),
-                                                    self.viewport_size.height.to_f32_px());
-
-                    webrender_api.set_root_stacking_context(
-                        sc_id,
-                        root_background_color,
-                        epoch,
-                        pipeline_id,
-                        viewport_size,
-                        frame_builder.stacking_contexts,
-                        frame_builder.display_lists,
-                        frame_builder.auxiliary_lists_builder.finalize());
-                } else {
-                    self.paint_chan
-                        .send(LayoutToPaintMsg::PaintInit(self.epoch, display_list))
-                        .unwrap();
-                }
+            debug!("Layout done!");
+
+            self.epoch.next();
+
+            if let Some(ref mut webrender_api) = self.webrender_api {
+                // TODO: Avoid the temporary conversion and build webrender sc/dl directly!
+                let Epoch(epoch_number) = self.epoch;
+                let epoch = webrender_traits::Epoch(epoch_number);
+                let pipeline_id = self.id.to_webrender();
+
+                // TODO(gw) For now only create a root scrolling layer!
+                let mut frame_builder = WebRenderFrameBuilder::new(pipeline_id);
+                let root_scroll_layer_id = frame_builder.next_scroll_layer_id();
+                let sc_id = rw_data.display_list.as_ref().unwrap().convert_to_webrender(
+                    webrender_api,
+                    pipeline_id,
+                    epoch,
+                    Some(root_scroll_layer_id),
+                    &mut frame_builder);
+                let root_background_color = get_root_flow_background_color(layout_root);
+                let root_background_color =
+                    webrender_traits::ColorF::new(root_background_color.r,
+                                                  root_background_color.g,
+                                                  root_background_color.b,
+                                                  root_background_color.a);
+
+                let viewport_size = Size2D::new(self.viewport_size.width.to_f32_px(),
+                                                self.viewport_size.height.to_f32_px());
+
+                webrender_api.set_root_stacking_context(
+                    sc_id,
+                    root_background_color,
+                    epoch,
+                    pipeline_id,
+                    viewport_size,
+                    frame_builder.stacking_contexts,
+                    frame_builder.display_lists,
+                    frame_builder.auxiliary_lists_builder.finalize());
+            } else {
+                self.paint_chan
+                    .send(LayoutToPaintMsg::PaintInit(self.epoch, display_list))
+                    .unwrap();
             }
         });
     }
@@ -1205,6 +1217,7 @@ impl LayoutThread {
 
         // Perform post-style recalculation layout passes.
         self.perform_post_style_recalc_layout_passes(&data.reflow_info,
+                                                     Some(&document),
                                                      &mut rw_data,
                                                      &mut shared_layout_context);
 
@@ -1327,7 +1340,7 @@ impl LayoutThread {
                                                                   false,
                                                                   reflow_info.goal);
 
-        self.perform_post_main_layout_passes(&reflow_info, &mut *rw_data, &mut layout_context);
+        self.perform_post_main_layout_passes(&reflow_info, None, &mut *rw_data, &mut layout_context);
         true
     }
 
@@ -1385,6 +1398,7 @@ impl LayoutThread {
         }
 
         self.perform_post_style_recalc_layout_passes(&reflow_info,
+                                                     None,
                                                      &mut *rw_data,
                                                      &mut layout_context);
     }
@@ -1407,12 +1421,14 @@ impl LayoutThread {
             return
         }
         self.perform_post_style_recalc_layout_passes(&reflow_info,
+                                                     None,
                                                      &mut *rw_data,
                                                      &mut layout_context);
     }
 
     fn perform_post_style_recalc_layout_passes(&mut self,
                                                data: &Reflow,
+                                               document: Option<&ServoLayoutDocument>,
                                                rw_data: &mut LayoutThreadData,
                                                layout_context: &mut SharedLayoutContext) {
         if let Some(mut root_flow) = self.root_flow.clone() {
@@ -1487,17 +1503,19 @@ impl LayoutThread {
                                            flow_ref::deref_mut(&mut root_flow) as &mut Flow);
             });
 
-            self.perform_post_main_layout_passes(data, rw_data, layout_context);
+            self.perform_post_main_layout_passes(data, document, rw_data, layout_context);
         }
     }
 
     fn perform_post_main_layout_passes(&mut self,
                                        data: &Reflow,
+                                       document: Option<&ServoLayoutDocument>,
                                        rw_data: &mut LayoutThreadData,
                                        layout_context: &mut SharedLayoutContext) {
         // Build the display list if necessary, and send it to the painter.
         if let Some(mut root_flow) = self.root_flow.clone() {
             self.compute_abs_pos_and_build_display_list(data,
+                                                        document,
                                                         flow_ref::deref_mut(&mut root_flow),
                                                         &mut *layout_context,
                                                         rw_data);
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index cbd4b2ac253..58995645c27 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -221,6 +221,9 @@ pub struct Document {
     /// For each element that has had a state or attribute change since the last restyle,
     /// track the original condition of the element.
     modified_elements: DOMRefCell<HashMap<JS<Element>, ElementSnapshot>>,
+    /// This flag will be true if layout suppressed a reflow attempt that was
+    /// needed in order for the page to be painted.
+    needs_paint: Cell<bool>,
     /// http://w3c.github.io/touch-events/#dfn-active-touch-point
     active_touch_points: DOMRefCell<Vec<JS<Touch>>>,
     /// Navigation Timing properties:
@@ -376,6 +379,10 @@ impl Document {
         }
     }
 
+    pub fn needs_paint(&self) -> bool {
+        self.needs_paint.get()
+    }
+
     pub fn needs_reflow(&self) -> bool {
         // FIXME: This should check the dirty bit on the document,
         // not the document element. Needs some layout changes to make
@@ -384,7 +391,8 @@ impl Document {
             Some(root) => {
                 root.upcast::<Node>().is_dirty() ||
                 root.upcast::<Node>().has_dirty_descendants() ||
-                !self.modified_elements.borrow().is_empty()
+                !self.modified_elements.borrow().is_empty() ||
+                self.needs_paint()
             }
             None => false,
         }
@@ -1602,6 +1610,8 @@ pub enum DocumentSource {
 pub trait LayoutDocumentHelpers {
     unsafe fn is_html_document_for_layout(&self) -> bool;
     unsafe fn drain_modified_elements(&self) -> Vec<(LayoutJS<Element>, ElementSnapshot)>;
+    unsafe fn needs_paint_from_layout(&self);
+    unsafe fn will_paint(&self);
 }
 
 #[allow(unsafe_code)]
@@ -1618,6 +1628,16 @@ impl LayoutDocumentHelpers for LayoutJS<Document> {
         let result = elements.drain().map(|(k, v)| (k.to_layout(), v)).collect();
         result
     }
+
+    #[inline]
+    unsafe fn needs_paint_from_layout(&self) {
+        (*self.unsafe_get()).needs_paint.set(true)
+    }
+
+    #[inline]
+    unsafe fn will_paint(&self) {
+        (*self.unsafe_get()).needs_paint.set(false)
+    }
 }
 
 /// https://url.spec.whatwg.org/#network-scheme
@@ -1723,6 +1743,7 @@ impl Document {
             base_element: Default::default(),
             appropriate_template_contents_owner_document: Default::default(),
             modified_elements: DOMRefCell::new(HashMap::new()),
+            needs_paint: Cell::new(false),
             active_touch_points: DOMRefCell::new(Vec::new()),
             dom_loading: Cell::new(Default::default()),
             dom_interactive: Cell::new(Default::default()),
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index cd00390a305..b6bf6a35643 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -1200,9 +1200,11 @@ impl Window {
         if !for_display || self.Document().needs_reflow() {
             issued_reflow = self.force_reflow(goal, query_type, reason);
 
-            // 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.
+            // 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, except if we're waiting for a deferred paint.
             assert!(!self.Document().needs_reflow() ||
+                    (!for_display && self.Document().needs_paint()) ||
                     self.window_size.get().is_none() ||
                     self.suppress_reflow.get());
         } else {
diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs
index 906bde60550..4edd821ad0b 100644
--- a/components/script/layout_wrapper.rs
+++ b/components/script/layout_wrapper.rs
@@ -405,6 +405,14 @@ impl<'ld> TDocument for ServoLayoutDocument<'ld> {
         let elements =  unsafe { self.document.drain_modified_elements() };
         elements.into_iter().map(|(el, snapshot)| (ServoLayoutElement::from_layout_js(el), snapshot)).collect()
     }
+
+    fn needs_paint_from_layout(&self) {
+        unsafe { self.document.needs_paint_from_layout(); }
+    }
+
+    fn will_paint(&self) {
+        unsafe { self.document.will_paint(); }
+    }
 }
 
 impl<'ld> ServoLayoutDocument<'ld> {
diff --git a/components/style/dom.rs b/components/style/dom.rs
index ce7ba70a1bf..f503917e866 100644
--- a/components/style/dom.rs
+++ b/components/style/dom.rs
@@ -179,6 +179,9 @@ pub trait TDocument : Sized + Copy + Clone {
 
     fn drain_modified_elements(&self) -> Vec<(Self::ConcreteElement,
                                               <Self::ConcreteElement as ElementExt>::Snapshot)>;
+
+    fn needs_paint_from_layout(&self);
+    fn will_paint(&self);
 }
 
 pub trait PresentationalHintsSynthetizer {
diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs
index c168b1dcaaa..6a356b2e3c4 100644
--- a/ports/geckolib/wrapper.rs
+++ b/ports/geckolib/wrapper.rs
@@ -414,6 +414,8 @@ impl<'ld> TDocument for GeckoDocument<'ld> {
         let elements =  unsafe { self.document.drain_modified_elements() };
         elements.into_iter().map(|(el, snapshot)| (ServoLayoutElement::from_layout_js(el), snapshot)).collect()*/
     }
+    fn will_paint(&self) { unimplemented!() }
+    fn needs_paint_from_layout(&self) { unimplemented!() }
 }
 
 #[derive(Clone, Copy)]