Bug 1330190 - Part 5: Get style context without all of animation data for base styles. r=birtles
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Mon, 16 Jan 2017 17:41:24 +0900
changeset 374541 a2532fa6577d85c66c5cd6bea57c48db9a2a1c75
parent 374540 0a9984a0cc54c20cb54b8e07c9a84aa72b83e501
child 374542 8f52299c4c6366e7d1edfdd4f93aa3f7cfc4eb76
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1330190, 1325193
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
dom/animation/test/crashtests/1330190-1.html
dom/animation/test/crashtests/1330190-2.html
dom/animation/test/crashtests/crashtests.list
--- 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