From fce58015d6cb298047e05a08f9bcd465ecf08c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 9 Apr 2019 18:03:41 +0000 Subject: [PATCH] style: Cleanup unused style traversal flags. Some of these were unused, some of them were only used in combination with others, so I've unified them. In particular, Forgetful and ClearAnimationOnlyDirtyDescendants were used only together for a very specific task (the final animation traversal), so I merged them into something that has that name. ClearDirtyBits was unused, so I removed along with some code that would no longer be called. Differential Revision: https://phabricator.services.mozilla.com/D25454 --- components/style/dom.rs | 9 --------- components/style/gecko/wrapper.rs | 10 ---------- components/style/matching.rs | 6 +++--- components/style/traversal.rs | 28 +++------------------------- components/style/traversal_flags.rs | 17 ++++------------- 5 files changed, 10 insertions(+), 60 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 63f82a3754f..967b978bad4 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -642,15 +642,6 @@ pub trait TElement: self.unset_dirty_descendants(); } - /// Clear all element flags related to dirtiness. - /// - /// In Gecko, this corresponds to the regular dirty descendants bit, the - /// animation-only dirty descendants bit, the lazy frame construction bit, - /// and the lazy frame construction descendants bit. - unsafe fn clear_dirty_bits(&self) { - self.unset_dirty_descendants(); - } - /// Returns true if this element is a visited link. /// /// Servo doesn't support visited styles yet. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index bab4b1e2113..a1f06a679d9 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1423,16 +1423,6 @@ impl<'le> TElement for GeckoElement<'le> { ) } - #[inline] - unsafe fn clear_dirty_bits(&self) { - self.unset_flags( - ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 | - ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 | - NODE_DESCENDANTS_NEED_FRAMES as u32 | - NODE_NEEDS_FRAME as u32, - ) - } - fn is_visited_link(&self) -> bool { self.state().intersects(ElementState::IN_VISITED_STATE) } diff --git a/components/style/matching.rs b/components/style/matching.rs index b1240443dea..583e41ad82e 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -471,7 +471,7 @@ trait PrivateMatchMethods: TElement { debug!("accumulate_damage_for: {:?}", self); debug_assert!(!shared_context .traversal_flags - .contains(TraversalFlags::Forgetful)); + .contains(TraversalFlags::FinalAnimationTraversal)); let difference = self.compute_style_difference(old_values, new_values, pseudo); @@ -723,11 +723,11 @@ pub trait MatchMethods: TElement { } } - // Don't accumulate damage if we're in a forgetful traversal. + // Don't accumulate damage if we're in the final animation traversal. if context .shared .traversal_flags - .contains(TraversalFlags::Forgetful) + .contains(TraversalFlags::FinalAnimationTraversal) { return ChildCascadeRequirement::MustCascadeChildren; } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 5ed2388ca5d..8bc57ada8b7 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -535,12 +535,6 @@ pub fn recalc_style_at( debug_assert!(!element.has_animation_only_dirty_descendants()); } - debug_assert!( - flags.for_animation_only() || - !flags.contains(TraversalFlags::ClearDirtyBits) || - !element.has_animation_only_dirty_descendants(), - "Should have cleared animation bits already" - ); clear_state_after_traversing(element, data, flags); } @@ -548,27 +542,11 @@ fn clear_state_after_traversing(element: E, data: &mut ElementData, flags: Tr where E: TElement, { - // 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 flags.contains(TraversalFlags::Forgetful) { + if flags.intersects(TraversalFlags::FinalAnimationTraversal) { + debug_assert!(flags.for_animation_only()); data.clear_restyle_flags_and_damage(); - } - - // Clear dirty bits as appropriate. - if flags.for_animation_only() { - if flags.intersects( - TraversalFlags::ClearDirtyBits | TraversalFlags::ClearAnimationOnlyDirtyDescendants, - ) { - unsafe { - element.unset_animation_only_dirty_descendants(); - } - } - } else if flags.contains(TraversalFlags::ClearDirtyBits) { - // The animation traversal happens first, so we don't need to guard against - // clearing the animation bit on the regular traversal. unsafe { - element.clear_dirty_bits(); + element.unset_animation_only_dirty_descendants(); } } } diff --git a/components/style/traversal_flags.rs b/components/style/traversal_flags.rs index a9a2acbf555..0987230ed02 100644 --- a/components/style/traversal_flags.rs +++ b/components/style/traversal_flags.rs @@ -16,15 +16,9 @@ bitflags! { /// Traverse and update all elements with CSS animations since /// @keyframes rules may have changed. Triggered by CSS rule changes. const ForCSSRuleChanges = 1 << 1; - /// 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; - /// Clears all the dirty bits on the elements traversed. - const ClearDirtyBits = 1 << 5; - /// Clears the animation-only dirty descendants bit in the subtree. - const ClearAnimationOnlyDirtyDescendants = 1 << 6; + /// The final animation-only traversal, which shouldn't really care about other + /// style changes anymore. + const FinalAnimationTraversal = 1 << 2; /// Allows the traversal to run in parallel if there are sufficient cores on /// the machine. const ParallelTraversal = 1 << 7; @@ -58,10 +52,7 @@ pub fn assert_traversal_flags_match() { check_traversal_flags! { ServoTraversalFlags_AnimationOnly => TraversalFlags::AnimationOnly, ServoTraversalFlags_ForCSSRuleChanges => TraversalFlags::ForCSSRuleChanges, - ServoTraversalFlags_Forgetful => TraversalFlags::Forgetful, - ServoTraversalFlags_ClearDirtyBits => TraversalFlags::ClearDirtyBits, - ServoTraversalFlags_ClearAnimationOnlyDirtyDescendants => - TraversalFlags::ClearAnimationOnlyDirtyDescendants, + ServoTraversalFlags_FinalAnimationTraversal => TraversalFlags::FinalAnimationTraversal, ServoTraversalFlags_ParallelTraversal => TraversalFlags::ParallelTraversal, ServoTraversalFlags_FlushThrottledAnimations => TraversalFlags::FlushThrottledAnimations, }