From 9b7dedba2c414a82aab502b22adf56ef84544cbe Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Thu, 16 Feb 2012 08:40:46 +0900 Subject: [PATCH] Bug 629200 part 19 - Remove unnecessary serialisation from setting nsSVGNumberList; r=jwatt --- content/base/src/nsAttrValue.cpp | 31 +++++++++ content/base/src/nsAttrValue.h | 11 +++- content/svg/content/src/DOMSVGNumber.cpp | 6 +- content/svg/content/src/DOMSVGNumberList.cpp | 12 ++-- content/svg/content/src/Makefile.in | 1 + content/svg/content/src/nsSVGElement.cpp | 32 ++++++---- content/svg/content/src/nsSVGElement.h | 4 +- content/svg/content/test/Makefile.in | 1 + .../svg/content/test/test_SVGNumberList.xhtml | 64 +++++++++++++++++++ 9 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 content/svg/content/test/test_SVGNumberList.xhtml diff --git a/content/base/src/nsAttrValue.cpp b/content/base/src/nsAttrValue.cpp index d361156d4c83..de049970cf47 100644 --- a/content/base/src/nsAttrValue.cpp +++ b/content/base/src/nsAttrValue.cpp @@ -58,6 +58,7 @@ #include "nsSVGViewBox.h" #include "SVGAnimatedPreserveAspectRatio.h" #include "SVGLengthList.h" +#include "SVGNumberList.h" namespace css = mozilla::css; @@ -291,6 +292,11 @@ nsAttrValue::SetTo(const nsAttrValue& aOther) cont->mSVGLengthList = otherCont->mSVGLengthList; break; } + case eSVGNumberList: + { + cont->mSVGNumberList = otherCont->mSVGNumberList; + break; + } case eSVGNumberPair: { cont->mSVGNumberPair = otherCont->mSVGNumberPair; @@ -452,6 +458,18 @@ nsAttrValue::SetTo(const mozilla::SVGLengthList& aValue, } } +void +nsAttrValue::SetTo(const mozilla::SVGNumberList& aValue, + const nsAString* aSerialized) +{ + if (EnsureEmptyMiscContainer()) { + MiscContainer* cont = GetMiscContainer(); + cont->mSVGNumberList = &aValue; + cont->mType = eSVGNumberList; + SetMiscAtomOrString(aSerialized); + } +} + void nsAttrValue::SetTo(const nsSVGNumberPair& aValue, const nsAString* aSerialized) { @@ -603,6 +621,11 @@ nsAttrValue::ToString(nsAString& aResult) const GetMiscContainer()->mSVGLengthList->GetValueAsString(aResult); break; } + case eSVGNumberList: + { + GetMiscContainer()->mSVGNumberList->GetValueAsString(aResult); + break; + } case eSVGNumberPair: { GetMiscContainer()->mSVGNumberPair->GetBaseValueString(aResult); @@ -818,6 +841,10 @@ nsAttrValue::HashValue() const { return NS_PTR_TO_INT32(cont->mSVGLengthList); } + case eSVGNumberList: + { + return NS_PTR_TO_INT32(cont->mSVGNumberList); + } case eSVGNumberPair: { return NS_PTR_TO_INT32(cont->mSVGNumberPair); @@ -938,6 +965,10 @@ nsAttrValue::Equals(const nsAttrValue& aOther) const { return thisCont->mSVGLengthList == otherCont->mSVGLengthList; } + case eSVGNumberList: + { + return thisCont->mSVGNumberList == otherCont->mSVGNumberList; + } case eSVGNumberPair: { return thisCont->mSVGNumberPair == otherCont->mSVGNumberPair; diff --git a/content/base/src/nsAttrValue.h b/content/base/src/nsAttrValue.h index b3d55e87a490..3f600be4bb7f 100644 --- a/content/base/src/nsAttrValue.h +++ b/content/base/src/nsAttrValue.h @@ -70,6 +70,7 @@ class StyleRule; } class SVGAnimatedPreserveAspectRatio; class SVGLengthList; +class SVGNumberList; } #define NS_ATTRVALUE_MAX_STRINGLENGTH_ATOM 12 @@ -138,9 +139,10 @@ class nsAttrValue { ,eSVGIntegerPair = 0x15 ,eSVGLength = 0x16 ,eSVGLengthList = 0x17 - ,eSVGNumberPair = 0x18 - ,eSVGPreserveAspectRatio = 0x19 - ,eSVGViewBox = 0x20 + ,eSVGNumberList = 0x18 + ,eSVGNumberPair = 0x19 + ,eSVGPreserveAspectRatio = 0x20 + ,eSVGViewBox = 0x21 }; ValueType Type() const; @@ -160,6 +162,8 @@ class nsAttrValue { void SetTo(const nsSVGLength2& aValue, const nsAString* aSerialized); void SetTo(const mozilla::SVGLengthList& aValue, const nsAString* aSerialized); + void SetTo(const mozilla::SVGNumberList& aValue, + const nsAString* aSerialized); void SetTo(const nsSVGNumberPair& aValue, const nsAString* aSerialized); void SetTo(const mozilla::SVGAnimatedPreserveAspectRatio& aValue, const nsAString* aSerialized); @@ -399,6 +403,7 @@ class nsAttrValue { const nsSVGIntegerPair* mSVGIntegerPair; const nsSVGLength2* mSVGLength; const mozilla::SVGLengthList* mSVGLengthList; + const mozilla::SVGNumberList* mSVGNumberList; const nsSVGNumberPair* mSVGNumberPair; const mozilla::SVGAnimatedPreserveAspectRatio* mSVGPreserveAspectRatio; const nsSVGViewBox* mSVGViewBox; diff --git a/content/svg/content/src/DOMSVGNumber.cpp b/content/svg/content/src/DOMSVGNumber.cpp index 2648f32b1d4b..a9f815964a2f 100644 --- a/content/svg/content/src/DOMSVGNumber.cpp +++ b/content/svg/content/src/DOMSVGNumber.cpp @@ -123,8 +123,12 @@ DOMSVGNumber::SetValue(float aValue) NS_ENSURE_FINITE(aValue, NS_ERROR_ILLEGAL_VALUE); if (HasOwner()) { + if (InternalItem() == aValue) { + return NS_OK; + } + nsAttrValue emptyOrOldValue = Element()->WillChangeNumberList(mAttrEnum); InternalItem() = aValue; - Element()->DidChangeNumberList(mAttrEnum, true); + Element()->DidChangeNumberList(mAttrEnum, emptyOrOldValue); if (mList->mAList->IsAnimating()) { Element()->AnimationNeedsResample(); } diff --git a/content/svg/content/src/DOMSVGNumberList.cpp b/content/svg/content/src/DOMSVGNumberList.cpp index 6e9d8b804584..1f458180f52f 100644 --- a/content/svg/content/src/DOMSVGNumberList.cpp +++ b/content/svg/content/src/DOMSVGNumberList.cpp @@ -171,6 +171,7 @@ DOMSVGNumberList::Clear() } if (Length() > 0) { + nsAttrValue emptyOrOldValue = Element()->WillChangeNumberList(AttrEnum()); // Notify any existing DOM items of removal *before* truncating the lists // so that they can find their SVGNumber internal counterparts and copy // their values. This also notifies the animVal list: @@ -178,7 +179,7 @@ DOMSVGNumberList::Clear() mItems.Clear(); InternalList().Clear(); - Element()->DidChangeNumberList(AttrEnum(), true); + Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue); if (mAList->IsAnimating()) { Element()->AnimationNeedsResample(); } @@ -256,6 +257,7 @@ DOMSVGNumberList::InsertItemBefore(nsIDOMSVGNumber *newItem, return NS_ERROR_OUT_OF_MEMORY; } + nsAttrValue emptyOrOldValue = Element()->WillChangeNumberList(AttrEnum()); // Now that we know we're inserting, keep animVal list in sync as necessary. MaybeInsertNullInAnimValListAt(index); @@ -269,7 +271,7 @@ DOMSVGNumberList::InsertItemBefore(nsIDOMSVGNumber *newItem, UpdateListIndicesFromIndex(mItems, index + 1); - Element()->DidChangeNumberList(AttrEnum(), true); + Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue); if (mAList->IsAnimating()) { Element()->AnimationNeedsResample(); } @@ -298,6 +300,7 @@ DOMSVGNumberList::ReplaceItem(nsIDOMSVGNumber *newItem, domItem = domItem->Clone(); // must do this before changing anything! } + nsAttrValue emptyOrOldValue = Element()->WillChangeNumberList(AttrEnum()); if (mItems[index]) { // Notify any existing DOM item of removal *before* modifying the lists so // that the DOM item can copy the *old* value at its index: @@ -311,7 +314,7 @@ DOMSVGNumberList::ReplaceItem(nsIDOMSVGNumber *newItem, // would end up reading bad data from InternalList()! domItem->InsertingIntoList(this, AttrEnum(), index, IsAnimValList()); - Element()->DidChangeNumberList(AttrEnum(), true); + Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue); if (mAList->IsAnimating()) { Element()->AnimationNeedsResample(); } @@ -340,6 +343,7 @@ DOMSVGNumberList::RemoveItem(PRUint32 index, // We have to return the removed item, so make sure it exists: EnsureItemAt(index); + nsAttrValue emptyOrOldValue = Element()->WillChangeNumberList(AttrEnum()); // Notify the DOM item of removal *before* modifying the lists so that the // DOM item can copy its *old* value: mItems[index]->RemovingFromList(); @@ -350,7 +354,7 @@ DOMSVGNumberList::RemoveItem(PRUint32 index, UpdateListIndicesFromIndex(mItems, index); - Element()->DidChangeNumberList(AttrEnum(), true); + Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue); if (mAList->IsAnimating()) { Element()->AnimationNeedsResample(); } diff --git a/content/svg/content/src/Makefile.in b/content/svg/content/src/Makefile.in index 3640ae7d446d..aee55ff3a7db 100644 --- a/content/svg/content/src/Makefile.in +++ b/content/svg/content/src/Makefile.in @@ -178,6 +178,7 @@ EXPORTS = \ SVGAnimatedPreserveAspectRatio.h \ SVGLength.h \ SVGLengthList.h \ + SVGNumberList.h \ $(NULL) include $(topsrcdir)/config/rules.mk diff --git a/content/svg/content/src/nsSVGElement.cpp b/content/svg/content/src/nsSVGElement.cpp index 00641427ded6..ac98f0be7c71 100644 --- a/content/svg/content/src/nsSVGElement.cpp +++ b/content/svg/content/src/nsSVGElement.cpp @@ -349,6 +349,10 @@ nsSVGElement::ParseAttribute(PRInt32 aNamespaceID, rv = numberListInfo.mNumberLists[i].SetBaseValueString(aValue); if (NS_FAILED(rv)) { numberListInfo.Reset(i); + } else { + aResult.SetTo(numberListInfo.mNumberLists[i].GetBaseValue(), + &aValue); + didSetResult = true; } foundMatch = true; break; @@ -657,8 +661,8 @@ nsSVGElement::UnsetAttrInternal(PRInt32 aNamespaceID, nsIAtom* aName, for (PRUint32 i = 0; i < numberListInfo.mNumberListCount; i++) { if (aName == *numberListInfo.mNumberListInfo[i].mName) { + MaybeSerializeAttrBeforeRemoval(aName, aNotify); numberListInfo.Reset(i); - DidChangeNumberList(i, false); return; } } @@ -1687,24 +1691,28 @@ nsSVGElement::NumberListAttributesInfo::Reset(PRUint8 aAttrEnum) // caller notifies } -void -nsSVGElement::DidChangeNumberList(PRUint8 aAttrEnum, bool aDoSetAttr) +nsAttrValue +nsSVGElement::WillChangeNumberList(PRUint8 aAttrEnum) { - if (!aDoSetAttr) - return; + return WillChangeValue(*GetNumberListInfo().mNumberListInfo[aAttrEnum].mName); +} +void +nsSVGElement::DidChangeNumberList(PRUint8 aAttrEnum, + const nsAttrValue& aEmptyOrOldValue) +{ NumberListAttributesInfo info = GetNumberListInfo(); NS_ABORT_IF_FALSE(info.mNumberListCount > 0, - "DidChangeNumberList on element with no number list attribs"); - NS_ABORT_IF_FALSE(aAttrEnum < info.mNumberListCount, "aAttrEnum out of range"); + "DidChangeNumberList on element with no number list attribs"); + NS_ABORT_IF_FALSE(aAttrEnum < info.mNumberListCount, + "aAttrEnum out of range"); - nsAutoString serializedValue; - info.mNumberLists[aAttrEnum].GetBaseValue().GetValueAsString(serializedValue); + nsAttrValue newValue; + newValue.SetTo(info.mNumberLists[aAttrEnum].GetBaseValue(), nsnull); - nsAttrValue attrValue(serializedValue); - SetParsedAttr(kNameSpaceID_None, *info.mNumberListInfo[aAttrEnum].mName, - nsnull, attrValue, true); + DidChangeValue(*info.mNumberListInfo[aAttrEnum].mName, aEmptyOrOldValue, + newValue); } void diff --git a/content/svg/content/src/nsSVGElement.h b/content/svg/content/src/nsSVGElement.h index f0408eec410b..98550482d552 100644 --- a/content/svg/content/src/nsSVGElement.h +++ b/content/svg/content/src/nsSVGElement.h @@ -176,6 +176,7 @@ class nsSVGElement : public nsSVGElementBase // nsIContent nsAttrValue WillChangeAngle(PRUint8 aAttrEnum); nsAttrValue WillChangeViewBox(); nsAttrValue WillChangePreserveAspectRatio(); + nsAttrValue WillChangeNumberList(PRUint8 aAttrEnum); nsAttrValue WillChangeLengthList(PRUint8 aAttrEnum); void DidChangeLength(PRUint8 aAttrEnum, const nsAttrValue& aEmptyOrOldValue); @@ -190,7 +191,8 @@ class nsSVGElement : public nsSVGElementBase // nsIContent void DidChangeEnum(PRUint8 aAttrEnum); void DidChangeViewBox(const nsAttrValue& aEmptyOrOldValue); void DidChangePreserveAspectRatio(const nsAttrValue& aEmptyOrOldValue); - virtual void DidChangeNumberList(PRUint8 aAttrEnum, bool aDoSetAttr); + void DidChangeNumberList(PRUint8 aAttrEnum, + const nsAttrValue& aEmptyOrOldValue); void DidChangeLengthList(PRUint8 aAttrEnum, const nsAttrValue& aEmptyOrOldValue); virtual void DidChangePointList(bool aDoSetAttr); diff --git a/content/svg/content/test/Makefile.in b/content/svg/content/test/Makefile.in index 261e9b3a2b7b..a0e9a94b40f5 100644 --- a/content/svg/content/test/Makefile.in +++ b/content/svg/content/test/Makefile.in @@ -90,6 +90,7 @@ _TEST_FILES = \ test_SVGLengthList.xhtml \ test_SVGLengthList-2.xhtml \ test_SVGMatrix.xhtml \ + test_SVGNumberList.xhtml \ test_SVGPathSegList.xhtml \ test_SVGStyleElement.xhtml \ test_SVGStringList.xhtml \ diff --git a/content/svg/content/test/test_SVGNumberList.xhtml b/content/svg/content/test/test_SVGNumberList.xhtml new file mode 100644 index 000000000000..fcd471f4c4f2 --- /dev/null +++ b/content/svg/content/test/test_SVGNumberList.xhtml @@ -0,0 +1,64 @@ + + + + Tests specific to SVGNumberList + + + + + +Mozilla Bug 629200 +

+ +
+
+
+ +