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 86960 03ff5cb466daba5aac03c37ae6fde63f64d80f8a
parent 86959 ffb8f5c16fb6c4bd910b819e8461e9f791182167
child 86961 b6ba04315303eafa314d2715cf3321f7ba226306
push id537
push usertim.taubert@gmx.de
push dateThu, 16 Feb 2012 11:54:26 +0000
treeherderfx-team@6989376471f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs629200
milestone13.0a1
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);