Bug 554202 - SVG SMIL: Fix crash due to infinite recursion negotiating cyclic dependencies. r=roc
new file mode 100644
--- /dev/null
+++ b/content/smil/crashtests/554202-1.svg
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<svg xmlns="http://www.w3.org/2000/svg">
+
+<bindings xmlns="http://www.mozilla.org/xbl">
+ <binding id="foo">
+ <content>
+ <animate xmlns="http://www.w3.org/2000/svg" begin="a.begin">
+ <children xmlns="http://www.mozilla.org/xbl"/>
+ </animate>
+ </content>
+ </binding>
+</bindings>
+
+<animate id="a" begin="b.begin; 3s"/>
+<animate id="b" begin="a.begin"/>
+
+<script type="text/javascript">
+
+function boom()
+{
+ document.documentElement.pauseAnimations();
+ document.documentElement.setCurrentTime(0);
+ document.getElementById('a').beginElementAt(1);
+ document.documentElement.style.MozBinding = 'url(#foo)';
+}
+
+window.addEventListener("load", boom, false);
+
+</script>
+
+</svg>
new file mode 100644
--- /dev/null
+++ b/content/smil/crashtests/554202-2.svg
@@ -0,0 +1,19 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+ onload="
+ document.documentElement.pauseAnimations();
+ document.documentElement.setCurrentTime(0);
+ document.getElementById('a').beginElementAt(1);
+ document.documentElement.setCurrentTime(2)">
+ <!--
+ This test case sets up a cycle between simultaneous instance times such that
+ when the instance times are sorted if this cycle is not detected we will
+ crash.
+ -->
+ <rect width="100" height="100" fill="red">
+ <set attributeName="fill" to="blue" begin="a.begin" dur="4s"/>
+ <set attributeName="fill" to="green" id="a"
+ begin="b.begin; 3s" dur="4s"/>
+ <set attributeName="fill" to="red" id="b"
+ begin="a.begin" dur="4s"/>
+ </rect>
+</svg>
--- a/content/smil/crashtests/crashtests.list
+++ b/content/smil/crashtests/crashtests.list
@@ -5,11 +5,13 @@ load 525099-1.svg
load 526536-1.svg
load 526875-1.svg
load 526875-2.svg
load 529387-1.xhtml
load 537157-1.svg
load 541297-1.svg
load 547333-1.svg
load 548899-1.svg
+load 554202-1.svg
+load 554202-2.svg
load 554141-1.svg
load 555026-1.svg
load 556841-1.svg
--- a/content/smil/nsSMILInstanceTime.cpp
+++ b/content/smil/nsSMILInstanceTime.cpp
@@ -158,51 +158,46 @@ nsSMILInstanceTime::HandleDeletedInterva
mBaseInterval = nsnull;
nsRefPtr<nsSMILInstanceTime> deathGrip(this);
mCreator->HandleDeletedInstanceTime(*this);
mCreator = nsnull;
}
PRBool
-nsSMILInstanceTime::IsDependent(const nsSMILInstanceTime& aOther,
- PRUint32 aRecursionDepth) const
+nsSMILInstanceTime::IsDependent(const nsSMILInstanceTime& aOther) const
{
- NS_ABORT_IF_FALSE(aRecursionDepth < 1000,
- "We seem to have created a cycle between instance times");
+ if (mVisited || mChainEnd)
+ return PR_FALSE;
const nsSMILInstanceTime* myBaseTime = GetBaseTime();
if (!myBaseTime)
return PR_FALSE;
if (myBaseTime == &aOther)
return PR_TRUE;
- return myBaseTime->IsDependent(aOther, ++aRecursionDepth);
+ // mVisited is mutable
+ AutoBoolSetter setVisited(const_cast<nsSMILInstanceTime*>(this)->mVisited);
+ return myBaseTime->IsDependent(aOther);
}
void
nsSMILInstanceTime::SetBaseInterval(nsSMILInterval* aBaseInterval)
{
NS_ABORT_IF_FALSE(!mBaseInterval,
"Attempting to reassociate an instance time with a different interval.");
- // Make sure we don't end up creating a cycle between the dependent time
- // pointers.
if (aBaseInterval) {
NS_ABORT_IF_FALSE(mCreator,
"Attempting to create a dependent instance time without reference "
"to the creating nsSMILTimeValueSpec object.");
if (!mCreator)
return;
- const nsSMILInstanceTime* dependentTime = mCreator->DependsOnBegin() ?
- aBaseInterval->Begin() :
- aBaseInterval->End();
- dependentTime->BreakPotentialCycle(this);
aBaseInterval->AddDependentTime(*this);
}
mBaseInterval = aBaseInterval;
}
const nsSMILInstanceTime*
nsSMILInstanceTime::GetBaseTime() const
@@ -214,26 +209,8 @@ nsSMILInstanceTime::GetBaseTime() const
NS_ABORT_IF_FALSE(mCreator, "Base interval is set but there is no creator.");
if (!mCreator) {
return nsnull;
}
return mCreator->DependsOnBegin() ? mBaseInterval->Begin() :
mBaseInterval->End();
}
-
-void
-nsSMILInstanceTime::BreakPotentialCycle(
- const nsSMILInstanceTime* aNewTail) const
-{
- const nsSMILInstanceTime* myBaseTime = GetBaseTime();
- if (!myBaseTime)
- return;
-
- if (myBaseTime == aNewTail) {
- // Making aNewTail the new tail of the chain would create a cycle so we
- // prevent this by unlinking the pointer to aNewTail.
- mBaseInterval->RemoveDependentTime(*this);
- return;
- }
-
- myBaseTime->BreakPotentialCycle(aNewTail);
-}
--- a/content/smil/nsSMILInstanceTime.h
+++ b/content/smil/nsSMILInstanceTime.h
@@ -104,34 +104,32 @@ public:
void DependentUpdate(const nsSMILTimeValue& aNewTime)
{
NS_ABORT_IF_FALSE(MayUpdate(),
"Updating an instance time that is not expected to be updated");
mTime = aNewTime;
}
- PRBool IsDependent(const nsSMILInstanceTime& aOther,
- PRUint32 aRecursionDepth = 0) const;
+ PRBool IsDependent(const nsSMILInstanceTime& aOther) const;
PRBool SameTimeAndBase(const nsSMILInstanceTime& aOther) const
{
return mTime == aOther.mTime && GetBaseTime() == aOther.GetBaseTime();
}
// Get and set a serial number which may be used by a containing class to
// control the sort order of otherwise similar instance times.
PRUint32 Serial() const { return mSerial; }
void SetSerial(PRUint32 aIndex) { mSerial = aIndex; }
NS_INLINE_DECL_REFCOUNTING(nsSMILInstanceTime)
protected:
void SetBaseInterval(nsSMILInterval* aBaseInterval);
- void BreakPotentialCycle(const nsSMILInstanceTime* aNewTail) const;
const nsSMILInstanceTime* GetBaseTime() const;
nsSMILTimeValue mTime;
// Internal flags used to represent the behaviour of different instance times
enum {
// Indicates if this instance time should be removed when the owning timed
// element is reset. True for events and DOM calls.
@@ -150,18 +148,21 @@ protected:
// that attribute (and hence an nsSMILTimeValueSpec), but not those from the
// DOM.
kFromDOM = 4
};
PRUint8 mFlags; // Combination of kClearOnReset, kMayUpdate, etc.
PRUint32 mSerial; // A serial number used by the containing class to
// specify the sort order for instance times with the
// same mTime.
- PRPackedBool mVisited;
- PRPackedBool mChainEnd;
+ PRPackedBool mVisited; // (mutable) Cycle tracking
+ PRPackedBool mChainEnd; // Flag to indicate that this instance time is part
+ // of some cyclic dependency and that in order to
+ // avoid infinite recursion the cycle should not be
+ // followed any further than this point.
nsSMILTimeValueSpec* mCreator; // The nsSMILTimeValueSpec object that created
// us. (currently only needed for syncbase
// instance times.)
nsSMILInterval* mBaseInterval; // Interval from which this time is derived
// (only used for syncbase instance times)
};
--- a/layout/reftests/svg/smil/syncbase/reftest.list
+++ b/layout/reftests/svg/smil/syncbase/reftest.list
@@ -78,13 +78,14 @@
== sandwich-priority-4.svg green-box-ref.svg
== sandwich-priority-5.svg green-box-ref.svg
== sandwich-priority-6.svg green-box-ref.svg
== sandwich-priority-7.svg green-box-ref.svg
== sandwich-priority-8.svg green-box-ref.svg
== sandwich-priority-9.svg green-box-ref.svg
== sandwich-priority-10.svg green-box-ref.svg
== sandwich-priority-11.svg green-box-ref.svg
+== sandwich-priority-12.svg green-box-ref.svg
# Cross-time container dependencies
== cross-container-1.xhtml green-box-ref.xhtml
== cross-container-2.xhtml green-box-ref.xhtml
== cross-container-3.xhtml green-box-ref.xhtml
new file mode 100644
--- /dev/null
+++ b/layout/reftests/svg/smil/syncbase/sandwich-priority-12.svg
@@ -0,0 +1,24 @@
+<svg xmlns="http://www.w3.org/2000/svg"
+ xmlns:xlink="http://www.w3.org/1999/xlink"
+ class="reftest-wait"
+ onload="
+ document.documentElement.pauseAnimations();
+ document.documentElement.setCurrentTime(0);
+ document.getElementById('a').beginElementAt(1);
+ setTimeAndSnapshot(2, false)">
+ <script xlink:href="../smil-util.js" type="text/javascript"/>
+ <!--
+ Test of animation sandwich priority based on syncbase dependencies.
+
+ This case includes a complex cycle that should nevertheless produce
+ consistent results.
+
+ If this fails, it will fail intermittently. The test is not so much
+ concerned with which colour should win (there are other tests for that) but
+ simply with determinancy.
+ -->
+ <rect width="100" height="100" fill="orange">
+ <set attributeName="fill" id="a" to="green" begin="b.begin; 3s"/>
+ <set attributeName="fill" id="b" to="red" begin="a.begin"/>
+ </rect>
+</svg>