Bug 523188: Allow for ClearAnimValue() (and SetSMILOverrideStyleRule()) to be called for animation targets that aren't in a document. r=birtles r=roc
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 20 Oct 2009 09:54:47 -0700
changeset 34033 1ebdaad9b24a5e08e6ebe5c01641b9dc0dc3576b
parent 34032 68490d9daf2309a55bfcdb3fc47cf5a664197efe
child 34034 6108fdf296de6cd5b9d695e7a6f01117a6c2a6ab
push idunknown
push userunknown
push dateunknown
reviewersbirtles, roc
bugs523188
milestone1.9.3a1pre
Bug 523188: Allow for ClearAnimValue() (and SetSMILOverrideStyleRule()) to be called for animation targets that aren't in a document. r=birtles r=roc
content/base/src/nsGenericElement.cpp
content/smil/crashtests/523188-1.svg
content/smil/crashtests/crashtests.list
content/smil/nsISMILAttr.h
layout/reftests/svg/smil/anim-remove-8css.svg
layout/reftests/svg/smil/anim-remove-8xml.svg
layout/reftests/svg/smil/reftest.list
testing/crashtest/crashtests.list
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -2969,23 +2969,26 @@ nsGenericElement::SetSMILOverrideStyleRu
 {
   nsGenericElement::nsDOMSlots *slots = GetDOMSlots();
   NS_ENSURE_TRUE(slots, NS_ERROR_OUT_OF_MEMORY);
 
   slots->mSMILOverrideStyleRule = aStyleRule;
 
   if (aNotify) {
     nsIDocument* doc = GetCurrentDoc();
-    NS_ABORT_IF_FALSE(doc, "Shouldn't be able to animate style on a node "
-                      "unless it's in a document...");
-    nsPresShellIterator iter(doc);
-    nsCOMPtr<nsIPresShell> shell;
-    while (shell = iter.GetNextShell()) {
-      nsPresContext* presContext = shell->GetPresContext();
-      presContext->SMILOverrideStyleChanged(this);
+    // Only need to notify PresContexts if we're in a document.  (We might not
+    // be in a document, if we're clearing animation effects on a target node
+    // that's been detached since the previous animation sample.)
+    if (doc) {
+      nsPresShellIterator iter(doc);
+      nsCOMPtr<nsIPresShell> shell;
+      while (shell = iter.GetNextShell()) {
+        nsPresContext* presContext = shell->GetPresContext();
+        presContext->SMILOverrideStyleChanged(this);
+      }
     }
   }
 
   return NS_OK;
 }
 #endif // MOZ_SMIL
 
 nsICSSStyleRule*
new file mode 100644
--- /dev/null
+++ b/content/smil/crashtests/523188-1.svg
@@ -0,0 +1,15 @@
+<?xml version="1.0"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     class="reftest-wait"
+     onload="setTimeout(removeNode, 0)">
+  <script>
+    function removeNode() {
+      var node = document.getElementById("myRect");
+      node.parentNode.removeChild(node);
+      document.documentElement.removeAttribute("class");
+    }
+  </script>
+  <rect id="myRect" x="20" y="20" height="50" width="50" stroke="blue">
+    <animate attributeName="stroke-width" from="1" to="9" begin="0s" dur="2s"/>
+  </rect>
+</svg>
new file mode 100644
--- /dev/null
+++ b/content/smil/crashtests/crashtests.list
@@ -0,0 +1,1 @@
+load 523188-1.svg
--- a/content/smil/nsISMILAttr.h
+++ b/content/smil/nsISMILAttr.h
@@ -79,16 +79,19 @@ public:
    *
    * @return an nsSMILValue object. returned_object.IsNull() will be true if an
    * error occurred.
    */
   virtual nsSMILValue GetBaseValue() const = 0;
 
   /**
    * Clears the animated value of this attribute.
+   *
+   * NOTE: The animation target is not guaranteed to be in a document when this
+   * method is called. (See bug 523188)
    */
   virtual void ClearAnimValue() = 0;
 
   /**
    * Sets the presentation value of this attribute.
    *
    * @param aValue  The value to set.
    * @return NS_OK on success or an error code if setting failed.
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-remove-8css.svg
@@ -0,0 +1,35 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     onload="go()"
+     class="reftest-wait">
+  <!-- In this test, we temporarily remove the animation target, detach
+       its <animate> node, and then reinsert the (now former) animation target.
+       We verify that animation effects are removed from the former target. -->
+  <script>
+    var parent;
+    var rect;
+    function go() {
+      // Seek animation before we start tweaking things, to make sure we've
+      // already started sampling it.
+      document.documentElement.setCurrentTime(0.5);
+
+      var anim = document.getElementById("anim");
+      rect = anim.parentNode;
+      parent = rect.parentNode;
+      parent.removeChild(rect);
+      rect.removeChild(anim);
+
+      setTimeout(reinsert, 50); // Allow time for a sample
+    }
+    function reinsert() {
+      parent.appendChild(rect);
+      document.documentElement.removeAttribute("class");
+    }
+  </script>
+  <script xlink:href="smil-util.js" type="text/javascript"/>
+  <rect x="15" y="15" width="200" height="200" fill="blue"
+        stroke="red" stroke-width="0">
+    <animate id="anim" attributeName="stroke-width"
+             begin="0s" dur="2s" from="25" to="50" fill="freeze"/>
+  </rect>
+</svg>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/anim-remove-8xml.svg
@@ -0,0 +1,34 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     onload="go()"
+     class="reftest-wait">
+  <!-- In this test, we temporarily remove the animation target, detach
+       its <animate> node, and then reinsert the (now former) animation target.
+       We verify that animation effects are removed from the former target. -->
+  <script>
+    var parent;
+    var rect;
+    function go() {
+      // Seek animation before we start tweaking things, to make sure we've
+      // already started sampling it.
+      document.documentElement.setCurrentTime(0.5);
+
+      var anim = document.getElementById("anim");
+      rect = anim.parentNode;
+      parent = rect.parentNode;
+      parent.removeChild(rect);
+      rect.removeChild(anim);
+
+      setTimeout(reinsert, 50); // Allow time for a sample
+    }
+    function reinsert() {
+      parent.appendChild(rect);
+      document.documentElement.removeAttribute("class");
+    }
+  </script>
+  <script xlink:href="smil-util.js" type="text/javascript"/>
+  <rect x="15" y="15" width="200" height="200" fill="blue">
+    <animate id="anim" attributeName="height"
+             begin="0s" dur="2s" from="25" to="50" fill="freeze"/>
+  </rect>
+</svg>
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -69,16 +69,18 @@ fails == anim-fillopacity-1xml.svg  anim
 
 == anim-remove-1.svg anim-standard-ref.svg
 == anim-remove-2.svg anim-standard-ref.svg
 == anim-remove-3.svg anim-standard-ref.svg
 == anim-remove-4.svg anim-standard-ref.svg
 == anim-remove-5.svg anim-standard-ref.svg
 == anim-remove-6.svg anim-standard-ref.svg
 == anim-remove-7.svg anim-standard-ref.svg
+== anim-remove-8css.svg anim-standard-ref.svg
+== anim-remove-8xml.svg anim-standard-ref.svg
 == anim-retarget-1.svg anim-standard-ref.svg
 == anim-retarget-2.svg anim-standard-ref.svg
 == anim-retarget-3.svg anim-standard-ref.svg
 == anim-retarget-4.svg anim-standard-ref.svg
 == anim-retarget-5.svg anim-standard-ref.svg
 == anim-retarget-6.svg anim-standard-ref.svg
 == anim-retarget-7.svg anim-standard-ref.svg
 == anim-retarget-8.svg anim-standard-ref.svg
--- a/testing/crashtest/crashtests.list
+++ b/testing/crashtest/crashtests.list
@@ -3,16 +3,17 @@
 
 include ../../testing/crashtest/sanity/crashtests.list
 
 include ../../content/base/crashtests/crashtests.list
 include ../../content/canvas/crashtests/crashtests.list
 include ../../content/events/crashtests/crashtests.list
 include ../../content/html/document/crashtests/crashtests.list
 include ../../content/html/content/crashtests/crashtests.list
+include ../../content/smil/crashtests/crashtests.list
 include ../../content/svg/content/src/crashtests/crashtests.list
 include ../../content/xml/content/crashtest/crashtests.list
 include ../../content/xml/document/crashtests/crashtests.list
 include ../../content/xbl/crashtests/crashtests.list
 include ../../content/xslt/crashtests/crashtests.list
 include ../../content/xul/content/crashtests/crashtests.list
 include ../../content/xul/document/crashtests/crashtests.list
 include ../../content/xul/templates/src/crashtests/crashtests.list