script: Remove 'pending reflow' concept and some explicit reflows (#34558)

The `pending reflow` concept isn't necessary now that *update the
rendering* is taking care of triggering reflows at the correct time.
`Window::reflow` already avoids reflows if the page is not dirty, so
pending reflows is now just an extraneous check as long as *update the
rendering* runs properly.

This change also removes some explicit reflows, which now wait until the
appropriate moment during *update the rendering*. This should remove
some extra reflows that are not necessary.

Servo needs some way to track that resizing the web view needs to
re-layout due to the initial containing block changing. Move handling
of `Document::needs_paint` to the script thread and use this, expanding
the rustdoc to explain what it is for a bit more clearly.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/layout_thread/lib.rs

@@ -59,7 +59,7 @@ use profile_traits::time::{
     self as profile_time, TimerMetadata, TimerMetadataFrameType, TimerMetadataReflowType,
 };
 use profile_traits::{path, time_profile};
-use script::layout_dom::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode};
+use script::layout_dom::{ServoLayoutElement, ServoLayoutNode};
 use script_layout_interface::wrapper_traits::LayoutNode;
 use script_layout_interface::{
     Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse, Reflow,
@@ -844,7 +844,6 @@ impl LayoutThread {
         &self,
         data: &Reflow,
         reflow_goal: &ReflowGoal,
-        document: Option<&ServoLayoutDocument>,
         layout_root: &mut dyn Flow,
         layout_context: &mut LayoutContext,
     ) {
@@ -901,18 +900,8 @@ impl LayoutThread {
                 }
 
                 if !reflow_goal.needs_display() {
-                    // 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 mut display_list = self.display_list.borrow_mut();
                 let display_list = display_list.as_mut().unwrap();
@@ -1176,7 +1165,6 @@ impl LayoutThread {
                 &mut root_flow,
                 &data.reflow_info,
                 &data.reflow_goal,
-                Some(&document),
                 &mut layout_context,
                 thread_pool,
             );
@@ -1251,7 +1239,6 @@ impl LayoutThread {
         root_flow: &mut FlowRef,
         data: &Reflow,
         reflow_goal: &ReflowGoal,
-        document: Option<&ServoLayoutDocument>,
         context: &mut LayoutContext,
         thread_pool: Option<&rayon::ThreadPool>,
     ) {
@@ -1337,7 +1324,7 @@ impl LayoutThread {
             },
         );
 
-        self.perform_post_main_layout_passes(data, root_flow, reflow_goal, document, context);
+        self.perform_post_main_layout_passes(data, root_flow, reflow_goal, context);
     }
 
     fn perform_post_main_layout_passes(
@@ -1345,14 +1332,12 @@ impl LayoutThread {
         data: &Reflow,
         root_flow: &mut FlowRef,
         reflow_goal: &ReflowGoal,
-        document: Option<&ServoLayoutDocument>,
         layout_context: &mut LayoutContext,
     ) {
         // Build the display list if necessary, and send it to the painter.
         self.compute_abs_pos_and_build_display_list(
             data,
             reflow_goal,
-            document,
             FlowRef::deref_mut(root_flow),
             &mut *layout_context,
         );

components/layout_thread_2020/lib.rs

@@ -47,7 +47,7 @@ use profile_traits::time::{
     self as profile_time, TimerMetadata, TimerMetadataFrameType, TimerMetadataReflowType,
 };
 use profile_traits::{path, time_profile};
-use script::layout_dom::{ServoLayoutDocument, ServoLayoutElement, ServoLayoutNode};
+use script::layout_dom::{ServoLayoutElement, ServoLayoutNode};
 use script_layout_interface::{
     Layout, LayoutConfig, LayoutFactory, NodesFromPointQueryType, OffsetParentResponse,
     ReflowComplete, ReflowGoal, ScriptReflow, TrustedNodeAddress,
@@ -899,7 +899,6 @@ impl LayoutThread {
             self.perform_post_style_recalc_layout_passes(
                 root.clone(),
                 &data.reflow_goal,
-                Some(&document),
                 &mut layout_context,
             );
         }
@@ -929,7 +928,6 @@ impl LayoutThread {
         &self,
         fragment_tree: Arc<FragmentTree>,
         reflow_goal: &ReflowGoal,
-        document: Option<&ServoLayoutDocument>,
         context: &mut LayoutContext,
     ) {
         Self::cancel_animations_for_nodes_not_in_fragment_tree(
@@ -944,20 +942,9 @@ impl LayoutThread {
         }
 
         if !reflow_goal.needs_display_list() {
-            // 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 mut epoch = self.epoch.get();
         epoch.next();
         self.epoch.set(epoch);

components/script/dom/document.rs

@@ -345,8 +345,10 @@ pub struct Document {
     /// Information on elements needing restyle to ship over to layout when the
     /// time comes.
     pending_restyles: DomRefCell<HashMap<Dom<Element>, NoTrace<PendingRestyle>>>,
-    /// This flag will be true if layout suppressed a reflow attempt that was
-    /// needed in order for the page to be painted.
+    /// This flag will be true if the `Document` needs to be painted again
+    /// during the next full layout attempt due to some external change such as
+    /// the web view changing size, or because the previous layout was only for
+    /// layout queries (which do not trigger display).
     needs_paint: Cell<bool>,
     /// <http://w3c.github.io/touch-events/#dfn-active-touch-point>
     active_touch_points: DomRefCell<Vec<Dom<Touch>>>,
@@ -823,8 +825,8 @@ impl Document {
         }
     }
 
-    pub fn needs_paint(&self) -> bool {
-        self.needs_paint.get()
+    pub(crate) fn set_needs_paint(&self, value: bool) {
+        self.needs_paint.set(value)
     }
 
     pub fn needs_reflow(&self) -> Option<ReflowTriggerCondition> {
@@ -844,7 +846,7 @@ impl Document {
             return Some(ReflowTriggerCondition::PendingRestyles);
         }
 
-        if self.needs_paint() {
+        if self.needs_paint.get() {
             return Some(ReflowTriggerCondition::PaintPostponed);
         }
 
@@ -3169,8 +3171,6 @@ pub enum DocumentSource {
 #[allow(unsafe_code)]
 pub trait LayoutDocumentHelpers<'dom> {
     fn is_html_document_for_layout(&self) -> bool;
-    fn needs_paint_from_layout(self);
-    fn will_paint(self);
     fn quirks_mode(self) -> QuirksMode;
     fn style_shared_lock(self) -> &'dom StyleSharedRwLock;
     fn shadow_roots(self) -> Vec<LayoutDom<'dom, ShadowRoot>>;
@@ -3185,16 +3185,6 @@ impl<'dom> LayoutDocumentHelpers<'dom> for LayoutDom<'dom, Document> {
         self.unsafe_get().is_html_document
     }
 
-    #[inline]
-    fn needs_paint_from_layout(self) {
-        (self.unsafe_get()).needs_paint.set(true)
-    }
-
-    #[inline]
-    fn will_paint(self) {
-        (self.unsafe_get()).needs_paint.set(false)
-    }
-
     #[inline]
     fn quirks_mode(self) -> QuirksMode {
         self.unsafe_get().quirks_mode.get()
@@ -4171,14 +4161,9 @@ impl Document {
 
     pub(crate) fn maybe_mark_animating_nodes_as_dirty(&self) {
         let current_timeline_value = self.current_animation_timeline_value();
-        let marked_dirty = self
-            .animations
+        self.animations
             .borrow()
             .mark_animating_nodes_as_dirty(current_timeline_value);
-
-        if marked_dirty {
-            self.window().add_pending_reflow();
-        }
     }
 
     pub(crate) fn current_animation_timeline_value(&self) -> f64 {

components/script/dom/htmliframeelement.rs

@@ -442,8 +442,6 @@ impl HTMLIFrameElement {
         }
 
         self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
-        let window = window_from_node(self);
-        window.add_pending_reflow();
     }
 
     fn new_inherited(

components/script/dom/htmlimageelement.rs

@@ -503,9 +503,7 @@ impl HTMLImageElement {
                 .fire_event(atom!("loadend"), can_gc);
         }
 
-        // Trigger reflow
-        let window = window_from_node(self);
-        window.add_pending_reflow();
+        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
     }
 
     fn process_image_response_for_environment_change(

components/script/dom/window.rs

@@ -173,14 +173,12 @@ pub enum ReflowReason {
     CachedPageNeededReflow,
     ElementStateChanged,
     FirstLoad,
-    MissingExplicitReflow,
-    PendingReflow,
     Query,
     RefreshTick,
     RequestAnimationFrame,
     ScrollFromScript,
+    UpdateTheRendering,
     Viewport,
-    WindowResize,
     WorkletLoaded,
 }
 
@@ -257,9 +255,6 @@ pub struct Window {
     /// suppress others like MissingExplicitReflow.
     suppress_reflow: Cell<bool>,
 
-    /// A counter of the number of pending reflows for this window.
-    pending_reflow_count: Cell<u32>,
-
     /// A channel for communicating results of async scripts back to the webdriver server
     #[ignore_malloc_size_of = "channels are hard"]
     #[no_trace]
@@ -529,7 +524,6 @@ impl Window {
                 nodes.remove();
             },
         }
-        self.add_pending_reflow();
     }
 
     pub fn compositor_api(&self) -> &CrossProcessCompositorApi {
@@ -1831,9 +1825,12 @@ impl Window {
             _ => (),
         }
 
-        let for_display = reflow_goal == ReflowGoal::Full;
+        let for_display = reflow_goal.needs_display();
         let pipeline_id = self.upcast::<GlobalScope>().pipeline_id();
-        if for_display && self.suppress_reflow.get() {
+
+        // If this was just a normal reflow (not triggered by script which forces reflow),
+        // and the document is still suppressing reflows, exit early.
+        if reflow_goal == ReflowGoal::Full && self.suppress_reflow.get() {
             debug!(
                 "Suppressing reflow pipeline {} for reason {:?} before FirstLoad or RefreshTick",
                 pipeline_id, reason
@@ -1891,7 +1888,6 @@ impl Window {
             .map(|root| root.upcast::<Node>().to_trusted_node_address());
 
         // Send new document and relevant styles to layout.
-        let needs_display = reflow_goal.needs_display();
         let reflow = ScriptReflow {
             reflow_info: Reflow {
                 page_clip_rect: self.page_clip_rect.get(),
@@ -1924,16 +1920,16 @@ impl Window {
 
         debug!("script: layout complete");
 
-        // Pending reflows require display, so only reset the pending reflow count if this reflow
-        // was to be displayed.
-        if needs_display {
-            self.pending_reflow_count.set(0);
-        }
-
         if let Some(marker) = marker {
             self.emit_timeline_marker(marker.end());
         }
 
+        // Either this reflow caused new contents to be displayed or on the next
+        // full layout attempt a reflow should be forced in order to update the
+        // visual contents of the page. A case where full display might be delayed
+        // is when reflowing just for the purpose of doing a layout query.
+        document.set_needs_paint(!for_display);
+
         for image in complete.pending_images {
             let id = image.id;
             let node = unsafe { from_untrusted_node_address(image.node) };
@@ -2365,15 +2361,6 @@ impl Window {
         self.dom_static.windowproxy_handler
     }
 
-    pub fn get_pending_reflow_count(&self) -> u32 {
-        self.pending_reflow_count.get()
-    }
-
-    pub fn add_pending_reflow(&self) {
-        self.pending_reflow_count
-            .set(self.pending_reflow_count.get() + 1);
-    }
-
     pub fn add_resize_event(&self, event: WindowSizeData, event_type: WindowSizeType) {
         // Whenever we receive a new resize event we forget about all the ones that came before
         // it, to avoid unnecessary relayouts
@@ -2407,6 +2394,10 @@ impl Window {
 
         self.page_clip_rect.set(proposed_clip_rect);
 
+        // The document needs to be repainted, because the initial containing block
+        // is now a different size.
+        self.Document().set_needs_paint(true);
+
         // If we didn't have a clip rect, the previous display doesn't need rebuilding
         // because it was built for infinite clip (MaxRect::amax_rect()).
         had_clip_rect
@@ -2504,9 +2495,6 @@ impl Window {
         );
         self.set_window_size(new_size);
 
-        // TODO: This should just trigger a pending reflow instead of forcing one now.
-        self.force_reflow(ReflowGoal::Full, ReflowReason::WindowResize, None);
-
         // http://dev.w3.org/csswg/cssom-view/#resizing-viewports
         if size_type == WindowSizeType::Resize {
             let uievent = UIEvent::new(
@@ -2521,6 +2509,10 @@ impl Window {
             uievent.upcast::<Event>().fire(self.upcast(), can_gc);
         }
 
+        // The document needs to be repainted, because the initial containing block
+        // is now a different size.
+        self.Document().set_needs_paint(true);
+
         true
     }
 
@@ -2707,7 +2699,6 @@ impl Window {
             window_size: Cell::new(window_size),
             current_viewport: Cell::new(initial_viewport.to_untyped()),
             suppress_reflow: Cell::new(true),
-            pending_reflow_count: Default::default(),
             current_state: Cell::new(WindowState::Alive),
             devtools_marker_sender: Default::default(),
             devtools_markers: Default::default(),

components/script/layout_dom/document.rs

@@ -49,14 +49,6 @@ impl<'ld> ServoLayoutDocument<'ld> {
             .next()
     }
 
-    pub fn needs_paint_from_layout(&self) {
-        self.document.needs_paint_from_layout()
-    }
-
-    pub fn will_paint(&self) {
-        self.document.will_paint()
-    }
-
     pub fn style_shared_lock(&self) -> &StyleSharedRwLock {
         self.document.style_shared_lock()
     }

components/script/script_thread.rs

@@ -1635,9 +1635,8 @@ impl ScriptThread {
             // > Step 22: For each doc of docs, update the rendering or user interface of
             // > doc and its node navigable to reflect the current state.
             let window = document.window();
-            let pending_reflows = window.get_pending_reflow_count();
-            if document.is_fully_active() && pending_reflows > 0 {
-                window.reflow(ReflowGoal::Full, ReflowReason::PendingReflow, can_gc);
+            if document.is_fully_active() {
+                window.reflow(ReflowGoal::Full, ReflowReason::UpdateTheRendering, can_gc);
             }
 
             // TODO: Process top layer removals according to
@@ -1790,7 +1789,7 @@ impl ScriptThread {
                 },
                 FromConstellation(ConstellationControlMsg::Viewport(id, rect)) => self
                     .profile_event(ScriptThreadEventCategory::SetViewport, Some(id), || {
-                        self.handle_viewport(id, rect, can_gc);
+                        self.handle_viewport(id, rect);
                     }),
                 FromConstellation(ConstellationControlMsg::TickAllAnimations(
                     pipeline_id,
@@ -2492,7 +2491,7 @@ impl ScriptThread {
                 self.collect_reports(chan)
             },
             MainThreadScriptMsg::WorkletLoaded(pipeline_id) => {
-                self.handle_worklet_loaded(pipeline_id, CanGc::note())
+                self.handle_worklet_loaded(pipeline_id)
             },
             MainThreadScriptMsg::RegisterPaintWorklet {
                 pipeline_id,
@@ -2867,12 +2866,10 @@ impl ScriptThread {
         }
     }
 
-    fn handle_viewport(&self, id: PipelineId, rect: Rect<f32>, can_gc: CanGc) {
+    fn handle_viewport(&self, id: PipelineId, rect: Rect<f32>) {
         let document = self.documents.borrow().find_document(id);
         if let Some(document) = document {
-            if document.window().set_page_clip_rect_with_new_viewport(rect) {
-                self.rebuild_and_force_reflow(&document, ReflowReason::Viewport, can_gc);
-            }
+            document.window().set_page_clip_rect_with_new_viewport(rect);
             return;
         }
         let loads = self.incomplete_loads.borrow();
@@ -3381,7 +3378,6 @@ impl ScriptThread {
 
         // TODO: This should only dirty nodes that are waiting for a web font to finish loading!
         document.dirty_all_nodes();
-        document.window().add_pending_reflow();
 
         // This is required because the handlers added to the promise exposed at
         // `document.fonts.ready` are run by the event loop only when it performs a microtask
@@ -3390,11 +3386,11 @@ impl ScriptThread {
         self.rendering_opportunity(pipeline_id);
     }
 
-    /// Handles a worklet being loaded. Does nothing if the page no longer exists.
-    fn handle_worklet_loaded(&self, pipeline_id: PipelineId, can_gc: CanGc) {
-        let document = self.documents.borrow().find_document(pipeline_id);
-        if let Some(document) = document {
-            self.rebuild_and_force_reflow(&document, ReflowReason::WorkletLoaded, can_gc);
+    /// Handles a worklet being loaded by triggering a relayout of the page. Does nothing if the
+    /// page no longer exists.
+    fn handle_worklet_loaded(&self, pipeline_id: PipelineId) {
+        if let Some(document) = self.documents.borrow().find_document(pipeline_id) {
+            document.set_needs_paint(true)
         }
     }
 
@@ -3857,13 +3853,6 @@ impl ScriptThread {
         }
     }
 
-    /// Reflows non-incrementally, rebuilding the entire layout tree in the process.
-    fn rebuild_and_force_reflow(&self, document: &Document, reason: ReflowReason, can_gc: CanGc) {
-        let window = window_from_node(document);
-        document.dirty_all_nodes();
-        window.reflow(ReflowGoal::Full, reason, can_gc);
-    }
-
     /// Queue compositor events for later dispatching as part of a
     /// `update_the_rendering` task.
     fn handle_event(&self, pipeline_id: PipelineId, event: CompositorEvent) {

tests/wpt/meta/css/css-ui/transparent-accent-color-001.html.ini

@@ -1,2 +0,0 @@
-[transparent-accent-color-001.html]
-  expected: TIMEOUT

tests/wpt/meta/css/css-ui/transparent-accent-color-002.html.ini

@@ -1,2 +0,0 @@
-[transparent-accent-color-002.html]
-  expected: TIMEOUT