From fdd11f4f75c0e3151ddb704316dd3eb98b458090 Mon Sep 17 00:00:00 2001
From: Hiroyuki Ikezoe <hikezoe@mozilla.com>
Date: Mon, 16 Jan 2017 17:41:24 +0900
Subject: [PATCH] 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

--HG--
extra : rebase_source : f5efbae0cc30036a511659eab9b29b5091df7539
---
 dom/animation/KeyframeEffectReadOnly.cpp      | 31 +++++++++++++---
 dom/animation/KeyframeEffectReadOnly.h        | 13 ++++++-
 dom/animation/test/crashtests/1330190-1.html  | 11 ++++++
 dom/animation/test/crashtests/1330190-2.html  | 35 +++++++++++++++++++
 dom/animation/test/crashtests/crashtests.list |  4 ++-
 5 files changed, 87 insertions(+), 7 deletions(-)
 create mode 100644 dom/animation/test/crashtests/1330190-1.html
 create mode 100644 dom/animation/test/crashtests/1330190-2.html

diff --git a/dom/animation/KeyframeEffectReadOnly.cpp b/dom/animation/KeyframeEffectReadOnly.cpp
index d413438d6ed2..ed8d6e9b0e0f 100644
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -363,7 +363,8 @@ KeyframeEffectReadOnly::GetUnderlyingStyle(
     // 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,
@@ -443,7 +444,7 @@ KeyframeEffectReadOnly::EnsureBaseStylesForCompositor(
       }
 
       if (!styleContext) {
-        styleContext = GetTargetStyleContext();
+        styleContext = GetTargetStyleContextWithoutAnimation();
       }
       MOZ_RELEASE_ASSERT(styleContext);
 
@@ -900,8 +901,9 @@ KeyframeEffectReadOnly::RequestRestyle(
   }
 }
 
+template<KeyframeEffectReadOnly::AnimationStyle aAnimationStyle>
 already_AddRefed<nsStyleContext>
-KeyframeEffectReadOnly::GetTargetStyleContext()
+KeyframeEffectReadOnly::DoGetTargetStyleContext()
 {
   nsIPresShell* shell = GetPresShell();
   if (!shell) {
@@ -914,8 +916,27 @@ KeyframeEffectReadOnly::GetTargetStyleContext()
   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
diff --git a/dom/animation/KeyframeEffectReadOnly.h b/dom/animation/KeyframeEffectReadOnly.h
index 9591c9ffd570..15cf8ca4c94e 100644
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -409,8 +409,19 @@ protected:
   // 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.
diff --git a/dom/animation/test/crashtests/1330190-1.html b/dom/animation/test/crashtests/1330190-1.html
new file mode 100644
index 000000000000..fa14e0f74134
--- /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>
diff --git a/dom/animation/test/crashtests/1330190-2.html b/dom/animation/test/crashtests/1330190-2.html
new file mode 100644
index 000000000000..2229529f2575
--- /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>
diff --git a/dom/animation/test/crashtests/crashtests.list b/dom/animation/test/crashtests/crashtests.list
index 430501d9342b..91eba9c33175 100644
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -17,5 +17,7 @@ skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-1.html #
 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 1330513-1.html # bug 1311257
 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