Bug 485157: SMIL event timing, part 1 refactor added end time handling, r=dholbert, sr=roc, a=roc
authorBrian Birtles <birtles@gmail.com>
Wed, 18 Aug 2010 19:20:24 +0900
changeset 50802 d736c1a4ad2f
parent 50801 f4c97baa0e51
child 50803 f23bab9fa177
push id15162
push userbbirtles@mozilla.com
push date2010-08-18 10:24 +0000
treeherdermozilla-central@ca457b5758e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, roc, roc
bugs485157
milestone2.0b5pre
Bug 485157: SMIL event timing, part 1 refactor added end time handling, r=dholbert, sr=roc, a=roc
content/smil/nsSMILInstanceTime.cpp
content/smil/nsSMILInstanceTime.h
content/smil/nsSMILTimedElement.cpp
layout/reftests/svg/smil/restart/reftest.list
layout/reftests/svg/smil/restart/reset-2.svg
layout/reftests/svg/smil/restart/reset-3.svg
layout/reftests/svg/smil/restart/reset-4.svg
layout/reftests/svg/smil/restart/reset-5.svg
layout/reftests/svg/smil/restart/reset-6.svg
--- a/content/smil/nsSMILInstanceTime.cpp
+++ b/content/smil/nsSMILInstanceTime.cpp
@@ -99,17 +99,17 @@ nsSMILInstanceTime::nsSMILInstanceTime(c
       break;
   }
 
   SetBaseInterval(aBaseInterval);
 }
 
 nsSMILInstanceTime::~nsSMILInstanceTime()
 {
-  NS_ABORT_IF_FALSE(!mBaseInterval && !mCreator,
+  NS_ABORT_IF_FALSE(!mBaseInterval,
       "Destroying instance time without first calling Unlink()");
   NS_ABORT_IF_FALSE(mFixedEndpointRefCnt == 0,
       "Destroying instance time that is still used as the fixed endpoint of an "
       "interval");
 }
 
 void
 nsSMILInstanceTime::Unlink()
--- a/content/smil/nsSMILInstanceTime.h
+++ b/content/smil/nsSMILInstanceTime.h
@@ -135,18 +135,19 @@ protected:
   const nsSMILInstanceTime* GetBaseTime() const;
 
   nsSMILTimeValue mTime;
 
   // Internal flags used to represent the behaviour of different instance times
   enum {
     // Indicates that this instance time was generated by an event or a DOM
     // call. Such instance times require special handling when (i) the owning
-    // element is reset, and (ii) when a backwards seek is performed and the
-    // timing model is reconstructed.
+    // element is reset, (ii) when they are to be added as a new end instance
+    // times (as per SMIL's event sensitivity contraints), and (iii) when
+    // a backwards seek is performed and the timing model is reconstructed.
     kDynamic = 1,
 
     // Indicates that this instance time is referred to by an
     // nsSMILTimeValueSpec and as such may be updated. Such instance time should
     // not be filtered out by the nsSMILTimedElement even if they appear to be
     // in the past as they may be updated to a future time.
     kMayUpdate = 2,
 
--- a/content/smil/nsSMILTimedElement.cpp
+++ b/content/smil/nsSMILTimedElement.cpp
@@ -338,16 +338,29 @@ nsSMILTimedElement::GetStartTime() const
 // nsSMILTimedElement
 
 void
 nsSMILTimedElement::AddInstanceTime(nsSMILInstanceTime* aInstanceTime,
                                     PRBool aIsBegin)
 {
   NS_ABORT_IF_FALSE(aInstanceTime, "Attempting to add null instance time");
 
+  // Event-sensitivity: If an element is not active (but the parent time
+  // container is), then events are only handled for begin specifications.
+  if (mElementState != STATE_ACTIVE && !aIsBegin &&
+      aInstanceTime->IsDynamic())
+  {
+    // No need to call Unlink here--dynamic instance times shouldn't be linked
+    // to anything that's going to miss them
+    NS_ABORT_IF_FALSE(!aInstanceTime->GetBaseInterval(),
+        "Dynamic instance time has a base interval--we probably need to unlink"
+        " it if we're not going to use it");
+    return;
+  }
+
   aInstanceTime->SetSerial(++mInstanceSerialIndex);
   InstanceTimeList& instanceList = aIsBegin ? mBeginInstances : mEndInstances;
   nsRefPtr<nsSMILInstanceTime>* inserted =
     instanceList.InsertElementSorted(aInstanceTime, InstanceTimeComparator());
   if (!inserted) {
     NS_WARNING("Insufficient memory to insert instance time");
     return;
   }
--- a/layout/reftests/svg/smil/restart/reftest.list
+++ b/layout/reftests/svg/smil/restart/reftest.list
@@ -1,8 +1,8 @@
 # Tests for restart behaviour
 == reset-1.svg reset-1-ref.svg
 == reset-2.svg green-box-ref.svg
 == reset-3.svg green-box-ref.svg
 == reset-4.svg green-box-ref.svg
-== reset-5.svg green-box-ref.svg
+# reset-5.svg is no longer valid and has been removed
 == reset-6.svg green-box-ref.svg
 == reset-7.svg green-box-ref.svg
--- a/layout/reftests/svg/smil/restart/reset-2.svg
+++ b/layout/reftests/svg/smil/restart/reset-2.svg
@@ -2,31 +2,32 @@
   We want to test that reset behaviour is actually applied when an animation
   restarts and not before.
 
   Therefore we build up the following graph:
 
     |..|  |..|
     1  2  3  4
 
-  But at t=2.5s we add an end instance at t=3.5s. This should be cleared when we
-  restart at t=3s and hence the animation should still be playing after t=3.5s.
+  But at t=2.5s we add a begin instance at t=3.9s. This should be cleared when
+  we restart at t=3s and hence the animation should no longer be playing at
+  t=4s.
  -->
 <svg xmlns="http://www.w3.org/2000/svg"
   xmlns:xlink="http://www.w3.org/1999/xlink"
   class="reftest-wait"
   onload="addInstanceTimes()">
   <script type="text/ecmascript"><![CDATA[
     function addInstanceTimes() {
       var svg = document.documentElement;
       svg.pauseAnimations();
       svg.setCurrentTime(2.5);
       var anim = document.getElementById('anim');
-      anim.endElementAt(1);
-      setTimeAndSnapshot(3.7, true);
+      anim.beginElementAt(1.4);
+      setTimeAndSnapshot(4.0, true);
     }
   ]]></script>
   <script xlink:href="../smil-util.js" type="text/javascript"/>
-  <rect width="100" height="100" fill="red">
+  <rect width="100" height="100" fill="green">
     <set attributeName="fill" attributeType="CSS"
-      to="green" begin="1s; 3s" end="2s; 4s" dur="1s" fill="remove" id="anim"/>
+      to="red" begin="1s; 3s" dur="1s" fill="remove" id="anim"/>
   </rect>
 </svg>
--- a/layout/reftests/svg/smil/restart/reset-3.svg
+++ b/layout/reftests/svg/smil/restart/reset-3.svg
@@ -8,32 +8,32 @@
   In this test we ensure that times are NOT cleared upon starting the first
   interval.
 
   Therefore we build up the following graph:
 
     |..|
     1  2
 
-  But at t=0.5s we add an end instance at t=1.5s. This should NOT be cleared
-  when we start at t=1s and hence the animation should NOT still be playing
-  after t=1.5s.
+  But at t=0.5s we add a begin instance at t=1.5s. This should NOT be cleared
+  when we start at t=1s and hence the animation should STILL be playing
+  at t=2.0s.
  -->
 <svg xmlns="http://www.w3.org/2000/svg"
   xmlns:xlink="http://www.w3.org/1999/xlink"
   class="reftest-wait"
   onload="addInstanceTimes()">
   <script type="text/ecmascript"><![CDATA[
     function addInstanceTimes() {
       var svg = document.documentElement;
       svg.pauseAnimations();
       svg.setCurrentTime(0.5);
       var anim = document.getElementById('anim');
-      anim.endElementAt(1);
-      setTimeAndSnapshot(1.5, true);
+      anim.beginElementAt(1);
+      setTimeAndSnapshot(2.0, true);
     }
   ]]></script>
   <script xlink:href="../smil-util.js" type="text/javascript"/>
-  <rect width="100" height="100" fill="green">
+  <rect width="100" height="100" fill="red">
     <set attributeName="fill" attributeType="CSS"
-      to="red" begin="1s" end="2s" dur="1s" fill="remove" id="anim"/>
+      to="green" begin="1s" dur="1s" fill="remove" id="anim"/>
   </rect>
 </svg>
--- a/layout/reftests/svg/smil/restart/reset-4.svg
+++ b/layout/reftests/svg/smil/restart/reset-4.svg
@@ -8,20 +8,20 @@
 <svg xmlns="http://www.w3.org/2000/svg"
   xmlns:xlink="http://www.w3.org/1999/xlink"
   class="reftest-wait"
   onload="addInstanceTimes()">
   <script type="text/ecmascript"><![CDATA[
     function addInstanceTimes() {
       var svg = document.documentElement;
       svg.pauseAnimations();
-      svg.setCurrentTime(0.5);
+      svg.setCurrentTime(1.0);
       var anim = document.getElementById('anim');
-      anim.endElementAt(1);
+      anim.endElementAt(0.5);
       setTimeAndSnapshot(1.5, true);
     }
   ]]></script>
   <script xlink:href="../smil-util.js" type="text/javascript"/>
   <rect width="100" height="100" fill="green">
     <set attributeName="fill" attributeType="CSS"
-      to="red" begin="1s" end="2s" dur="1s" fill="remove" id="anim"/>
+      to="red" begin="1s" dur="1s" fill="remove" id="anim"/>
   </rect>
 </svg>
deleted file mode 100644
--- a/layout/reftests/svg/smil/restart/reset-5.svg
+++ /dev/null
@@ -1,26 +0,0 @@
-<!--
-  A variation on reset-4.svg. If the added end instance is before the current
-  interval we should not delete it but just ignore it and use the active
-  duration to calculate the end of the interval. This is consistent with the
-  operation of the SMIL pseudocode for getNextInterval when we have end events.
- -->
-<svg xmlns="http://www.w3.org/2000/svg"
-  xmlns:xlink="http://www.w3.org/1999/xlink"
-  class="reftest-wait"
-  onload="addInstanceTimes()">
-  <script type="text/ecmascript"><![CDATA[
-    function addInstanceTimes() {
-      var svg = document.documentElement;
-      svg.pauseAnimations();
-      svg.setCurrentTime(0.5);
-      var anim = document.getElementById('anim');
-      anim.endElementAt(0);
-      setTimeAndSnapshot(1.5, true);
-    }
-  ]]></script>
-  <script xlink:href="../smil-util.js" type="text/javascript"/>
-  <rect width="100" height="100" fill="red">
-    <set attributeName="fill" attributeType="CSS"
-      to="green" begin="1s" end="2s" dur="1s" fill="remove" id="anim"/>
-  </rect>
-</svg>
--- a/layout/reftests/svg/smil/restart/reset-6.svg
+++ b/layout/reftests/svg/smil/restart/reset-6.svg
@@ -22,17 +22,17 @@
     anim.endElementAt(2);
 
   potentially giving us:
 
     |...|  | |
       ^  
     1   2  3 3.5
 
-  At t=2s we'll go to look for the next interval and construct one from 3s-4s.
+  At t=2s we'll go to look for the next interval and construct one from 3s-3.5s.
   We should apply restart behaviour at t=3s meaning we'll reset instance times
   generated by DOM calls in the past however we'll keep the begin instance time
   at 3s since it defines the beginning of the (now) current interval.  Sticking
   to the letter of the spec quoted above however, we'll end up clearing the end
   instance at 3.5s. Yet in this case we should use the active end (t=4s) since
   there's no end attribute specified.
  -->
 <svg xmlns="http://www.w3.org/2000/svg"
@@ -41,17 +41,17 @@
   onload="addInstanceTimes()">
   <script type="text/ecmascript"><![CDATA[
     function addInstanceTimes() {
       var svg = document.documentElement;
       svg.pauseAnimations();
       svg.setCurrentTime(1.5);
       var anim = document.getElementById('anim');
       anim.beginElementAt(1.5);
-      anim.endElementAt(2.5);
+      anim.endElementAt(2);
       setTimeAndSnapshot(3.7, true);
     }
   ]]></script>
   <script xlink:href="../smil-util.js" type="text/javascript"/>
   <rect width="100" height="100" fill="red">
     <set attributeName="fill" attributeType="CSS"
       to="green" begin="1s" dur="1s" fill="remove" id="anim"/>
   </rect>