Bug 1330190 - Part 5: Get style context without all of animation data for base styles. r?birtles
Test case, 1330190-2.html, is another variant of 1325193-1.html. It's for
animation on a pseudo element, causes timeout without part 3 patch.
MozReview-Commit-ID: KX6FE8mkZY2
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -358,17 +358,18 @@ KeyframeEffectReadOnly::GetUnderlyingSty
// If we have already composed style for the property, we use the style
// as the underlying style.
DebugOnly<bool> success = aAnimationRule->GetValue(aProperty, result);
MOZ_ASSERT(success, "AnimValuesStyleRule::GetValue should not fail");
} else {
// If we are composing with composite operation that is not 'replace'
// and we have not composed style for the property yet, we have to get
// the base style for the property.
- RefPtr<nsStyleContext> styleContext = GetTargetStyleContext();
+ RefPtr<nsStyleContext> styleContext =
+ GetTargetStyleContextWithoutAnimation();
result = EffectCompositor::GetBaseStyle(aProperty,
styleContext,
*mTarget->mElement,
mTarget->mPseudoType);
MOZ_ASSERT(!result.IsNull(), "The base style should be set");
SetNeedsBaseStyle(aProperty);
}
@@ -438,17 +439,17 @@ KeyframeEffectReadOnly::EnsureBaseStyles
for (const AnimationPropertySegment& segment : property.mSegments) {
if (segment.mFromComposite == dom::CompositeOperation::Replace &&
segment.mToComposite == dom::CompositeOperation::Replace) {
continue;
}
if (!styleContext) {
- styleContext = GetTargetStyleContext();
+ styleContext = GetTargetStyleContextWithoutAnimation();
}
MOZ_RELEASE_ASSERT(styleContext);
Unused << EffectCompositor::GetBaseStyle(property.mProperty,
styleContext,
*mTarget->mElement,
mTarget->mPseudoType);
// Make this property as needing a base style so that we send the (now
@@ -895,32 +896,52 @@ KeyframeEffectReadOnly::RequestRestyle(
nsPresContext* presContext = GetPresContext();
if (presContext && mTarget && mAnimation) {
presContext->EffectCompositor()->
RequestRestyle(mTarget->mElement, mTarget->mPseudoType,
aRestyleType, mAnimation->CascadeLevel());
}
}
+template<KeyframeEffectReadOnly::AnimationStyle aAnimationStyle>
already_AddRefed<nsStyleContext>
-KeyframeEffectReadOnly::GetTargetStyleContext()
+KeyframeEffectReadOnly::DoGetTargetStyleContext()
{
nsIPresShell* shell = GetPresShell();
if (!shell) {
return nullptr;
}
MOZ_ASSERT(mTarget,
"Should only have a presshell when we have a target element");
nsIAtom* pseudo = mTarget->mPseudoType < CSSPseudoElementType::Count
? nsCSSPseudoElements::GetPseudoAtom(mTarget->mPseudoType)
: nullptr;
- return nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement,
- pseudo, shell);
+
+ if (aAnimationStyle == AnimationStyle::Include) {
+ return nsComputedDOMStyle::GetStyleContextForElement(mTarget->mElement,
+ pseudo,
+ shell);
+ }
+
+ return nsComputedDOMStyle::GetStyleContextForElementWithoutAnimation(
+ mTarget->mElement, pseudo, shell);
+}
+
+already_AddRefed<nsStyleContext>
+KeyframeEffectReadOnly::GetTargetStyleContext()
+{
+ return DoGetTargetStyleContext<AnimationStyle::Include>();
+}
+
+already_AddRefed<nsStyleContext>
+KeyframeEffectReadOnly::GetTargetStyleContextWithoutAnimation()
+{
+ return DoGetTargetStyleContext<AnimationStyle::Skip>();
}
#ifdef DEBUG
void
DumpAnimationProperties(nsTArray<AnimationProperty>& aAnimationProperties)
{
for (auto& p : aAnimationProperties) {
printf("%s\n", nsCSSProps::GetStringValue(p.mProperty).get());
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -404,18 +404,29 @@ protected:
// have changed, or when the target frame might have changed.
void MaybeUpdateFrameForCompositor();
// Looks up the style context associated with the target element, if any.
// We need to be careful to *not* call this when we are updating the style
// context. That's because calling GetStyleContextForElement when we are in
// the process of building a style context may trigger various forms of
// infinite recursion.
+ already_AddRefed<nsStyleContext> GetTargetStyleContext();
+
+ // Similar to the above but ignoring animation rules. We use this to get base
+ // styles (which don't include animation rules).
already_AddRefed<nsStyleContext>
- GetTargetStyleContext();
+ GetTargetStyleContextWithoutAnimation();
+
+ enum AnimationStyle {
+ Skip,
+ Include
+ };
+ template<AnimationStyle aAnimationStyle>
+ already_AddRefed<nsStyleContext> DoGetTargetStyleContext();
// A wrapper for marking cascade update according to the current
// target and its effectSet.
void MarkCascadeNeedsUpdate();
// Composites |aValueToComposite| using |aCompositeOperation| onto the value
// for |aProperty| in |aAnimationRule|, or, if there is no suitable value in
// |aAnimationRule|, uses the base value for the property recorded on the
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1330190-1.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<span id=a />
+<script>
+addEventListener("DOMContentLoaded", function(){
+ a.animate([{"left": "38%"}], 100);
+ a.appendChild(document.createElement("div"));
+ document.documentElement.classList.remove("reftest-wait");
+});
+</script>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1330190-2.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<style>
+@keyframes anim {
+}
+
+#o_0:before {
+ animation: anim 10s;
+}
+</style>
+<meta charset="UTF-8">
+<script>
+function boom(){
+ function getPseudoElement() {
+ var anim = document.getAnimations()[0];
+ anim.cancel();
+ return anim.effect.target;
+ }
+
+ var target = getPseudoElement();
+ target.animate([{"perspective": "262ex"}, {}], {duration: 1070, iterationStart: 68, iterationComposite: "accumulate"});
+ target.animate([{"color": "white", "flex": "inherit"}, {"color": "red", "flex": "66% "}], 3439);
+ target.animate([{"font": "icon"}], 1849);
+ setTimeout(function() {
+ document.documentElement.classList.remove("reftest-wait");
+ }, 500);
+}
+document.addEventListener("DOMContentLoaded", boom);
+</script>
+</head>
+<body>
+<div id=o_0></div>
+</body>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -12,10 +12,12 @@ asserts-if(stylo,5) pref(dom.animations-
asserts-if(stylo,31) pref(dom.animations-api.core.enabled,true) load 1277272-1.html # bug 1324694
asserts-if(stylo,2) pref(dom.animations-api.core.enabled,true) load 1290535-1.html # bug 1324690
pref(dom.animations-api.core.enabled,true) load 1304886-1.html
pref(dom.animations-api.core.enabled,true) load 1322382-1.html
skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-1.html # bug 1311257
skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-2.html # bug 1311257
asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-1.html # bug 1324690
asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-2.html # bug 1324690
+skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1325193-1.html # bug 1311257
+skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330190-1.html # bug 1311257
+skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330190-2.html # bug 1311257
skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330513-1.html # bug 1311257
-skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1325193-1.html # bug 1311257