Cache the result of retrieving an element's client rectangle from layout.

4 files changed, 124 insertions, 21 deletions

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 1e60b3b79fc..d7ba280cdf8 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -588,16 +588,28 @@ impl Document {
         self.needs_paint.get()
     }
 
-    pub fn needs_reflow(&self) -> bool {
+    pub fn needs_reflow(&self) -> Option<ReflowTriggerCondition> {
         // FIXME: This should check the dirty bit on the document,
         // not the document element. Needs some layout changes to make
         // that workable.
-        self.stylesheets.borrow().has_changed() ||
-            self.GetDocumentElement().map_or(false, |root| {
-                root.upcast::<Node>().has_dirty_descendants() ||
-                    !self.pending_restyles.borrow().is_empty() ||
-                    self.needs_paint()
-            })
+        if self.stylesheets.borrow().has_changed() {
+            return Some(ReflowTriggerCondition::StylesheetsChanged);
+        }
+
+        let root = self.GetDocumentElement()?;
+        if root.upcast::<Node>().has_dirty_descendants() {
+            return Some(ReflowTriggerCondition::DirtyDescendants);
+        }
+
+        if !self.pending_restyles.borrow().is_empty() {
+            return Some(ReflowTriggerCondition::PendingRestyles);
+        }
+
+        if self.needs_paint() {
+            return Some(ReflowTriggerCondition::PaintPostponed);
+        }
+
+        None
     }
 
     /// Returns the first `base` element in the DOM that has an `href` attribute.
@@ -1683,7 +1695,7 @@ impl Document {
             // is considered spurious, we need to ensure that the layout
             // and compositor *do* tick the animation.
             self.window
-                .force_reflow(ReflowGoal::Full, ReflowReason::RequestAnimationFrame);
+                .force_reflow(ReflowGoal::Full, ReflowReason::RequestAnimationFrame, None);
         }
 
         // Only send the animation change state message after running any callbacks.
@@ -4954,3 +4966,11 @@ impl PendingScript {
             .map(|result| (DomRoot::from_ref(&*self.element), result))
     }
 }
+
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum ReflowTriggerCondition {
+    StylesheetsChanged,
+    DirtyDescendants,
+    PendingRestyles,
+    PaintPostponed,
+}
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index 7d167bf2574..fa9e0f82cbc 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -85,6 +85,7 @@ use crate::stylesheet_loader::StylesheetOwner;
 use crate::task::TaskOnce;
 use devtools_traits::AttrInfo;
 use dom_struct::dom_struct;
+use euclid::default::Rect;
 use html5ever::serialize;
 use html5ever::serialize::SerializeOpts;
 use html5ever::serialize::TraversalScope;
@@ -2438,22 +2439,22 @@ impl ElementMethods for Element {
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clienttop
     fn ClientTop(&self) -> i32 {
-        self.upcast::<Node>().client_rect().origin.y
+        self.client_rect().origin.y
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientleft
     fn ClientLeft(&self) -> i32 {
-        self.upcast::<Node>().client_rect().origin.x
+        self.client_rect().origin.x
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientwidth
     fn ClientWidth(&self) -> i32 {
-        self.upcast::<Node>().client_rect().size.width
+        self.client_rect().size.width
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-element-clientheight
     fn ClientHeight(&self) -> i32 {
-        self.upcast::<Node>().client_rect().size.height
+        self.client_rect().size.height
     }
 
     /// <https://w3c.github.io/DOM-Parsing/#widl-Element-innerHTML>
@@ -3206,6 +3207,20 @@ impl<'a> SelectorsElement for DomRoot<Element> {
 }
 
 impl Element {
+    fn client_rect(&self) -> Rect<i32> {
+        if let Some(rect) = self
+            .rare_data()
+            .as_ref()
+            .and_then(|data| data.client_rect.as_ref())
+            .and_then(|rect| rect.get().ok())
+        {
+            return rect;
+        }
+        let rect = self.upcast::<Node>().client_rect();
+        self.ensure_rare_data().client_rect = Some(window_from_node(self).cache_layout_value(rect));
+        rect
+    }
+
     pub fn as_maybe_activatable(&self) -> Option<&dyn Activatable> {
         let element = match self.upcast::<Node>().type_id() {
             NodeTypeId::Element(ElementTypeId::HTMLElement(
diff --git a/components/script/dom/raredata.rs b/components/script/dom/raredata.rs
index 198b6bfe720..9d4a82d63d6 100644
--- a/components/script/dom/raredata.rs
+++ b/components/script/dom/raredata.rs
@@ -9,6 +9,8 @@ use crate::dom::customelementregistry::{
 use crate::dom::mutationobserver::RegisteredObserver;
 use crate::dom::node::UniqueId;
 use crate::dom::shadowroot::ShadowRoot;
+use crate::dom::window::LayoutValue;
+use euclid::default::Rect;
 use servo_atoms::Atom;
 use std::rc::Rc;
 
@@ -46,4 +48,6 @@ pub struct ElementRareData {
     /// The "name" content attribute; not used as frequently as id, but used
     /// in named getter loops so it's worth looking up quickly when present
     pub name_attribute: Option<Atom>,
+    /// The client rect reported by layout.
+    pub client_rect: Option<LayoutValue<Rect<i32>>>,
 }
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 688ced4fda7..89a49de8d9c 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -23,14 +23,14 @@ use crate::dom::bindings::reflector::DomObject;
 use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom};
 use crate::dom::bindings::str::{DOMString, USVString};
 use crate::dom::bindings::structuredclone;
-use crate::dom::bindings::trace::RootedTraceableBox;
+use crate::dom::bindings::trace::{JSTraceable, RootedTraceableBox};
 use crate::dom::bindings::utils::{GlobalStaticData, WindowProxyHandler};
 use crate::dom::bindings::weakref::DOMTracker;
 use crate::dom::bluetooth::BluetoothExtraPermissionData;
 use crate::dom::crypto::Crypto;
 use crate::dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
 use crate::dom::customelementregistry::CustomElementRegistry;
-use crate::dom::document::{AnimationFrameCallback, Document};
+use crate::dom::document::{AnimationFrameCallback, Document, ReflowTriggerCondition};
 use crate::dom::element::Element;
 use crate::dom::event::{Event, EventStatus};
 use crate::dom::eventtarget::EventTarget;
@@ -55,6 +55,7 @@ use crate::dom::worklet::Worklet;
 use crate::dom::workletglobalscope::WorkletGlobalScopeType;
 use crate::fetch;
 use crate::layout_image::fetch_image_for_layout;
+use crate::malloc_size_of::MallocSizeOf;
 use crate::microtask::MicrotaskQueue;
 use crate::realms::InRealm;
 use crate::script_runtime::{
@@ -334,6 +335,12 @@ pub struct Window {
     event_loop_waker: Option<Box<dyn EventLoopWaker>>,
 
     visible: Cell<bool>,
+
+    /// A shared marker for the validity of any cached layout values. A value of true
+    /// indicates that any such values remain valid; any new layout that invalidates
+    /// those values will cause the marker to be set to false.
+    #[ignore_malloc_size_of = "Rc is hard"]
+    layout_marker: DomRefCell<Rc<Cell<bool>>>,
 }
 
 impl Window {
@@ -1561,7 +1568,12 @@ impl Window {
     ///
     /// Returns true if layout actually happened, false otherwise.
     #[allow(unsafe_code)]
-    pub fn force_reflow(&self, reflow_goal: ReflowGoal, reason: ReflowReason) -> bool {
+    pub fn force_reflow(
+        &self,
+        reflow_goal: ReflowGoal,
+        reason: ReflowReason,
+        condition: Option<ReflowTriggerCondition>,
+    ) -> bool {
         self.Document().ensure_safe_to_run_script_or_layout();
         // Check if we need to unsuppress reflow. Note that this needs to be
         // *before* any early bailouts, or reflow might never be unsuppresed!
@@ -1580,6 +1592,19 @@ impl Window {
             return false;
         }
 
+        if condition != Some(ReflowTriggerCondition::PaintPostponed) {
+            debug!(
+                "Invalidating layout cache due to reflow condition {:?}",
+                condition
+            );
+            // Invalidate any existing cached layout values.
+            self.layout_marker.borrow().set(false);
+            // Create a new layout caching token.
+            *self.layout_marker.borrow_mut() = Rc::new(Cell::new(true));
+        } else {
+            debug!("Not invalidating cached layout values for paint-only reflow.");
+        }
+
         debug!("script: performing reflow for reason {:?}", reason);
 
         let marker = if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) {
@@ -1714,16 +1739,18 @@ impl Window {
         let for_display = reflow_goal == ReflowGoal::Full;
 
         let mut issued_reflow = false;
-        if !for_display || self.Document().needs_reflow() {
-            issued_reflow = self.force_reflow(reflow_goal, reason);
+        let condition = self.Document().needs_reflow();
+        if !for_display || condition.is_some() {
+            issued_reflow = self.force_reflow(reflow_goal, reason, condition);
 
             // 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()) ||
+            assert!({
+                let condition = self.Document().needs_reflow();
+                condition.is_none() ||
+                    (!for_display && condition == Some(ReflowTriggerCondition::PaintPostponed)) ||
                     self.suppress_reflow.get()
-            );
+            });
         } else {
             debug!(
                 "Document doesn't need reflow - skipping it (reason {:?})",
@@ -2348,6 +2375,7 @@ impl Window {
             player_context,
             event_loop_waker,
             visible: Cell::new(true),
+            layout_marker: DomRefCell::new(Rc::new(Cell::new(true))),
         });
 
         unsafe { WindowBinding::Wrap(JSContext::from_ptr(runtime.cx()), win) }
@@ -2356,6 +2384,42 @@ impl Window {
     pub fn pipeline_id(&self) -> PipelineId {
         self.upcast::<GlobalScope>().pipeline_id()
     }
+
+    /// Create a new cached instance of the given value.
+    pub fn cache_layout_value<T>(&self, value: T) -> LayoutValue<T>
+    where
+        T: Copy + JSTraceable + MallocSizeOf,
+    {
+        LayoutValue::new(self.layout_marker.borrow().clone(), value)
+    }
+}
+
+/// An instance of a value associated with a particular snapshot of layout. This stored
+/// value can only be read as long as the associated layout marker that is considered
+/// valid. It will automatically become unavailable when the next layout operation is
+/// performed.
+#[derive(JSTraceable, MallocSizeOf)]
+pub struct LayoutValue<T: JSTraceable + MallocSizeOf> {
+    #[ignore_malloc_size_of = "Rc is hard"]
+    is_valid: Rc<Cell<bool>>,
+    value: T,
+}
+
+impl<T: Copy + JSTraceable + MallocSizeOf> LayoutValue<T> {
+    fn new(marker: Rc<Cell<bool>>, value: T) -> Self {
+        LayoutValue {
+            is_valid: marker,
+            value,
+        }
+    }
+
+    /// Retrieve the stored value if it is still valid.
+    pub fn get(&self) -> Result<T, ()> {
+        if self.is_valid.get() {
+            return Ok(self.value);
+        }
+        Err(())
+    }
 }
 
 fn should_move_clip_rect(clip_rect: UntypedRect<Au>, new_viewport: UntypedRect<f32>) -> bool {