Bug 629200 part 25 - Remove unnecessary serialisation from setting SVGStringList; r=jwatt
authorBrian Birtles <birtles@gmail.com>
Thu, 16 Feb 2012 08:40:46 +0900
changeset 89836 03ff5cb466daba5aac03c37ae6fde63f64d80f8a
parent 89835 ffb8f5c16fb6c4bd910b819e8461e9f791182167
child 89837 b6ba04315303eafa314d2715cf3321f7ba226306
push id783
push userlsblakk@mozilla.com
push dateTue, 24 Apr 2012 17:33:42 +0000
treeherdermozilla-beta@11faed19f136 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs629200
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 629200 part 25 - Remove unnecessary serialisation from setting SVGStringList; r=jwatt
content/base/src/nsAttrValue.cpp
content/base/src/nsAttrValue.h
content/svg/content/src/DOMSVGStringList.cpp
content/svg/content/src/DOMSVGStringList.h
content/svg/content/src/DOMSVGTests.cpp
content/svg/content/src/DOMSVGTests.h
content/svg/content/src/SVGAttrValueWrapper.cpp
content/svg/content/src/SVGAttrValueWrapper.h
content/svg/content/src/SVGStringList.cpp
content/svg/content/src/SVGStringList.h
content/svg/content/src/nsSVGElement.cpp
content/svg/content/src/nsSVGElement.h
--- a/content/base/src/nsAttrValue.cpp
+++ b/content/base/src/nsAttrValue.cpp
@@ -448,16 +448,28 @@ nsAttrValue::SetTo(const mozilla::SVGPoi
 void
 nsAttrValue::SetTo(const mozilla::SVGAnimatedPreserveAspectRatio& aValue,
                    const nsAString* aSerialized)
 {
   SetSVGType(eSVGPreserveAspectRatio, &aValue, aSerialized);
 }
 
 void
+nsAttrValue::SetTo(const mozilla::SVGStringList& aValue,
+                   const nsAString* aSerialized)
+{
+  // While an empty string will parse as a string list, there's no need to store
+  // it (and SetMiscAtomOrString will assert if we try)
+  if (aSerialized && aSerialized->IsEmpty()) {
+    aSerialized = nsnull;
+  }
+  SetSVGType(eSVGStringList, &aValue, aSerialized);
+}
+
+void
 nsAttrValue::SetTo(const mozilla::SVGTransformList& aValue,
                    const nsAString* aSerialized)
 {
   // While an empty string will parse as a transform list, there's no need to
   // store it (and SetMiscAtomOrString will assert if we try)
   if (aSerialized && aSerialized->IsEmpty()) {
     aSerialized = nsnull;
   }
@@ -612,16 +624,22 @@ nsAttrValue::ToString(nsAString& aResult
       break;
     }
     case eSVGPreserveAspectRatio:
     {
       SVGAttrValueWrapper::ToString(GetMiscContainer()->mSVGPreserveAspectRatio,
                                     aResult);
       break;
     }
+    case eSVGStringList:
+    {
+      SVGAttrValueWrapper::ToString(GetMiscContainer()->mSVGStringList,
+                                    aResult);
+      break;
+    }
     case eSVGTransformList:
     {
       SVGAttrValueWrapper::ToString(GetMiscContainer()->mSVGTransformList,
                                     aResult);
       break;
     }
     case eSVGViewBox:
     {
--- a/content/base/src/nsAttrValue.h
+++ b/content/base/src/nsAttrValue.h
@@ -133,18 +133,19 @@ public:
     ,eSVGIntegerPair = 0x15
     ,eSVGLength =      0x16
     ,eSVGLengthList =  0x17
     ,eSVGNumberList =  0x18
     ,eSVGNumberPair =  0x19
     ,eSVGPathData   =  0x20
     ,eSVGPointList  =  0x21
     ,eSVGPreserveAspectRatio = 0x22
-    ,eSVGTransformList = 0x23
-    ,eSVGViewBox =     0x24
+    ,eSVGStringList =  0x23
+    ,eSVGTransformList = 0x24
+    ,eSVGViewBox =     0x25
     ,eSVGTypesEnd =    0x34
   };
 
   ValueType Type() const;
 
   void Reset();
 
   void SetTo(const nsAttrValue& aOther);
@@ -162,16 +163,18 @@ public:
              const nsAString* aSerialized);
   void SetTo(const mozilla::SVGNumberList& aValue,
              const nsAString* aSerialized);
   void SetTo(const nsSVGNumberPair& aValue, const nsAString* aSerialized);
   void SetTo(const mozilla::SVGPathData& aValue, const nsAString* aSerialized);
   void SetTo(const mozilla::SVGPointList& aValue, const nsAString* aSerialized);
   void SetTo(const mozilla::SVGAnimatedPreserveAspectRatio& aValue,
              const nsAString* aSerialized);
+  void SetTo(const mozilla::SVGStringList& aValue,
+             const nsAString* aSerialized);
   void SetTo(const mozilla::SVGTransformList& aValue,
              const nsAString* aSerialized);
   void SetTo(const nsSVGViewBox& aValue, const nsAString* aSerialized);
 
   /**
    * Sets this object with the string or atom representation of aValue.
    *
    * After calling this method, this object will have type eString unless the
@@ -405,16 +408,17 @@ private:
       const nsSVGIntegerPair* mSVGIntegerPair;
       const nsSVGLength2* mSVGLength;
       const mozilla::SVGLengthList* mSVGLengthList;
       const mozilla::SVGNumberList* mSVGNumberList;
       const nsSVGNumberPair* mSVGNumberPair;
       const mozilla::SVGPathData* mSVGPathData;
       const mozilla::SVGPointList* mSVGPointList;
       const mozilla::SVGAnimatedPreserveAspectRatio* mSVGPreserveAspectRatio;
+      const mozilla::SVGStringList* mSVGStringList;
       const mozilla::SVGTransformList* mSVGTransformList;
       const nsSVGViewBox* mSVGViewBox;
     };
   };
 
   inline ValueBaseType BaseType() const;
   inline bool IsSVGType(ValueType aType) const;
 
--- a/content/svg/content/src/DOMSVGStringList.cpp
+++ b/content/svg/content/src/DOMSVGStringList.cpp
@@ -104,19 +104,22 @@ DOMSVGStringList::GetLength(PRUint32 *aL
 {
   return GetNumberOfItems(aLength);
 }
 
 NS_IMETHODIMP
 DOMSVGStringList::Clear()
 {
   if (InternalList().IsExplicitlySet()) {
+    nsAttrValue emptyOrOldValue =
+      mElement->WillChangeStringList(mIsConditionalProcessingAttribute,
+                                     mAttrEnum);
     InternalList().Clear();
     mElement->DidChangeStringList(mIsConditionalProcessingAttribute,
-                                  mAttrEnum);
+                                  mAttrEnum, emptyOrOldValue);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMSVGStringList::Initialize(const nsAString & newItem, nsAString & _retval)
 {
   if (InternalList().IsExplicitlySet()) {
@@ -146,19 +149,23 @@ DOMSVGStringList::InsertItemBefore(const
   }
   index = NS_MIN(index, InternalList().Length());
 
   // Ensure we have enough memory so we can avoid complex error handling below:
   if (!InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
+  nsAttrValue emptyOrOldValue =
+    mElement->WillChangeStringList(mIsConditionalProcessingAttribute,
+                                   mAttrEnum);
   InternalList().InsertItem(index, newItem);
 
-  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum);
+  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum,
+                                emptyOrOldValue);
   _retval = newItem;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMSVGStringList::ReplaceItem(const nsAString & newItem,
                               PRUint32 index,
                               nsAString & _retval)
@@ -166,33 +173,41 @@ DOMSVGStringList::ReplaceItem(const nsAS
   if (newItem.IsEmpty()) { // takes care of DOMStringIsNull too
     return NS_ERROR_DOM_SVG_INVALID_VALUE_ERR;
   }
   if (index >= InternalList().Length()) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
 
   _retval = InternalList()[index];
+  nsAttrValue emptyOrOldValue =
+    mElement->WillChangeStringList(mIsConditionalProcessingAttribute,
+                                   mAttrEnum);
   InternalList().ReplaceItem(index, newItem);
 
-  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum);
+  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum,
+                                emptyOrOldValue);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMSVGStringList::RemoveItem(PRUint32 index,
                              nsAString & _retval)
 {
   if (index >= InternalList().Length()) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
 
+  nsAttrValue emptyOrOldValue =
+    mElement->WillChangeStringList(mIsConditionalProcessingAttribute,
+                                   mAttrEnum);
   InternalList().RemoveItem(index);
 
-  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum);
+  mElement->DidChangeStringList(mIsConditionalProcessingAttribute, mAttrEnum,
+                                emptyOrOldValue);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DOMSVGStringList::AppendItem(const nsAString & newItem,
                              nsAString & _retval)
 {
   return InsertItemBefore(newItem, InternalList().Length(), _retval);
--- a/content/svg/content/src/DOMSVGStringList.h
+++ b/content/svg/content/src/DOMSVGStringList.h
@@ -106,18 +106,16 @@ private:
                    bool aIsConditionalProcessingAttribute, PRUint8 aAttrEnum)
     : mElement(aElement)
     , mAttrEnum(aAttrEnum)
     , mIsConditionalProcessingAttribute(aIsConditionalProcessingAttribute)
   {}
 
   ~DOMSVGStringList();
 
-  void DidChangeStringList(PRUint8 aAttrEnum, bool aDoSetAttr);
-
   SVGStringList &InternalList();
 
   // Strong ref to our element to keep it alive.
   nsRefPtr<nsSVGElement> mElement;
 
   PRUint8 mAttrEnum;
 
   bool    mIsConditionalProcessingAttribute;
--- a/content/svg/content/src/DOMSVGTests.cpp
+++ b/content/svg/content/src/DOMSVGTests.cpp
@@ -43,23 +43,28 @@
 #include "nsStyleUtil.h"
 #include "nsSVGUtils.h"
 #include "mozilla/Preferences.h"
 
 using namespace mozilla;
 
 NS_IMPL_ISUPPORTS1(DOMSVGTests, nsIDOMSVGTests)
 
-DOMSVGTests::StringListInfo DOMSVGTests::sStringListInfo[3] =
+nsIAtom** DOMSVGTests::sStringListNames[3] =
 {
-  { &nsGkAtoms::requiredFeatures, false },
-  { &nsGkAtoms::requiredExtensions, false },
-  { &nsGkAtoms::systemLanguage, true }
+  &nsGkAtoms::requiredFeatures,
+  &nsGkAtoms::requiredExtensions,
+  &nsGkAtoms::systemLanguage,
 };
 
+DOMSVGTests::DOMSVGTests()
+{
+  mStringListAttributes[LANGUAGE].SetIsCommaSeparated(true);
+}
+
 /* readonly attribute nsIDOMSVGStringList requiredFeatures; */
 NS_IMETHODIMP
 DOMSVGTests::GetRequiredFeatures(nsIDOMSVGStringList * *aRequiredFeatures)
 {
   nsCOMPtr<nsSVGElement> element = do_QueryInterface(this);
   *aRequiredFeatures = DOMSVGStringList::GetDOMWrapper(
                          &mStringListAttributes[FEATURES], element, true, FEATURES).get();
   return NS_OK;
@@ -91,18 +96,18 @@ DOMSVGTests::HasExtension(const nsAStrin
 {
   *_retval = nsSVGFeatures::HasExtension(extension);
   return NS_OK;
 }
 
 bool
 DOMSVGTests::IsConditionalProcessingAttribute(const nsIAtom* aAttribute) const
 {
-  for (PRUint32 i = 0; i < ArrayLength(sStringListInfo); i++) {
-    if (aAttribute == *sStringListInfo[i].mName) {
+  for (PRUint32 i = 0; i < ArrayLength(sStringListNames); i++) {
+    if (aAttribute == *sStringListNames[i]) {
       return true;
     }
   }
   return false;
 }
 
 PRInt32
 DOMSVGTests::GetBestLanguagePreferenceRank(const nsSubstring& aAcceptLangs) const
@@ -217,73 +222,59 @@ DOMSVGTests::PassesConditionalProcessing
   return true;
 }
 
 bool
 DOMSVGTests::ParseConditionalProcessingAttribute(nsIAtom* aAttribute,
                                                  const nsAString& aValue,
                                                  nsAttrValue& aResult)
 {
-  for (PRUint32 i = 0; i < ArrayLength(sStringListInfo); i++) {
-    if (aAttribute == *sStringListInfo[i].mName) {
-      nsresult rv = mStringListAttributes[i].SetValue(
-                      aValue, sStringListInfo[i].mIsCommaSeparated);
+  for (PRUint32 i = 0; i < ArrayLength(sStringListNames); i++) {
+    if (aAttribute == *sStringListNames[i]) {
+      nsresult rv = mStringListAttributes[i].SetValue(aValue);
       if (NS_FAILED(rv)) {
         mStringListAttributes[i].Clear();
       }
       MaybeInvalidate();
       return true;
     }
   }
   return false;
 }
 
 void
-DOMSVGTests::GetValue(PRUint8 aAttrEnum, nsAString& aValue) const
-{
-  NS_ABORT_IF_FALSE(aAttrEnum >= 0 && aAttrEnum < ArrayLength(sStringListInfo),
-                    "aAttrEnum out of range");
-  mStringListAttributes[aAttrEnum].GetValue(
-    aValue, sStringListInfo[aAttrEnum].mIsCommaSeparated);
-}
-
-void
 DOMSVGTests::UnsetAttr(const nsIAtom* aAttribute)
 {
-  for (PRUint32 i = 0; i < ArrayLength(sStringListInfo); i++) {
-    if (aAttribute == *sStringListInfo[i].mName) {
+  for (PRUint32 i = 0; i < ArrayLength(sStringListNames); i++) {
+    if (aAttribute == *sStringListNames[i]) {
       mStringListAttributes[i].Clear();
       MaybeInvalidate();
       return;
     }
   }
 }
 
-void
-DOMSVGTests::DidChangeStringList(PRUint8 aAttrEnum)
+nsIAtom*
+DOMSVGTests::GetAttrName(PRUint8 aAttrEnum) const
 {
-  NS_ASSERTION(aAttrEnum < ArrayLength(sStringListInfo), "aAttrEnum out of range");
-
-  nsCOMPtr<nsSVGElement> element = do_QueryInterface(this);
+  return *sStringListNames[aAttrEnum];
+}
 
-  nsAutoString serializedValue;
-  GetValue(aAttrEnum, serializedValue);
-
-  nsAttrValue attrValue(serializedValue);
-  element->SetParsedAttr(kNameSpaceID_None,
-                         *sStringListInfo[aAttrEnum].mName,
-                         nsnull, attrValue, true);
-
-  MaybeInvalidate();
+void
+DOMSVGTests::GetAttrValue(PRUint8 aAttrEnum, nsAttrValue& aValue) const
+{
+  NS_ABORT_IF_FALSE(aAttrEnum >= 0 && aAttrEnum < ArrayLength(sStringListNames),
+                    "aAttrEnum out of range");
+  aValue.SetTo(mStringListAttributes[aAttrEnum], nsnull);
 }
 
 void
 DOMSVGTests::MaybeInvalidate()
 {
   nsCOMPtr<nsSVGElement> element = do_QueryInterface(this);
 
   nsIContent* parent = element->GetFlattenedTreeParent();
-  
+
   if (parent &&
       parent->NodeInfo()->Equals(nsGkAtoms::svgSwitch, kNameSpaceID_SVG)) {
     static_cast<nsSVGSwitchElement*>(parent)->MaybeInvalidate();
   }
 }
--- a/content/svg/content/src/DOMSVGTests.h
+++ b/content/svg/content/src/DOMSVGTests.h
@@ -54,16 +54,18 @@ class DOMSVGTests : public nsIDOMSVGTest
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIDOMSVGTESTS
 
   friend class mozilla::DOMSVGStringList;
   typedef mozilla::SVGStringList SVGStringList;
 
+  DOMSVGTests();
+
   /**
    * Compare the language name(s) in a systemLanguage attribute to the
    * user's language preferences, as defined in
    * http://www.w3.org/TR/SVG11/struct.html#SystemLanguageAttribute
    * We have a match if a language name in the users language preferences
    * exactly equals one of the language names or exactly equals a prefix of
    * one of the language names in the systemLanguage attribute.
    * @returns 2 * the lowest index in the aAcceptLangs that matches + 1
@@ -99,34 +101,24 @@ public:
   bool IsConditionalProcessingAttribute(const nsIAtom* aAttribute) const;
 
   bool ParseConditionalProcessingAttribute(
          nsIAtom* aAttribute,
          const nsAString& aValue,
          nsAttrValue& aResult);
 
   /**
-   * Serialises the conditional processing attribute.
-   */
-  void GetValue(PRUint8 aAttrEnum, nsAString& aValue) const;
-
-  /**
    * Unsets a conditional processing attribute.
    */
   void UnsetAttr(const nsIAtom* aAttribute);
 
-  void DidChangeStringList(PRUint8 aAttrEnum);
+  nsIAtom* GetAttrName(PRUint8 aAttrEnum) const;
+  void GetAttrValue(PRUint8 aAttrEnum, nsAttrValue &aValue) const;
 
   void MaybeInvalidate();
 
 private:
-
-  struct StringListInfo {
-    nsIAtom** mName;
-    bool      mIsCommaSeparated;
-  };
-
   enum { FEATURES, EXTENSIONS, LANGUAGE };
   SVGStringList mStringListAttributes[3];
-  static StringListInfo sStringListInfo[3];
+  static nsIAtom** sStringListNames[3];
 };
 
 #endif // MOZILLA_DOMSVGTESTS_H__
--- a/content/svg/content/src/SVGAttrValueWrapper.cpp
+++ b/content/svg/content/src/SVGAttrValueWrapper.cpp
@@ -41,16 +41,17 @@
 #include "nsSVGLength2.h"
 #include "nsSVGNumberPair.h"
 #include "nsSVGViewBox.h"
 #include "SVGAnimatedPreserveAspectRatio.h"
 #include "SVGLengthList.h"
 #include "SVGNumberList.h"
 #include "SVGPathData.h"
 #include "SVGPointList.h"
+#include "SVGStringList.h"
 #include "SVGTransformList.h"
 
 using namespace mozilla;
 
 /*static*/ void
 SVGAttrValueWrapper::ToString(const nsSVGAngle* aAngle, nsAString& aResult)
 {
   aAngle->GetBaseValueString(aResult);
@@ -107,16 +108,23 @@ SVGAttrValueWrapper::ToString(const SVGP
 SVGAttrValueWrapper::ToString(
   const SVGAnimatedPreserveAspectRatio* aPreserveAspectRatio,
   nsAString& aResult)
 {
   aPreserveAspectRatio->GetBaseValueString(aResult);
 }
 
 /*static*/ void
+SVGAttrValueWrapper::ToString(const SVGStringList* aStringList,
+                              nsAString& aResult)
+{
+  aStringList->GetValue(aResult);
+}
+
+/*static*/ void
 SVGAttrValueWrapper::ToString(const SVGTransformList* aTransformList,
                               nsAString& aResult)
 {
   aTransformList->GetValueAsString(aResult);
 }
 
 /*static*/ void
 SVGAttrValueWrapper::ToString(const nsSVGViewBox* aViewBox, nsAString& aResult)
--- a/content/svg/content/src/SVGAttrValueWrapper.h
+++ b/content/svg/content/src/SVGAttrValueWrapper.h
@@ -52,16 +52,17 @@ class nsSVGNumberPair;
 class nsSVGViewBox;
 
 namespace mozilla {
 class SVGLengthList;
 class SVGNumberList;
 class SVGPathData;
 class SVGPointList;
 class SVGAnimatedPreserveAspectRatio;
+class SVGStringList;
 class SVGTransformList;
 }
 
 namespace mozilla {
 
 class SVGAttrValueWrapper
 {
 public:
@@ -76,16 +77,18 @@ public:
   static void ToString(const nsSVGNumberPair* aNumberPair, nsAString& aResult);
   static void ToString(const mozilla::SVGPathData* aPathData,
                        nsAString& aResult);
   static void ToString(const mozilla::SVGPointList* aPointList,
                        nsAString& aResult);
   static void ToString(
     const mozilla::SVGAnimatedPreserveAspectRatio* aPreserveAspectRatio,
     nsAString& aResult);
+  static void ToString(const mozilla::SVGStringList* aStringList,
+                       nsAString& aResult);
   static void ToString(const mozilla::SVGTransformList* aTransformList,
                        nsAString& aResult);
   static void ToString(const nsSVGViewBox* aViewBox, nsAString& aResult);
 };
 
 } /* namespace mozilla */
 
 #endif // MOZILLA_SVGATTRVALUEWRAPPER_H__
--- a/content/svg/content/src/SVGStringList.cpp
+++ b/content/svg/content/src/SVGStringList.cpp
@@ -57,37 +57,37 @@ SVGStringList::CopyFrom(const SVGStringL
     return NS_ERROR_OUT_OF_MEMORY;
   }
   mStrings = rhs.mStrings;
   mIsSet = true;
   return NS_OK;
 }
 
 void
-SVGStringList::GetValue(nsAString& aValue, bool aIsCommaSeparated) const
+SVGStringList::GetValue(nsAString& aValue) const
 {
   aValue.Truncate();
   PRUint32 last = mStrings.Length() - 1;
   for (PRUint32 i = 0; i < mStrings.Length(); ++i) {
     aValue.Append(mStrings[i]);
     if (i != last) {
-      if (aIsCommaSeparated) {
+      if (mIsCommaSeparated) {
         aValue.Append(',');
       }
       aValue.Append(' ');
     }
   }
 }
 
 nsresult
-SVGStringList::SetValue(const nsAString& aValue, bool aIsCommaSeparated)
+SVGStringList::SetValue(const nsAString& aValue)
 {
   SVGStringList temp;
 
-  if (aIsCommaSeparated) {
+  if (mIsCommaSeparated) {
     nsCharSeparatedTokenizerTemplate<IsSVGWhitespace>
       tokenizer(aValue, ',');
 
     while (tokenizer.hasMoreTokens()) {
       if (!temp.AppendItem(tokenizer.nextToken())) {
         return NS_ERROR_OUT_OF_MEMORY;
       }
     }
--- a/content/svg/content/src/SVGStringList.h
+++ b/content/svg/content/src/SVGStringList.h
@@ -49,28 +49,31 @@ namespace mozilla {
  * The DOM wrapper class for this class is DOMSVGStringList.
  */
 class SVGStringList
 {
   friend class DOMSVGStringList;
 
 public:
 
-  SVGStringList() : mIsSet(false) {}
+  SVGStringList() : mIsSet(false), mIsCommaSeparated(false) {}
   ~SVGStringList(){}
 
-  nsresult SetValue(const nsAString& aValue, bool aIsCommaSeparated);
+  void SetIsCommaSeparated(bool aIsCommaSeparated) {
+    mIsCommaSeparated = aIsCommaSeparated;
+  }
+  nsresult SetValue(const nsAString& aValue);
 
   void Clear() {
     mStrings.Clear();
     mIsSet = false;
   }
 
   /// This may return an incomplete string on OOM, but that's acceptable.
-  void GetValue(nsAString& aValue, bool aIsCommaSeparated) const;
+  void GetValue(nsAString& aValue) const;
 
   bool IsEmpty() const {
     return mStrings.IsEmpty();
   }
 
   PRUint32 Length() const {
     return mStrings.Length();
   }
@@ -86,19 +89,18 @@ public:
   bool SetCapacity(PRUint32 size) {
     return mStrings.SetCapacity(size);
   }
 
   void Compact() {
     mStrings.Compact();
   }
 
-  // Returns true if the animated value of this stringlist has been explicitly
-  // set by taking on the base value which has been explicitly set by markup
-  // or a DOM call, false otherwise.
+  // Returns true if the value of this stringlist has been explicitly
+  // set by markup or a DOM call, false otherwise.
   bool IsExplicitlySet() const
     { return mIsSet; }
 
   // Access to methods that can modify objects of this type is deliberately
   // limited. This is to reduce the chances of someone modifying objects of
   // this type without taking the necessary steps to keep DOM wrappers in sync.
   // If you need wider access to these methods, consider adding a method to
   // SVGAnimatedStringList and having that class act as an intermediary so it
@@ -163,13 +165,14 @@ private:
 
 protected:
 
   /* See SVGLengthList for the rationale for using nsTArray<float> instead
    * of nsTArray<float, 1>.
    */
   nsTArray<nsString> mStrings;
   bool mIsSet;
+  bool mIsCommaSeparated;
 };
 
 } // namespace mozilla
 
 #endif // MOZILLA_SVGSTRINGLIST_H__
--- a/content/svg/content/src/nsSVGElement.cpp
+++ b/content/svg/content/src/nsSVGElement.cpp
@@ -530,19 +530,22 @@ nsSVGElement::ParseAttribute(PRInt32 aNa
       }
     }
 
     if (!foundMatch) {
       // Check for StringList attribute
       StringListAttributesInfo stringListInfo = GetStringListInfo();
       for (i = 0; i < stringListInfo.mStringListCount; i++) {
         if (aAttribute == *stringListInfo.mStringListInfo[i].mName) {
-          rv = stringListInfo.mStringLists[i].SetValue(aValue, false);
+          rv = stringListInfo.mStringLists[i].SetValue(aValue);
           if (NS_FAILED(rv)) {
             stringListInfo.Reset(i);
+          } else {
+            aResult.SetTo(stringListInfo.mStringLists[i], &aValue);
+            didSetResult = true;
           }
           foundMatch = true;
           break;
         }
       }
     }
 
     if (!foundMatch) {
@@ -794,25 +797,27 @@ nsSVGElement::UnsetAttrInternal(PRInt32 
         transformList->ClearBaseValue();
         return;
       }
     }
 
     // Check for conditional processing attributes
     nsCOMPtr<DOMSVGTests> tests(do_QueryInterface(this));
     if (tests && tests->IsConditionalProcessingAttribute(aName)) {
+      MaybeSerializeAttrBeforeRemoval(aName, aNotify);
       tests->UnsetAttr(aName);
       return;
     }
 
     // Check if this is a string list attribute going away
     StringListAttributesInfo stringListInfo = GetStringListInfo();
 
     for (PRUint32 i = 0; i < stringListInfo.mStringListCount; i++) {
       if (aName == *stringListInfo.mStringListInfo[i].mName) {
+        MaybeSerializeAttrBeforeRemoval(aName, aNotify);
         stringListInfo.Reset(i);
         return;
       }
     }
   }
 
   // Check if this is a string attribute going away
   StringAttributesInfo stringInfo = GetStringInfo();
@@ -2340,39 +2345,59 @@ nsSVGElement::DidAnimateString(PRUint8 a
 }
 
 nsSVGElement::StringListAttributesInfo
 nsSVGElement::GetStringListInfo()
 {
   return StringListAttributesInfo(nsnull, nsnull, 0);
 }
 
+nsAttrValue
+nsSVGElement::WillChangeStringList(bool aIsConditionalProcessingAttribute,
+                                   PRUint8 aAttrEnum)
+{
+  nsIAtom* name;
+  if (aIsConditionalProcessingAttribute) {
+    nsCOMPtr<DOMSVGTests> tests(do_QueryInterface(this));
+    name = tests->GetAttrName(aAttrEnum);
+  } else {
+    name = *GetStringListInfo().mStringListInfo[aAttrEnum].mName;
+  }
+  return WillChangeValue(name);
+}
+
 void
 nsSVGElement::DidChangeStringList(bool aIsConditionalProcessingAttribute,
-                                  PRUint8 aAttrEnum)
+                                  PRUint8 aAttrEnum,
+                                  const nsAttrValue& aEmptyOrOldValue)
 {
+  nsIAtom* name;
+  nsAttrValue newValue;
+  nsCOMPtr<DOMSVGTests> tests;
+
   if (aIsConditionalProcessingAttribute) {
-    nsCOMPtr<DOMSVGTests> tests(do_QueryInterface(this));
-    tests->DidChangeStringList(aAttrEnum);
-    return;
+    tests = do_QueryInterface(this);
+    name = tests->GetAttrName(aAttrEnum);
+    tests->GetAttrValue(aAttrEnum, newValue);
+  } else {
+    StringListAttributesInfo info = GetStringListInfo();
+
+    NS_ASSERTION(info.mStringListCount > 0,
+                 "DidChangeStringList on element with no string list attribs");
+    NS_ASSERTION(aAttrEnum < info.mStringListCount, "aAttrEnum out of range");
+
+    name = *info.mStringListInfo[aAttrEnum].mName;
+    newValue.SetTo(info.mStringLists[aAttrEnum], nsnull);
   }
 
-  StringListAttributesInfo info = GetStringListInfo();
-
-  NS_ASSERTION(info.mStringListCount > 0,
-               "DidChangeStringList on element with no string list attribs");
-
-  NS_ASSERTION(aAttrEnum < info.mStringListCount, "aAttrEnum out of range");
+  DidChangeValue(name, aEmptyOrOldValue, newValue);
 
-  nsAutoString serializedValue;
-  info.mStringLists[aAttrEnum].GetValue(serializedValue, this);
-
-  nsAttrValue attrValue(serializedValue);
-  SetParsedAttr(kNameSpaceID_None, *info.mStringListInfo[aAttrEnum].mName,
-                nsnull, attrValue, true);
+  if (aIsConditionalProcessingAttribute) {
+    tests->MaybeInvalidate();
+  }
 }
 
 void
 nsSVGElement::StringListAttributesInfo::Reset(PRUint8 aAttrEnum)
 {
   mStringLists[aAttrEnum].Clear();
   // caller notifies
 }
--- a/content/svg/content/src/nsSVGElement.h
+++ b/content/svg/content/src/nsSVGElement.h
@@ -176,16 +176,18 @@ public:
   nsAttrValue WillChangeAngle(PRUint8 aAttrEnum);
   nsAttrValue WillChangeViewBox();
   nsAttrValue WillChangePreserveAspectRatio();
   nsAttrValue WillChangeNumberList(PRUint8 aAttrEnum);
   nsAttrValue WillChangeLengthList(PRUint8 aAttrEnum);
   nsAttrValue WillChangePointList();
   nsAttrValue WillChangePathSegList();
   nsAttrValue WillChangeTransformList();
+  nsAttrValue WillChangeStringList(bool aIsConditionalProcessingAttribute,
+                                   PRUint8 aAttrEnum);
 
   void DidChangeLength(PRUint8 aAttrEnum, const nsAttrValue& aEmptyOrOldValue);
   void DidChangeNumber(PRUint8 aAttrEnum);
   void DidChangeNumberPair(PRUint8 aAttrEnum,
                            const nsAttrValue& aEmptyOrOldValue);
   void DidChangeInteger(PRUint8 aAttrEnum);
   void DidChangeIntegerPair(PRUint8 aAttrEnum,
                             const nsAttrValue& aEmptyOrOldValue);
@@ -198,17 +200,18 @@ public:
                            const nsAttrValue& aEmptyOrOldValue);
   void DidChangeLengthList(PRUint8 aAttrEnum,
                            const nsAttrValue& aEmptyOrOldValue);
   void DidChangePointList(const nsAttrValue& aEmptyOrOldValue);
   void DidChangePathSegList(const nsAttrValue& aEmptyOrOldValue);
   void DidChangeTransformList(const nsAttrValue& aEmptyOrOldValue);
   void DidChangeString(PRUint8 aAttrEnum) {}
   void DidChangeStringList(bool aIsConditionalProcessingAttribute,
-                           PRUint8 aAttrEnum);
+                           PRUint8 aAttrEnum,
+                           const nsAttrValue& aEmptyOrOldValue);
 
   virtual void DidAnimateLength(PRUint8 aAttrEnum);
   virtual void DidAnimateNumber(PRUint8 aAttrEnum);
   virtual void DidAnimateNumberPair(PRUint8 aAttrEnum);
   virtual void DidAnimateInteger(PRUint8 aAttrEnum);
   virtual void DidAnimateIntegerPair(PRUint8 aAttrEnum);
   virtual void DidAnimateAngle(PRUint8 aAttrEnum);
   virtual void DidAnimateBoolean(PRUint8 aAttrEnum);