servo: Merge #15599 - Implement "handled for descendants" tracking for RestyleDamage (from bholley:damage_handled); r=emilio

Reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1340022

Source-Repo: https://github.com/servo/servo
Source-Revision: eb916f290334f9f56c4c8680e25224a16e4c7d59

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 1fce2695c7ef135fba573911450e116f540d8af9

servo/components/style/data.rs

@@ -245,7 +245,7 @@ static NO_SNAPSHOT: Option<Snapshot> = None;
 /// We really want to store an Option<Snapshot> here, but we can't drop Gecko
 /// Snapshots off-main-thread. So we make a convenient little wrapper to provide
 /// the semantics of Option<Snapshot>, while deferring the actual drop.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct SnapshotOption {
     snapshot: Option<Snapshot>,
     destroyed: bool,
@@ -292,7 +292,7 @@ impl Deref for SnapshotOption {
 /// Transient data used by the restyle algorithm. This structure is instantiated
 /// either before or during restyle traversal, and is cleared at the end of node
 /// processing.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct RestyleData {
     /// The restyle hint, which indicates whether selectors need to be rematched
     /// for this element, its children, and its descendants.
@@ -306,22 +306,21 @@ pub struct RestyleData {
     /// afte restyling.
     pub damage: RestyleDamage,
 
+    /// The restyle damage that has already been handled by our ancestors, and does
+    /// not need to be applied again at this element. Only non-empty during the
+    /// traversal, once ancestor damage has been calculated.
+    ///
+    /// Note that this optimization mostly makes sense in terms of Gecko's top-down
+    /// frame constructor and change list processing model. We don't bother with it
+    /// for Servo for now.
+    #[cfg(feature = "gecko")]
+    pub damage_handled: RestyleDamage,
+
     /// An optional snapshot of the original state and attributes of the element,
     /// from which we may compute additional restyle hints at traversal time.
     pub snapshot: SnapshotOption,
 }
 
-impl Default for RestyleData {
-    fn default() -> Self {
-        RestyleData {
-            hint: StoredRestyleHint::default(),
-            recascade: false,
-            damage: RestyleDamage::empty(),
-            snapshot: SnapshotOption::empty(),
-        }
-    }
-}
-
 impl RestyleData {
     /// Expands the snapshot (if any) into a restyle hint. Returns true if later
     /// siblings must be restyled.
@@ -354,6 +353,28 @@ impl RestyleData {
             self.recascade ||
             self.snapshot.is_some()
     }
+
+    /// Returns damage handled.
+    #[cfg(feature = "gecko")]
+    pub fn damage_handled(&self) -> RestyleDamage {
+        self.damage_handled
+    }
+
+    /// Returns damage handled (always empty for servo).
+    #[cfg(feature = "servo")]
+    pub fn damage_handled(&self) -> RestyleDamage {
+        RestyleDamage::empty()
+    }
+
+    /// Sets damage handled.
+    #[cfg(feature = "gecko")]
+    pub fn set_damage_handled(&mut self, d: RestyleDamage) {
+        self.damage_handled = d;
+    }
+
+    /// Sets damage handled. No-op for Servo.
+    #[cfg(feature = "servo")]
+    pub fn set_damage_handled(&mut self, _: RestyleDamage) {}
 }
 
 /// Style system data associated with an Element.

servo/components/style/gecko/restyle_damage.rs

@@ -9,7 +9,7 @@ use gecko_bindings::structs;
 use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
 use gecko_bindings::sugar::ownership::FFIArcHelpers;
 use properties::ComputedValues;
-use std::ops::{BitOr, BitOrAssign};
+use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
 use std::sync::Arc;
 
 /// The representation of Gecko's restyle damage is just a wrapper over
@@ -56,16 +56,35 @@ impl GeckoRestyleDamage {
         GeckoRestyleDamage(hint)
     }
 
-    /// Get a restyle damage that represents the maximum action to be taken
-    /// (rebuild and reflow).
-    pub fn rebuild_and_reflow() -> Self {
+    /// Returns true if this restyle damage contains all the damage of |other|.
+    pub fn contains(self, other: Self) -> bool {
+        self & other == other
+    }
+
+    /// Gets restyle damage to reconstruct the entire frame, subsuming all
+    /// other damage.
+    pub fn reconstruct() -> Self {
         GeckoRestyleDamage(structs::nsChangeHint_nsChangeHint_ReconstructFrame)
     }
+
+    /// Assuming |self| is applied to an element, returns the set of damage that
+    /// would be superfluous to apply for descendants.
+    pub fn handled_for_descendants(self) -> Self {
+        let hint = unsafe {
+            bindings::Gecko_HintsHandledForDescendants(self.0)
+        };
+        GeckoRestyleDamage(hint)
+    }
+}
+
+impl Default for GeckoRestyleDamage {
+    fn default() -> Self {
+        Self::empty()
+    }
 }
 
 impl BitOr for GeckoRestyleDamage {
     type Output = Self;
-
     fn bitor(self, other: Self) -> Self {
         GeckoRestyleDamage(self.0 | other.0)
     }
@@ -76,3 +95,17 @@ impl BitOrAssign for GeckoRestyleDamage {
         *self = *self | other;
     }
 }
+
+impl BitAnd for GeckoRestyleDamage {
+    type Output = Self;
+    fn bitand(self, other: Self) -> Self {
+        GeckoRestyleDamage(nsChangeHint((self.0).0 & (other.0).0))
+    }
+}
+
+impl Not for GeckoRestyleDamage {
+    type Output = Self;
+    fn not(self) -> Self {
+        GeckoRestyleDamage(nsChangeHint(!(self.0).0))
+    }
+}

servo/components/style/gecko_bindings/bindings.rs

@@ -649,6 +649,10 @@ extern "C" {
                                      newstyle: ServoComputedValuesBorrowed)
      -> nsChangeHint;
 }
+extern "C" {
+    pub fn Gecko_HintsHandledForDescendants(aHint: nsChangeHint)
+     -> nsChangeHint;
+}
 extern "C" {
     pub fn Gecko_CreateElementSnapshot(element: RawGeckoElementBorrowed)
      -> ServoElementSnapshotOwned;

servo/components/style/matching.rs

@@ -569,7 +569,36 @@ trait PrivateMatchMethods: TElement {
         }
     }
 
+    /// Computes and applies non-redundant damage.
+    #[cfg(feature = "gecko")]
+    fn accumulate_damage(&self,
+                         restyle: &mut RestyleData,
+                         old_values: &Arc<ComputedValues>,
+                         new_values: &Arc<ComputedValues>,
+                         pseudo: Option<&PseudoElement>) {
+        // If an ancestor is already getting reconstructed by Gecko's top-down
+        // frame constructor, no need to apply damage.
+        if restyle.damage_handled.contains(RestyleDamage::reconstruct()) {
+            restyle.damage = RestyleDamage::empty();
+            return;
+        }
+
+        // Add restyle damage, but only the bits that aren't redundant with respect
+        // to damage applied on our ancestors.
+        //
+        // See https://bugzilla.mozilla.org/show_bug.cgi?id=1301258#c12
+        // for followup work to make the optimization here more optimal by considering
+        // each bit individually.
+        if !restyle.damage.contains(RestyleDamage::reconstruct()) {
+            let new_damage = self.compute_restyle_damage(&old_values, &new_values, pseudo);
+            if !restyle.damage_handled.contains(new_damage) {
+                restyle.damage |= new_damage;
+            }
+        }
+    }
+
     /// Computes and applies restyle damage unless we've already maxed it out.
+    #[cfg(feature = "servo")]
     fn accumulate_damage(&self,
                          restyle: &mut RestyleData,
                          old_values: &Arc<ComputedValues>,
@@ -712,7 +741,7 @@ pub trait MatchMethods : TElement {
             if let Some(r) = data.get_restyle_mut() {
                 // Any changes to the matched pseudo-elements trigger
                 // reconstruction.
-                r.damage |= RestyleDamage::rebuild_and_reflow();
+                r.damage |= RestyleDamage::reconstruct();
             }
         }
 
@@ -924,7 +953,7 @@ pub trait MatchMethods : TElement {
                     RestyleDamage::empty()
                 } else {
                     // Something else. Be conservative for now.
-                    RestyleDamage::rebuild_and_reflow()
+                    RestyleDamage::reconstruct()
                 }
             }
         }

servo/components/style/servo/restyle_damage.rs

@@ -67,11 +67,19 @@ impl ServoRestyleDamage {
 
     /// Returns a bitmask that represents a flow that needs to be rebuilt and
     /// reflowed.
+    ///
+    /// FIXME(bholley): Do we ever actually need this? Shouldn't RECONSTRUCT_FLOW
+    /// imply everything else?
     pub fn rebuild_and_reflow() -> ServoRestyleDamage {
         REPAINT | REPOSITION | STORE_OVERFLOW | BUBBLE_ISIZES | REFLOW_OUT_OF_FLOW | REFLOW |
             RECONSTRUCT_FLOW
     }
 
+    /// Returns a bitmask indicating that the frame needs to be reconstructed.
+    pub fn reconstruct() -> ServoRestyleDamage {
+        RECONSTRUCT_FLOW
+    }
+
     /// Supposing a flow has the given `position` property and this damage,
     /// returns the damage that we should add to the *parent* of this flow.
     pub fn damage_for_parent(self, child_is_absolutely_positioned: bool) -> ServoRestyleDamage {
@@ -112,6 +120,17 @@ impl ServoRestyleDamage {
             }
         }
     }
+
+    /// Servo doesn't implement this optimization.
+    pub fn handled_for_descendants(self) -> Self {
+        Self::empty()
+    }
+}
+
+impl Default for ServoRestyleDamage {
+    fn default() -> Self {
+        Self::empty()
+    }
 }
 
 impl fmt::Display for ServoRestyleDamage {

servo/components/style/traversal.rs

@@ -455,7 +455,10 @@ pub fn recalc_style_at<E, D>(traversal: &D,
     // Preprocess children, propagating restyle hints and handling sibling relationships.
     if traversal.should_traverse_children(&mut context.thread_local, element, &data, DontLog) &&
        (element.has_dirty_descendants() || !propagated_hint.is_empty() || inherited_style_changed) {
-        preprocess_children(traversal, element, propagated_hint, inherited_style_changed);
+        let damage_handled = data.get_restyle().map_or(RestyleDamage::empty(), |r| {
+            r.damage_handled() | r.damage.handled_for_descendants()
+        });
+        preprocess_children(traversal, element, propagated_hint, damage_handled, inherited_style_changed);
     }
 
     // Make sure the dirty descendants bit is not set for the root of a
@@ -557,6 +560,7 @@ fn compute_style<E, D>(_traversal: &D,
 fn preprocess_children<E, D>(traversal: &D,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
+                             damage_handled: RestyleDamage,
                              parent_inherited_style_changed: bool)
     where E: TElement,
           D: DomTraversal<E>
@@ -580,8 +584,7 @@ fn preprocess_children<E, D>(traversal: &D,
         // any reason to create one, avoid the useless allocation and move on to
         // the next child.
         if propagated_hint.is_empty() && !parent_inherited_style_changed &&
-           !child_data.has_restyle()
-        {
+           damage_handled.is_empty() && !child_data.has_restyle() {
             continue;
         }
         let mut restyle_data = child_data.ensure_restyle();
@@ -598,6 +601,9 @@ fn preprocess_children<E, D>(traversal: &D,
             propagated_hint.insert(&(RESTYLE_SELF | RESTYLE_DESCENDANTS).into());
         }
 
+        // Store the damage already handled by ancestors.
+        restyle_data.set_damage_handled(damage_handled);
+
         // If properties that we inherited from the parent changed, we need to recascade.
         //
         // FIXME(bholley): Need to handle explicitly-inherited reset properties somewhere.