Bug 1322970 - Use primary frame for checking throttle instead of using style context. r=dholbert,hiro a=gchang
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Thu, 02 Feb 2017 15:23:22 +0900
changeset 376016 17b27168189deae84a92173000866595c5e81483
parent 376015 8ba4f117d820b2bdcc4118c70b2134b4f3de23d5
child 376017 73aac08829e835e6707b30fb6f0df3e2e0fe0e5d
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)
reviewersdholbert, hiro, gchang
bugs1322970
milestone53.0a2
Bug 1322970 - Use primary frame for checking throttle instead of using style context. r=dholbert,hiro a=gchang This changeset will skip finding the first SMIL animation function to affect the sandwich from multiple functions if element hasn't primary frame when composing attributes. This mean that target element's animation don't need to animate in the following cases. - ancestor elements has display:none attribute. - target element have display:none attribute. - ancestor element's tag produces a non-rendering subtree, by definition (like <desc>). MozReview-Commit-ID: 253qTpBLc8L
dom/smil/nsSMILCompositor.cpp
dom/svg/test/test_SVGxxxList.xhtml
layout/reftests/svg/smil/anim-change-display-block-for-dynamically-appended-elem.html
layout/reftests/svg/smil/anim-change-display-none-for-ancestor-elem.html
layout/reftests/svg/smil/anim-change-display-none-for-dynamically-appended-elem.html
layout/reftests/svg/smil/anim-change-display-none-for-target-elem.html
layout/reftests/svg/smil/anim-standard-ref.html
layout/reftests/svg/smil/lime.html
layout/reftests/svg/smil/reftest.list
--- a/dom/smil/nsSMILCompositor.cpp
+++ b/dom/smil/nsSMILCompositor.cpp
@@ -1,18 +1,19 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsSMILCompositor.h"
-#include "nsSMILCSSProperty.h"
+
 #include "nsCSSProps.h"
 #include "nsHashKeys.h"
+#include "nsSMILCSSProperty.h"
 
 // PLDHashEntryHdr methods
 bool
 nsSMILCompositor::KeyEquals(KeyTypePointer aKey) const
 {
   return aKey && aKey->Equals(mKey);
 }
 
@@ -138,24 +139,27 @@ nsSMILCompositor::CreateSMILAttr()
                                           mKey.mAttributeName);
   }
   return nullptr;
 }
 
 uint32_t
 nsSMILCompositor::GetFirstFuncToAffectSandwich()
 {
-  // canThrottle is true when attributeName is not 'display' and
-  // the element or subtree is display:none
-  RefPtr<nsStyleContext> styleContext =
-    nsComputedDOMStyle::GetStyleContextForElementNoFlush(mKey.mElement,
-                                                         nullptr, nullptr);
+  // For performance reasons, we throttle most animations on elements in
+  // display:none subtrees. (We can't throttle animations that target the
+  // "display" property itself, though -- if we did, display:none elements
+  // could never be dynamically displayed via animations.)
+  // To determine whether we're in a display:none subtree, we will check the
+  // element's primary frame since element in display:none subtree doesn't have
+  // a primary frame. Before this process, we will construct frame when we
+  // append an element to subtree. So we will not need to worry about pending
+  // frame construction in this step.
   bool canThrottle = mKey.mAttributeName != nsGkAtoms::display &&
-                     styleContext &&
-                     styleContext->IsInDisplayNoneSubtree();
+                     !mKey.mElement->GetPrimaryFrame();
 
   uint32_t i;
   for (i = mAnimationFunctions.Length(); i > 0; --i) {
     nsSMILAnimationFunction* curAnimFunc = mAnimationFunctions[i-1];
     // In the following, the lack of short-circuit behavior of |= means that we
     // will ALWAYS run UpdateCachedTarget (even if mForceCompositing is true)
     // but only call HasChanged and WasSkippedInPrevSample if necessary.  This
     // is important since we need UpdateCachedTarget to run in order to detect
--- a/dom/svg/test/test_SVGxxxList.xhtml
+++ b/dom/svg/test/test_SVGxxxList.xhtml
@@ -10,23 +10,23 @@ https://bugzilla.mozilla.org/show_bug.cg
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=515116">Mozilla Bug 515116</a>
 <p id="display"></p>
 <div id="content">
 <svg id="svg" xmlns="http://www.w3.org/2000/svg" width="100" height="100"
      onload="this.pauseAnimations();">
-  <desc>
+  <defs>
     <filter>
       <feComponentTransfer>
         <feFuncR id="feFuncR" type="table"/>
       </feComponentTransfer>
     </filter>
-  </desc>
+  </defs>
   <text id="text">text</text>
   <path id="path"/>
   <polyline id="polyline"/>
   <g id="g"/>
 </svg>
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-change-display-block-for-dynamically-appended-elem.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <head>
+    <title>Test dynamically-appended animation in a subtree that dynamically became 'display:none'</title>
+  </head>
+  <body style="background-color: lime;">
+    <div id="target" style="display: none;">
+      <svg>
+        <rect width="100" height="100" fill="brown" id="rect">
+        </rect>
+      </svg>
+    </div>
+    <script>
+      document.addEventListener('MozReftestInvalidate', function() {
+        var svg     = document.getElementsByTagName("svg")[0];
+        svg.pauseAnimations();  // pause svg animation.
+
+        var target  = document.getElementById("target");
+        var rect    = document.getElementById("rect");
+        var animate = document.createElementNS('http://www.w3.org/2000/svg',
+                                               'animate');
+        animate.setAttribute('attributeName', 'fill');
+        animate.setAttribute('from', 'blue');
+        animate.setAttribute('to', 'red');
+        animate.setAttribute('dur', '100s');
+        rect.appendChild(animate);
+
+        requestAnimationFrame(function(time) {
+          target.style.display = 'block';
+          requestAnimationFrame(function(time) {
+            document.documentElement.removeAttribute("class");
+          });
+        });
+      });
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-change-display-none-for-ancestor-elem.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <head>
+    <title>Test animation in a subtree that dynamically becames 'display:none'</title>
+  </head>
+  <body style="background-color: lime;">
+    <div id="target">
+      <svg>
+        <rect width="100%" height="100%" fill="blue">
+          <animate attributeName="fill" from="brown" to="red" dur="100s"/>
+        </rect>
+      </svg>
+    </div>
+    <script>
+      document.addEventListener('MozReftestInvalidate', function() {
+        var target = document.getElementById("target");
+        target.style.display = "none";
+
+        requestAnimationFrame(function(time) {
+          document.documentElement.removeAttribute("class");
+        });
+      });
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-change-display-none-for-dynamically-appended-elem.html
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <head>
+    <title>Test dynamically-appended animation on an element that dynamically becomes 'display:none'</title>
+  </head>
+  <body style="background-color: lime;">
+    <div>
+      <svg>
+        <rect width="100" height="100" fill="brown" id="rect">
+        </rect>
+      </svg>
+    </div>
+    <script>
+      document.addEventListener('MozReftestInvalidate', function() {
+        var rect    = document.getElementById("rect");
+        var animate = document.createElementNS('http://www.w3.org/2000/svg',
+                                               'animate');
+        animate.setAttribute('attributeName', 'fill');
+        animate.setAttribute('from', 'blue');
+        animate.setAttribute('to', 'red');
+        animate.setAttribute('dur', '100s');
+        rect.appendChild(animate);
+
+        requestAnimationFrame(function(time) {
+          rect.style.display = 'none';
+          requestAnimationFrame(function(time) {
+            document.documentElement.removeAttribute("class");
+          });
+        });
+      });
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-change-display-none-for-target-elem.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <head>
+    <title>Test animation on an element that dynamically becomes 'display:none'</title>
+  </head>
+  <body style="background-color: lime;">
+    <div>
+      <svg>
+        <rect width="100%" height="100%" fill="blue" id="rect">
+          <animate attributeName="fill" from="brown" to="red" dur="100s"/>
+        </rect>
+      </svg>
+    </div>
+    <script>
+      document.addEventListener('MozReftestInvalidate', function() {
+        var rect = document.getElementById("rect");
+        rect.style.display = "none";
+
+        requestAnimationFrame(function(time) {
+          document.documentElement.removeAttribute("class");
+        });
+      });
+    </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-standard-ref.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Testcase reference file for animated pass condition(HTML)</title>
+  </head>
+  <body style="background-color: lime;">
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect width="100" height="100" fill="blue"/>
+</svg>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/lime.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Testcase reference file for generic pass condition(HTML)</title>
+  </head>
+  <body style="background-color: lime;">
+  </body>
+</html>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -275,9 +275,15 @@ fuzzy-if(cocoaWidget&&layersGPUAccelerat
 == anim-defs-gradient-attribute.svg lime.svg
 == anim-defs-fill.svg lime.svg
 == anim-defs-width.svg lime.svg
 
 # Test animation that changes 'display' attribute
 == anim-display.svg lime.svg
 == anim-display-in-g-element.svg lime.svg
 
+# Test animation that change 'display' style value to 'none'
+== anim-change-display-none-for-ancestor-elem.html lime.html
+== anim-change-display-none-for-target-elem.html lime.html
+== anim-change-display-none-for-dynamically-appended-elem.html lime.html
+== anim-change-display-block-for-dynamically-appended-elem.html anim-standard-ref.html
+
 pref(layout.css.clip-path-shapes.enabled,true) fuzzy(63,146) == anim-clipPath-viewBox.svg anim-clipPath-viewBox-ref.svg