Bug 1388031 - Cleanup code that was used for verifying styling results for throttled animation flush in post traversal. r?bholley
Now that we do process normal traversal even in the case of throttled animation
flush so that we don't need to do special handling for the case.
Note about the comment in has_current_styles():
the remaining animation hints is not caused by either this patch or the
previous patch in this patch series, it's been there in the first place, but
it should be fixed somehow later.
Below is the tests that leave animation hints when an animating element is
moved into subtree of a contenteditable element.
layout/style/crashtests/1383319.html
layout/style/crashtests/1383001.html
MozReview-Commit-ID: 7nYr00Tx9cy
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -417,19 +417,17 @@ ServoRestyleManager::ClearRestyleStateFr
for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
if (n->IsElement()) {
ClearRestyleStateFromSubtree(n->AsElement());
}
}
}
bool wasRestyled;
- Unused << Servo_TakeChangeHint(aElement,
- ServoTraversalFlags::Empty,
- &wasRestyled);
+ Unused << Servo_TakeChangeHint(aElement, &wasRestyled);
aElement->UnsetHasDirtyDescendantsForServo();
aElement->UnsetHasAnimationOnlyDirtyDescendantsForServo();
aElement->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES);
}
/**
* This struct takes care of encapsulating some common state that text nodes may
* need to track during the post-traversal.
@@ -703,19 +701,17 @@ ServoRestyleManager::ProcessPostTraversa
primaryFrame &&
primaryFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW);
// Grab the change hint from Servo.
// In case of flushing throttled animations, any restyle hints other than
// animations are preserved since they are the hints which will be processed
// in normal restyle later.
bool wasRestyled;
- nsChangeHint changeHint = Servo_TakeChangeHint(aElement,
- aFlags,
- &wasRestyled);
+ nsChangeHint changeHint = Servo_TakeChangeHint(aElement, &wasRestyled);
// We should really fix the weird primary frame mapping for image maps
// (bug 135040)...
if (styleFrame && styleFrame->GetContent() != aElement) {
MOZ_ASSERT(styleFrame->IsImageFrame());
styleFrame = nullptr;
}
@@ -797,17 +793,17 @@ ServoRestyleManager::ProcessPostTraversa
// other elements without frames).
ServoRestyleState& childrenRestyleState =
thisFrameRestyleState ? *thisFrameRestyleState : aRestyleState;
RefPtr<ServoStyleContext> newContext = nullptr;
if (wasRestyled && oldStyleContext) {
MOZ_ASSERT(styleFrame || displayContentsStyle);
newContext =
- aRestyleState.StyleSet().ResolveServoStyle(aElement, aFlags);
+ aRestyleState.StyleSet().ResolveServoStyle(aElement);
MOZ_ASSERT(oldStyleContext->ComputedData() != newContext->ComputedData());
newContext->ResolveSameStructsAs(oldStyleContext);
// We want to walk all the continuations here, even the ones with different
// styles. In practice, the only reason we get continuations with different
// styles here is ::first-line (::first-letter never affects element
// styles). But in that case, newContext is the right context for the
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -1955,18 +1955,17 @@ nsCSSFrameConstructor::CreateGeneratedCo
if (!hasServoAnimations) {
Servo_SetExplicitStyle(container, servoStyle);
} else {
// If animations are involved, we avoid the SetExplicitStyle optimization
// above. We need to grab style with animations from the pseudo element
// and replace old one.
mPresShell->StyleSet()->AsServo()->StyleNewSubtree(container);
pseudoStyleContext =
- styleSet->AsServo()->ResolveServoStyle(container,
- ServoTraversalFlags::Empty);
+ styleSet->AsServo()->ResolveServoStyle(container);
}
} else {
mozilla::GeckoRestyleManager* geckoRM = RestyleManager()->AsGecko();
GeckoRestyleManager::ReframingStyleContexts* rsc =
geckoRM->GetReframingStyleContexts();
if (rsc) {
RefPtr<GeckoStyleContext> newContext =
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -499,22 +499,20 @@ SERVO_BINDING_FUNC(Servo_Initialize, voi
SERVO_BINDING_FUNC(Servo_Shutdown, void)
// Restyle and change hints.
SERVO_BINDING_FUNC(Servo_NoteExplicitHints, void, RawGeckoElementBorrowed element,
nsRestyleHint restyle_hint, nsChangeHint change_hint)
SERVO_BINDING_FUNC(Servo_TakeChangeHint,
nsChangeHint,
RawGeckoElementBorrowed element,
- mozilla::ServoTraversalFlags flags,
bool* was_restyled)
SERVO_BINDING_FUNC(Servo_ResolveStyle, ServoStyleContextStrong,
RawGeckoElementBorrowed element,
- RawServoStyleSetBorrowed set,
- mozilla::ServoTraversalFlags flags)
+ RawServoStyleSetBorrowed set)
SERVO_BINDING_FUNC(Servo_ResolveStyleAllowStale, ServoStyleContextStrong,
RawGeckoElementBorrowed element)
SERVO_BINDING_FUNC(Servo_ResolvePseudoStyle, ServoStyleContextStrong,
RawGeckoElementBorrowed element,
mozilla::CSSPseudoElementType pseudo_type,
bool is_probe,
ServoStyleContextBorrowedOrNull inherited_style,
RawServoStyleSetBorrowed set)
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -192,17 +192,17 @@ ServoStyleSet::ResolveStyleFor(Element*
{
RefPtr<ServoStyleContext> computedValues;
if (aMayCompute == LazyComputeBehavior::Allow) {
PreTraverseSync();
return ResolveStyleLazilyInternal(
aElement, CSSPseudoElementType::NotPseudo, nullptr, aParentContext);
}
- return ResolveServoStyle(aElement, ServoTraversalFlags::Empty);
+ return ResolveServoStyle(aElement);
}
/**
* Clears any stale Servo element data that might existing in the specified
* element's document. Upon destruction, asserts that the element and all
* its ancestors still have no element data, if the document has no pres shell.
*/
class MOZ_STACK_CLASS AutoClearStaleData
@@ -463,19 +463,17 @@ ServoStyleSet::ResolvePseudoElementStyle
MOZ_ASSERT(aType < CSSPseudoElementType::Count);
RefPtr<ServoStyleContext> computedValues;
if (aPseudoElement) {
MOZ_ASSERT(aType == aPseudoElement->GetPseudoElementType());
computedValues =
- Servo_ResolveStyle(aPseudoElement,
- mRawSet.get(),
- ServoTraversalFlags::Empty).Consume();
+ Servo_ResolveStyle(aPseudoElement, mRawSet.get()).Consume();
} else {
bool cacheable =
!nsCSSPseudoElements::IsEagerlyCascadedInServo(aType) && aParentContext;
computedValues =
cacheable ? aParentContext->GetCachedLazyPseudoStyle(aType) : nullptr;
if (!computedValues) {
computedValues = Servo_ResolvePseudoStyle(aOriginatingElement,
@@ -1148,23 +1146,21 @@ UpdateBodyTextColorIfNeeded(
// NOTE(emilio): We do the ComputedData() dance to avoid triggering the
// IsInServoTraversal() assertion in StyleColor(), which seems useful enough
// in the general case, I guess...
aPresContext.SetBodyTextColor(
aStyleContext.ComputedData()->GetStyleColor()->mColor);
}
already_AddRefed<ServoStyleContext>
-ServoStyleSet::ResolveServoStyle(Element* aElement, ServoTraversalFlags aFlags)
+ServoStyleSet::ResolveServoStyle(Element* aElement)
{
UpdateStylistIfNeeded();
RefPtr<ServoStyleContext> result =
- Servo_ResolveStyle(aElement,
- mRawSet.get(),
- aFlags).Consume();
+ Servo_ResolveStyle(aElement, mRawSet.get()).Consume();
UpdateBodyTextColorIfNeeded(*aElement, *result, *mPresContext);
return result.forget();
}
void
ServoStyleSet::ClearNonInheritingStyleContexts()
{
for (RefPtr<ServoStyleContext>& ptr : mNonInheritingStyleContexts) {
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -344,18 +344,17 @@ public:
void CompatibilityModeChanged();
/**
* Resolve style for the given element, and return it as a
* ServoStyleContext.
*
* FIXME(emilio): Is there a point in this after bug 1367904?
*/
- already_AddRefed<ServoStyleContext>
- ResolveServoStyle(dom::Element* aElement, ServoTraversalFlags aFlags);
+ already_AddRefed<ServoStyleContext> ResolveServoStyle(dom::Element* aElement);
bool GetKeyframesForName(const nsString& aName,
const nsTimingFunction& aTimingFunction,
nsTArray<Keyframe>& aKeyframes);
nsTArray<ComputedKeyframeValues>
GetComputedKeyframeValuesFor(const nsTArray<Keyframe>& aKeyframes,
dom::Element* aElement,
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -491,16 +491,31 @@ pub trait TElement : Eq + PartialEq + De
if self.has_snapshot() && !self.handled_snapshot() {
return false;
}
data.has_styles() && !data.restyle.hint.has_non_animation_invalidations()
}
+ /// Returns whether the element's styles are up-to-date after traversal
+ /// (i.e. in post traversal).
+ fn has_current_styles(&self, data: &ElementData) -> bool {
+ if self.has_snapshot() && !self.handled_snapshot() {
+ return false;
+ }
+
+ data.has_styles() &&
+ // TODO(hiro): When an animating element moved into subtree of
+ // contenteditable element, there remains animation restyle hints in
+ // post traversal. It's generally harmless since the hints will be
+ // processed in a next styling but ideally it should be processed soon.
+ !data.restyle.hint.has_non_animation_invalidations()
+ }
+
/// Flags an element and its ancestors with a given `DescendantsBit`.
///
/// TODO(emilio): We call this conservatively from restyle_element_internal
/// because we never flag unstyled stuff. A different setup for this may be
/// a bit cleaner, but it's probably not worth to invest on it right now
/// unless necessary.
unsafe fn note_descendants<B: DescendantsBit<Self>>(&self);
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2770,77 +2770,53 @@ pub extern "C" fn Servo_CSSSupports(cond
pub extern "C" fn Servo_NoteExplicitHints(element: RawGeckoElementBorrowed,
restyle_hint: nsRestyleHint,
change_hint: nsChangeHint) {
GeckoElement(element).note_explicit_hints(restyle_hint, change_hint);
}
#[no_mangle]
pub extern "C" fn Servo_TakeChangeHint(element: RawGeckoElementBorrowed,
- raw_flags: ServoTraversalFlags,
was_restyled: *mut bool) -> nsChangeHint
{
- let flags = TraversalFlags::from_bits_truncate(raw_flags);
let mut was_restyled = unsafe { was_restyled.as_mut().unwrap() };
let element = GeckoElement(element);
let damage = match element.mutate_data() {
Some(mut data) => {
*was_restyled = data.restyle.is_restyle();
let damage = data.restyle.damage;
- if flags.for_animation_only() {
- if !*was_restyled {
- // Don't touch elements if the element was not restyled
- // in throttled animation flush.
- debug!("Skip post traversal for throttled animation flush {:?} restyle={:?}",
- element, data.restyle);
- return nsChangeHint(0);
- }
- // In the case where we call this function for post traversal for
- // flusing throttled animations (i.e. without normal restyle
- // traversal), we need to preserve restyle hints for normal restyle
- // traversal. Restyle hints for animations have been already
- // removed during animation-only traversal.
- debug_assert!(!data.restyle.hint.has_animation_hint(),
- "Animation restyle hints should have been already removed");
- data.clear_restyle_flags_and_damage();
- } else {
- data.clear_restyle_state();
- }
+ data.clear_restyle_state();
damage
}
None => {
warn!("Trying to get change hint from unstyled element");
*was_restyled = false;
GeckoRestyleDamage::empty()
}
};
debug!("Servo_TakeChangeHint: {:?}, damage={:?}", element, damage);
damage.as_change_hint()
}
#[no_mangle]
pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed,
- _raw_data: RawServoStyleSetBorrowed,
- raw_flags: ServoTraversalFlags)
+ _raw_data: RawServoStyleSetBorrowed)
-> ServoStyleContextStrong
{
- let flags = TraversalFlags::from_bits_truncate(raw_flags);
let element = GeckoElement(element);
debug!("Servo_ResolveStyle: {:?}", element);
let data =
element.borrow_data().expect("Resolving style on unstyled element");
// TODO(emilio): Downgrade to debug assertions when close to release.
assert!(data.has_styles(), "Resolving style on unstyled element");
- // In the case where we process for throttled animation, there remaings
- // restyle hints other than animation hints.
- debug_assert!(element.has_current_styles_for_traversal(&*data, flags),
+ debug_assert!(element.has_current_styles(&*data),
"Resolving style on {:?} without current styles: {:?}", element, data);
data.styles.primary().clone().into()
}
#[no_mangle]
pub extern "C" fn Servo_ResolveStyleAllowStale(element: RawGeckoElementBorrowed)
-> ServoStyleContextStrong
{