Bug 534325: Apply SMIL restart semantics at interval start times rather than end times, and allow duration to determine interval-ending when there's no end attribute. r=dholbert sr=roc

content/smil/nsSMILTimedElement.cpp

@@ -401,9 +401,22 @@ nsSMILTimedElement::DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly)
         if (mCurrentInterval.Begin()->Time() <= sampleTime) {
           mElementState = STATE_ACTIVE;
           mCurrentInterval.FreezeBegin();
+          if (mPrevInterval.IsSet()) {
+            Reset(); // Apply restart behaviour
+          }
           if (mClient) {
             mClient->Activate(mCurrentInterval.Begin()->Time().GetMillis());
           }
+          if (mPrevInterval.IsSet()) {
+            // The call to Reset() may mean that the end point of our current
+            // interval should be changed and so we should update the interval
+            // now. However, calling UpdateCurrentInterval could result in the
+            // interval getting deleted (perhaps through some web of syncbase
+            // dependencies) therefore we make updating the interval the last
+            // thing we do. There is no guarantee that mCurrentInterval.IsSet()
+            // is true after this.
+            UpdateCurrentInterval();
+          }
           stateChanged = PR_TRUE;
         }
       }
@@ -435,7 +448,6 @@ nsSMILTimedElement::DoSampleAt(nsSMILTime aContainerTime, PRBool aEndOnly)
           mCurrentInterval = newInterval;
           // We must update mPrevInterval before calling SampleFillValue
           SampleFillValue();
-          Reset();
           if (mElementState == STATE_WAITING) {
             NotifyNewInterval();
           }
@@ -480,17 +492,16 @@ nsSMILTimedElement::HandleContainerTimeChange()
 void
 nsSMILTimedElement::Reset()
 {
+  // SMIL 3.0 section 5.4.3, 'Resetting element state':
+  //   Any instance times associated with past Event-values, Repeat-values,
+  //   Accesskey-values or added via DOM method calls are removed from the
+  //   dependent begin and end instance times lists. In effect, all events and
+  //   DOM methods calls in the past are cleared. This does not apply to an
+  //   instance time that defines the begin of the current interval.
   PRInt32 count = mBeginInstances.Length();
-
   for (PRInt32 i = count - 1; i >= 0; --i) {
     nsSMILInstanceTime* instance = mBeginInstances[i].get();
     NS_ABORT_IF_FALSE(instance, "NULL instance in begin instances array");
-    // SMIL 3.0 section 5.4.3, 'Resetting element state':
-    //   Any instance times associated with past Event-values, Repeat-values,
-    //   Accesskey-values or added via DOM method calls are removed from the
-    //   dependent begin and end instance times lists. In effect, all events and
-    //   DOM methods calls in the past are cleared. This does not apply to an
-    //   instance time that defines the begin of the current interval.
     if (instance->ClearOnReset() &&
        (!mCurrentInterval.IsSet() || instance != mCurrentInterval.Begin())) {
       mBeginInstances.RemoveElementAt(i);
@@ -498,7 +509,6 @@ nsSMILTimedElement::Reset()
   }
 
   count = mEndInstances.Length();
-
   for (PRInt32 j = count - 1; j >= 0; --j) {
     nsSMILInstanceTime* instance = mEndInstances[j].get();
     NS_ABORT_IF_FALSE(instance, "NULL instance in end instances array");
@@ -602,7 +612,6 @@ nsSMILTimedElement::SetEndSpec(const nsAString& aEndSpec,
                                nsIContent* aContextNode)
 {
   // XXX When implementing events etc., don't forget to ensure
-  // When implementing events etc., don't forget to ensure
   // mEndHasEventConditions is set if the specification contains conditions that
   // describe event-values, repeat-values or accessKey-values.
   return SetBeginOrEndSpec(aEndSpec, aContextNode, PR_FALSE);
@@ -833,8 +842,7 @@ nsSMILTimedElement::UnsetFillMode()
   PRUint16 previousFillMode = mFillMode;
   mFillMode = FILL_REMOVE;
   if ((mElementState == STATE_WAITING || mElementState == STATE_POSTACTIVE) &&
-      previousFillMode == FILL_FREEZE &&
-      mClient)
+      previousFillMode == FILL_FREEZE && mClient && mPrevInterval.IsSet())
     mClient->Inactivate(PR_FALSE);
 }
 
@@ -1013,6 +1021,10 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
     beginAfter = aPrevInterval->End()->Time();
     prevIntervalWasZeroDur
       = aPrevInterval->End()->Time() == aPrevInterval->Begin()->Time();
+    if (aFixedBeginTime) {
+      prevIntervalWasZeroDur &= 
+        aPrevInterval->Begin()->Time() == aFixedBeginTime->Time();
+    }
   } else {
     beginAfter.SetMillis(LL_MININT);
   }
@@ -1021,6 +1033,7 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
   nsRefPtr<nsSMILInstanceTime> tempEnd;
 
   while (PR_TRUE) {
+    // Calculate begin time
     if (aFixedBeginTime) {
       if (aFixedBeginTime->Time() < beginAfter)
         return NS_ERROR_FAILURE;
@@ -1042,13 +1055,8 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
     NS_ABORT_IF_FALSE(tempBegin && tempBegin->Time() >= beginAfter,
         "Got a bad begin time while fetching next interval");
 
-    if (mEndSpecs.IsEmpty() && mEndInstances.IsEmpty()) {
-      nsSMILTimeValue activeEnd =
-        CalcActiveEnd(tempBegin->Time(), nsSMILTimeValue::Indefinite());
-      tempEnd = new nsSMILInstanceTime(activeEnd, nsnull);
-      if (!tempEnd)
-        return NS_ERROR_OUT_OF_MEMORY;
-    } else {
+    // Calculate end time
+    {
       PRInt32 endPos = 0;
       tempEnd = GetNextGreaterOrEqual(mEndInstances, tempBegin->Time(), endPos);
 
@@ -1059,15 +1067,21 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
         tempEnd = GetNextGreater(mEndInstances, tempBegin->Time(), endPos);
       }
 
-      if (!tempEnd && !mEndHasEventConditions && !mEndInstances.IsEmpty()) {
-        //
-        // This is a little counter-intuitive but according to SMILANIM, if
-        // all the ends are before the begin, we _don't_ just assume an
-        // infinite end, it's actually a bad interval. ASV however will just
-        // use an indefinite end.
-        //
-        return NS_ERROR_FAILURE;
-      }
+      // If all the ends are before the beginning we have a bad interval UNLESS:
+      // a) We have end events which leave the interval open-ended, OR
+      // b) We never had any end attribute to begin with (and hence we should
+      //    just use the active duration after allowing for the possibility of
+      //    an end instance provided by a DOM call)
+      // c) We have an end attribute but no end instances--this is a special
+      //    case that is needed for syncbase timing so that animations of the
+      //    following sort: <animate id="a" end="a.begin+1s" ... /> can be
+      //    resolved (see SVGT 1.2 Test Suite animate-elem-221-t.svg) by first
+      //    establishing an interval of unresolved duration.
+      PRBool openEndedIntervalOk = mEndHasEventConditions ||
+          mEndSpecs.IsEmpty() ||
+          mEndInstances.IsEmpty();
+      if (!tempEnd && !openEndedIntervalOk)
+        return NS_ERROR_FAILURE; // Bad interval
 
       nsSMILTimeValue intervalEnd = tempEnd
                                   ? tempEnd->Time() : nsSMILTimeValue();
@@ -1076,7 +1090,6 @@ nsSMILTimedElement::GetNextInterval(const nsSMILInterval* aPrevInterval,
       if (!tempEnd || intervalEnd != activeEnd) {
         tempEnd = new nsSMILInstanceTime(activeEnd, nsnull);
       }
-
       if (!tempEnd)
         return NS_ERROR_OUT_OF_MEMORY;
     }
@@ -1352,7 +1365,10 @@ nsSMILTimedElement::UpdateCurrentInterval(PRBool aForceChangeNotice)
     RegisterMilestone();
   } else {
     if (mElementState == STATE_ACTIVE && mClient) {
-      mClient->Inactivate(PR_FALSE); // Don't apply fill
+      // Only apply a fill if it was already being applied before the (now
+      // deleted) interval was created
+      PRBool applyFill = mPrevInterval.IsSet() && mFillMode == FILL_FREEZE;
+      mClient->Inactivate(applyFill);
     }
 
     if (mElementState == STATE_ACTIVE || mElementState == STATE_WAITING) {

layout/reftests/svg/smil/restart/green-box-ref.svg

@@ -0,0 +1,3 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+  <rect width="100" height="100" fill="green"/>
+</svg>

layout/reftests/svg/smil/restart/reftest.list

@@ -1,2 +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-6.svg green-box-ref.svg
+== reset-7.svg green-box-ref.svg

layout/reftests/svg/smil/restart/reset-2.svg

@@ -0,0 +1,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.
+ -->
+<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);
+    }
+  ]]></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; 3s" end="2s; 4s" dur="1s" fill="remove" id="anim"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/restart/reset-3.svg

@@ -0,0 +1,39 @@
+<!--
+  A variation on reset-2.svg.
+  
+  Perhaps surprisingly, restart behaviour only applies for the second interval
+  onwards. This is inconsistent but it's the spec
+  (see http://lists.w3.org/Archives/Public/www-smil/2009OctDec/0004.html)
+
+  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.
+ -->
+<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);
+    }
+  ]]></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"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/restart/reset-4.svg

@@ -0,0 +1,27 @@
+<!--
+  According to the SMIL pseudocode if no end attribute is specified the end of
+  an interval is just the active end. This, however, effectively makes
+  endElement useless on animations which have no end attribute specified. This
+  seems counter-intuitive, so this test checks that endElement still takes
+  effect on animation without an end attribute.
+ -->
+<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);
+    }
+  ]]></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"/>
+  </rect>
+</svg>

layout/reftests/svg/smil/restart/reset-5.svg

@@ -0,0 +1,26 @@
+<!--
+  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>

layout/reftests/svg/smil/restart/reset-6.svg

@@ -0,0 +1,58 @@
+<!--
+  SMIL3 5.4.3, "Resetting element state" has:
+
+    Any instance times associated with past Event-values, Repeat-values,
+    Accesskey-values or added via DOM method calls are removed from the
+    dependent begin and end instance times lists. In effect, all events and DOM
+    methods calls in the past are cleared. This does not apply to an instance
+    time that defines the begin of the current interval.
+
+  In this test we have the following scenario:
+
+    <set begin="1s" end="2s" dur="1s" ... />
+
+  giving us:
+
+    |...|
+    1   2
+
+  Then at t=1.5s we have the following DOM calls
+
+    anim.beginElementAt(1.5);
+    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.
+  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"
+  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(1.5);
+      var anim = document.getElementById('anim');
+      anim.beginElementAt(1.5);
+      anim.endElementAt(2.5);
+      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>
+</svg>

layout/reftests/svg/smil/restart/reset-7.svg

@@ -0,0 +1,25 @@
+<!--
+  A variation on reset-6.svg but this time since the animation has an end
+  specification the second interval SHOULD be deleted.
+ -->
+<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(1.5);
+      var anim = document.getElementById('anim');
+      anim.beginElementAt(1.5);
+      anim.endElementAt(2.5);
+      setTimeAndSnapshot(3.2, 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"/>
+  </rect>
+</svg>