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 357565 a2532fa6577d85c66c5cd6bea57c48db9a2a1c75
parent 357564 0a9984a0cc54c20cb54b8e07c9a84aa72b83e501
child 357566 8f52299c4c6366e7d1edfdd4f93aa3f7cfc4eb76
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1330190, 1325193
milestone53.0a1
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