Auto merge of #17895 - bholley:clean_up_traversal, r=emilio

Clean up traversal modes

https://bugzilla.mozilla.org/show_bug.cgi?id=1384769

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17895)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -135,7 +135,8 @@ use style::stylesheets::{Origin, Stylesheet, StylesheetInDocument, UserAgentStyl
 use style::stylist::{ExtraStyleData, Stylist};
 use style::thread_state;
 use style::timer::Timer;
-use style::traversal::{DomTraversal, TraversalDriver, TraversalFlags};
+use style::traversal::{DomTraversal, TraversalDriver};
+use style::traversal_flags::TraversalFlags;
 
 /// Information needed by the layout thread.
 pub struct LayoutThread {

components/style/context.rs

@@ -34,7 +34,8 @@ use stylist::Stylist;
 use thread_state;
 use time;
 use timer::Timer;
-use traversal::{DomTraversal, TraversalFlags};
+use traversal::DomTraversal;
+use traversal_flags::TraversalFlags;
 
 pub use selectors::matching::QuirksMode;
 

components/style/dom.rs

@@ -33,7 +33,7 @@ use std::hash::Hash;
 use std::ops::Deref;
 use stylist::Stylist;
 use thread_state;
-use traversal::TraversalFlags;
+use traversal_flags::TraversalFlags;
 
 pub use style_traits::UnsafeNode;
 

components/style/gecko/generated/bindings.rs

@@ -44,8 +44,7 @@ use gecko_bindings::structs::RawGeckoXBLBinding;
 use gecko_bindings::structs::RefPtr;
 use gecko_bindings::structs::CSSPseudoClassType;
 use gecko_bindings::structs::CSSPseudoElementType;
-use gecko_bindings::structs::TraversalRestyleBehavior;
-use gecko_bindings::structs::TraversalRootBehavior;
+use gecko_bindings::structs::ServoTraversalFlags;
 use gecko_bindings::structs::ComputedTimingFunction_BeforeFlag;
 use gecko_bindings::structs::CounterStylePtr;
 use gecko_bindings::structs::FontFamilyList;
@@ -2781,13 +2780,17 @@ extern "C" {
 }
 extern "C" {
     pub fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,
-                                restyle_behavior: TraversalRestyleBehavior,
+                                flags: ServoTraversalFlags,
                                 was_restyled: *mut bool) -> nsChangeHint;
 }
 extern "C" {
     pub fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
                               set: RawServoStyleSetBorrowed,
-                              restyle_behavior: TraversalRestyleBehavior)
+                              flags: ServoTraversalFlags)
+     -> ServoStyleContextStrong;
+}
+extern "C" {
+    pub fn Servo_ResolveStyleAllowStale(element: RawGeckoElementBorrowed)
      -> ServoStyleContextStrong;
 }
 extern "C" {
@@ -2821,9 +2824,7 @@ extern "C" {
     pub fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
                                  set: RawServoStyleSetBorrowed,
                                  snapshots: *const ServoElementSnapshotTable,
-                                 root_behavior: TraversalRootBehavior,
-                                 restyle_behavior: TraversalRestyleBehavior)
-     -> bool;
+                                 flags: ServoTraversalFlags) -> bool;
 }
 extern "C" {
     pub fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed);
@@ -2887,5 +2888,6 @@ extern "C" {
                                               *const ::std::os::raw::c_char,
                                           sourceLen: u32, lineNumber: u32,
                                           colNumber: u32, aURI: *mut nsIURI,
-                                          followup: *const ::std::os::raw::c_char);
+                                          followup:
+                                              *const ::std::os::raw::c_char);
 }

components/style/invalidation/element/restyle_hints.rs

@@ -6,7 +6,7 @@
 
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::nsRestyleHint;
-use traversal::TraversalFlags;
+use traversal_flags::TraversalFlags;
 
 bitflags! {
     /// The kind of restyle we need to do for a given element.

components/style/lib.rs

@@ -135,6 +135,7 @@ pub mod stylesheets;
 pub mod thread_state;
 pub mod timer;
 pub mod traversal;
+pub mod traversal_flags;
 #[macro_use]
 #[allow(non_camel_case_types)]
 pub mod values;

components/style/matching.rs

@@ -19,6 +19,7 @@ use rule_tree::{CascadeLevel, StrongRuleNode};
 use selector_parser::{PseudoElement, RestyleDamage};
 use selectors::matching::ElementSelectorFlags;
 use servo_arc::{Arc, ArcBorrow};
+use traversal_flags;
 
 /// Represents the result of comparing an element's old and new style.
 pub struct StyleDifference {
@@ -172,7 +173,7 @@ trait PrivateMatchMethods: TElement {
             // running or not.
             // TODO: We should check which @keyframes changed/added/deleted
             // and update only animations corresponding to those @keyframes.
-            (context.shared.traversal_flags.for_css_rule_changes() &&
+            (context.shared.traversal_flags.contains(traversal_flags::ForCSSRuleChanges) &&
              has_new_animation_style) ||
             !old_box_style.animations_equals(&new_box_style) ||
              (old_display_style == display::T::none &&
@@ -301,8 +302,8 @@ trait PrivateMatchMethods: TElement {
                              new_values: &Arc<ComputedValues>,
                              pseudo: Option<&PseudoElement>)
                              -> ChildCascadeRequirement {
-        // Don't accumulate damage if we're in a restyle for reconstruction.
-        if shared_context.traversal_flags.for_reconstruct() {
+        // Don't accumulate damage if we're in a forgetful traversal.
+        if shared_context.traversal_flags.contains(traversal_flags::Forgetful) {
             return ChildCascadeRequirement::MustCascadeChildren;
         }
 
@@ -498,8 +499,8 @@ pub trait MatchMethods : TElement {
             }
         }
 
-        // Don't accumulate damage if we're in a restyle for reconstruction.
-        if context.shared.traversal_flags.for_reconstruct() {
+        // Don't accumulate damage if we're in a forgetful traversal.
+        if context.shared.traversal_flags.contains(traversal_flags::Forgetful) {
             return ChildCascadeRequirement::MustCascadeChildren;
         }
 

components/style/traversal.rs

@@ -13,6 +13,7 @@ use sharing::StyleSharingTarget;
 use smallvec::SmallVec;
 use style_resolver::StyleResolverForElement;
 use stylist::RuleInclusion;
+use traversal_flags::{TraversalFlags, self};
 
 /// A per-traversal-level chunk of data. This is sent down by the traversal, and
 /// currently only holds the dom depth for the bloom filter.
@@ -27,45 +28,6 @@ pub struct PerLevelTraversalData {
     pub current_dom_depth: usize,
 }
 
-bitflags! {
-    /// Flags that control the traversal process.
-    pub flags TraversalFlags: u8 {
-        /// Traverse only unstyled children.
-        const UNSTYLED_CHILDREN_ONLY = 0x01,
-        /// Traverse only elements for animation restyles.
-        const ANIMATION_ONLY = 0x02,
-        /// Traverse without generating any change hints.
-        const FOR_RECONSTRUCT = 0x04,
-        /// Traverse triggered by CSS rule changes.
-        ///
-        /// Traverse and update all elements with CSS animations since
-        /// @keyframes rules may have changed
-        const FOR_CSS_RULE_CHANGES = 0x08,
-    }
-}
-
-impl TraversalFlags {
-    /// Returns true if the traversal is for animation-only restyles.
-    pub fn for_animation_only(&self) -> bool {
-        self.contains(ANIMATION_ONLY)
-    }
-
-    /// Returns true if the traversal is for unstyled children.
-    pub fn for_unstyled_children_only(&self) -> bool {
-        self.contains(UNSTYLED_CHILDREN_ONLY)
-    }
-
-    /// Returns true if the traversal is for a frame reconstruction.
-    pub fn for_reconstruct(&self) -> bool {
-        self.contains(FOR_RECONSTRUCT)
-    }
-
-    /// Returns true if the traversal is triggered by CSS rule changes.
-    pub fn for_css_rule_changes(&self) -> bool {
-        self.contains(FOR_CSS_RULE_CHANGES)
-    }
-}
-
 /// This structure exists to enforce that callers invoke pre_traverse, and also
 /// to pass information from the pre-traversal into the primary traversal.
 pub struct PreTraverseToken {
@@ -208,12 +170,7 @@ pub trait DomTraversal<E: TElement> : Sync {
         shared_context: &SharedStyleContext,
         traversal_flags: TraversalFlags
     ) -> PreTraverseToken {
-        debug_assert!(!(traversal_flags.for_reconstruct() &&
-                        traversal_flags.for_unstyled_children_only()),
-                      "must not specify FOR_RECONSTRUCT in combination with \
-                       UNSTYLED_CHILDREN_ONLY");
-
-        if traversal_flags.for_unstyled_children_only() {
+        if traversal_flags.contains(traversal_flags::UnstyledChildrenOnly) {
             if root.borrow_data().map_or(true, |d| d.has_styles() && d.styles.is_display_none()) {
                 return PreTraverseToken {
                     traverse: false,
@@ -281,10 +238,6 @@ pub trait DomTraversal<E: TElement> : Sync {
             return true;
         }
 
-        if traversal_flags.for_reconstruct() {
-            return true;
-        }
-
         // If the element is native-anonymous and an ancestor frame will be
         // reconstructed, the child and all its descendants will be destroyed.
         // In that case, we wouldn't need to traverse the subtree...
@@ -356,9 +309,11 @@ pub trait DomTraversal<E: TElement> : Sync {
         // to traverse any element with damage so that we can perform fixup /
         // reconstruction on our way back up the tree.
         //
-        // We also need to traverse nodes with explicit damage and no other
-        // restyle data, so that this damage can be cleared.
-        if (cfg!(feature = "servo") || traversal_flags.for_reconstruct()) &&
+        // In aggressively forgetful traversals (where we seek out and clear damage
+        // in addition to not computing it) we also need to traverse nodes with
+        // explicit damage and no other restyle data, so that this damage can be cleared.
+        if (cfg!(feature = "servo") ||
+            traversal_flags.contains(traversal_flags::AggressivelyForgetful)) &&
            !data.restyle.damage.is_empty() {
             return true;
         }
@@ -608,7 +563,6 @@ where
     // * We generated a reconstruct hint on self (which could mean that we
     //   switched from display:none to something else, which means the children
     //   need initial styling).
-    // * This is a reconstruct traversal.
     // * This is a servo non-incremental traversal.
     //
     // Additionally, there are a few scenarios where we avoid traversing the
@@ -618,7 +572,6 @@ where
                                 !propagated_hint.is_empty() ||
                                 context.thread_local.is_initial_style() ||
                                 data.restyle.reconstructed_self() ||
-                                flags.for_reconstruct() ||
                                 is_servo_nonincremental_layout();
 
     traverse_children = traverse_children &&
@@ -636,17 +589,18 @@ where
         );
     }
 
-    // If we are in a restyle for reconstruction, drop the existing restyle
+    // If we are in a forgetful traversal, drop the existing restyle
     // data here, since we won't need to perform a post-traversal to pick up
     // any change hints.
-    if context.shared.traversal_flags.for_reconstruct() {
+    if flags.contains(traversal_flags::Forgetful) {
         data.clear_restyle_state();
     }
 
     // There are two cases when we want to clear the dity descendants bit here
-    // after styling this element.
+    // after styling this element. The first case is when we were explicitly
+    // asked to clear the bit by the caller.
     //
-    // The first case is when this element is the root of a display:none
+    // The second case is when this element is the root of a display:none
     // subtree, even if the style didn't change (since, if the style did change,
     // we'd have already cleared it above).
     //
@@ -655,14 +609,16 @@ where
     // moderately expensive). Instead, DOM implementations can unconditionally
     // set the dirty descendants bit on any styled parent, and let the traversal
     // sort it out.
-    //
-    // The second case is when we are in a restyle for reconstruction, where we
-    // won't need to perform a post-traversal to pick up any change hints.
-    if data.styles.is_display_none() ||
-       context.shared.traversal_flags.for_reconstruct() {
+    if flags.contains(traversal_flags::ClearDirtyDescendants) ||
+       data.styles.is_display_none() {
         unsafe { element.unset_dirty_descendants(); }
     }
 
+    // Similarly, check if we're supposed to clear the animation bit.
+    if flags.contains(traversal_flags::ClearAnimationOnlyDirtyDescendants) {
+        unsafe { element.unset_animation_only_dirty_descendants(); }
+    }
+
     context.thread_local.end_element(element);
 }
 
@@ -819,10 +775,9 @@ where
             // Set the dirty descendants bit on the parent as needed, so that we
             // can find elements during the post-traversal.
             //
-            // If we are in a restyle for reconstruction, there is no need to
-            // perform a post-traversal, so we don't need to set the dirty
-            // descendants bit on the parent.
-            if !flags.for_reconstruct() && !is_initial_style {
+            // Note that these bits may be cleared again at the bottom of
+            // recalc_style_at if requested by the caller.
+            if !is_initial_style {
                 if flags.for_animation_only() {
                     unsafe { element.set_animation_only_dirty_descendants(); }
                 } else {

components/style/traversal_flags.rs

@@ -0,0 +1,72 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! Flags that control the traversal process.
+//!
+//! We CamelCase rather than UPPER_CASING so that we can grep for the same
+//! strings across gecko and servo.
+#![allow(non_upper_case_globals)]
+
+bitflags! {
+    /// Flags that control the traversal process.
+    pub flags TraversalFlags: u32 {
+        /// Traverse only elements for animation restyles.
+        const AnimationOnly = 1 << 0,
+        /// Traverse and update all elements with CSS animations since
+        /// @keyframes rules may have changed. Triggered by CSS rule changes.
+        const ForCSSRuleChanges = 1 << 1,
+        /// Traverse only unstyled children of the root and their descendants.
+        const UnstyledChildrenOnly = 1 << 2,
+        /// A forgetful traversal ignores the previous state of the frame tree, and
+        /// thus does not compute damage or maintain other state describing the styles
+        /// pre-traversal. A forgetful traversal is usually the right thing if you
+        /// aren't going to do a post-traversal.
+        const Forgetful = 1 << 3,
+        /// Actively seeks out and clears change hints that may have been posted into
+        /// the tree. Nonsensical without also passing Forgetful.
+        const AggressivelyForgetful = 1 << 4,
+        /// Clears the dirty descendants bit in the subtree.
+        const ClearDirtyDescendants = 1 << 5,
+        /// Clears the animation-only dirty descendants bit in the subtree.
+        const ClearAnimationOnlyDirtyDescendants = 1 << 6,
+    }
+}
+
+/// Asserts that all TraversalFlags flags have a matching ServoTraversalFlags value in gecko.
+#[cfg(feature = "gecko")]
+#[inline]
+pub fn assert_traversal_flags_match() {
+    use gecko_bindings::structs;
+
+    macro_rules! check_traversal_flags {
+        ( $( $a:ident => $b:ident ),*, ) => {
+            if cfg!(debug_assertions) {
+                let mut modes = TraversalFlags::all();
+                $(
+                    assert_eq!(structs::$a as usize, $b.bits() as usize, stringify!($b));
+                    modes.remove($b);
+                )*
+                assert_eq!(modes, TraversalFlags::empty(), "all TraversalFlags bits should have an assertion");
+            }
+        }
+    }
+
+    check_traversal_flags! {
+        ServoTraversalFlags_AnimationOnly => AnimationOnly,
+        ServoTraversalFlags_ForCSSRuleChanges => ForCSSRuleChanges,
+        ServoTraversalFlags_UnstyledChildrenOnly => UnstyledChildrenOnly,
+        ServoTraversalFlags_Forgetful => Forgetful,
+        ServoTraversalFlags_AggressivelyForgetful => AggressivelyForgetful,
+        ServoTraversalFlags_ClearDirtyDescendants => ClearDirtyDescendants,
+        ServoTraversalFlags_ClearAnimationOnlyDirtyDescendants =>
+            ClearAnimationOnlyDirtyDescendants,
+    }
+}
+
+impl TraversalFlags {
+    /// Returns true if the traversal is for animation-only restyles.
+    pub fn for_animation_only(&self) -> bool {
+        self.contains(AnimationOnly)
+    }
+}