Skip to content

Commit

Permalink
Bug 629200 part 19 - Remove unnecessary serialisation from setting ns…
Browse files Browse the repository at this point in the history 
…SVGNumberList; r=jwatt
  • Loading branch information
@birtles
birtles committed Feb 15, 2012
1 parent 079a49e commit 9b7dedb
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 21 deletions.
31 changes: 31 additions & 0 deletions content/base/src/nsAttrValue.cpp
View file
Expand Up @@ -58,6 +58,7 @@
#include "nsSVGViewBox.h"
#include "SVGAnimatedPreserveAspectRatio.h"
#include "SVGLengthList.h"
#include "SVGNumberList.h"

namespace css = mozilla::css;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 8 additions & 3 deletions content/base/src/nsAttrValue.h
View file
Expand Up @@ -70,6 +70,7 @@ class StyleRule;
}
class SVGAnimatedPreserveAspectRatio;
class SVGLengthList;
class SVGNumberList;
}

#define NS_ATTRVALUE_MAX_STRINGLENGTH_ATOM 12
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion content/svg/content/src/DOMSVGNumber.cpp
View file
Expand Up @@ -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();
}
Expand Down
12 changes: 8 additions & 4 deletions content/svg/content/src/DOMSVGNumberList.cpp
View file
Expand Up @@ -171,14 +171,15 @@ 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:
mAList->InternalBaseValListWillChangeTo(SVGNumberList());

mItems.Clear();
InternalList().Clear();
Element()->DidChangeNumberList(AttrEnum(), true);
Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue);
if (mAList->IsAnimating()) {
Element()->AnimationNeedsResample();
}
Expand Down Expand Up @@ -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);

Expand All @@ -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();
}
Expand Down Expand Up @@ -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:
Expand All @@ -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();
}
Expand Down Expand Up @@ -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();
Expand All @@ -350,7 +354,7 @@ DOMSVGNumberList::RemoveItem(PRUint32 index,

UpdateListIndicesFromIndex(mItems, index);

Element()->DidChangeNumberList(AttrEnum(), true);
Element()->DidChangeNumberList(AttrEnum(), emptyOrOldValue);
if (mAList->IsAnimating()) {
Element()->AnimationNeedsResample();
}
Expand Down
1 change: 1 addition & 0 deletions content/svg/content/src/Makefile.in
View file
Expand Up @@ -178,6 +178,7 @@ EXPORTS = \
SVGAnimatedPreserveAspectRatio.h \
SVGLength.h \
SVGLengthList.h \
SVGNumberList.h \
$(NULL)

include $(topsrcdir)/config/rules.mk
Expand Down
32 changes: 20 additions & 12 deletions content/svg/content/src/nsSVGElement.cpp
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion content/svg/content/src/nsSVGElement.h
View file
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions content/svg/content/test/Makefile.in
View file
Expand Up @@ -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 \
Expand Down
64 changes: 64 additions & 0 deletions content/svg/content/test/test_SVGNumberList.xhtml
View file
@@ -0,0 +1,64 @@
<html xmlns="http://www.w3.org/1999/xhtml">
<!--
https://bugzilla.mozilla.org/show_bug.cgi?id=629200
-->
<head>
<title>Tests specific to SVGNumberList</title>
<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
<script type="text/javascript" src="MutationEventChecker.js"></script>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
</head>
<body>
<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=629200">Mozilla Bug 629200</a>
<p id="display"></p>
<div id="content" style="display:none;">
<svg id="svg" xmlns="http://www.w3.org/2000/svg" width="100" height="100">
<text id="text" rotate="10 20 30">abc</text>
</svg>
</div>
<pre id="test">
<script class="testbody" type="text/javascript">
<![CDATA[

SimpleTest.waitForExplicitFinish();

/*
This file runs a series of SVGNumberList specific tests. Generic SVGXxxList
tests can be found in test_SVGxxxList.xhtml. Anything that can be generalized
to other list types belongs there.
*/

function run_tests()
{
document.getElementById('svg').pauseAnimations();

var text = document.getElementById("text");
var numbers = text.rotate.baseVal;

is(numbers.numberOfItems, 3, 'Checking numberOfItems');

// Test mutation events
// --- Initialization
eventChecker = new MutationEventChecker;
eventChecker.watchAttr(text, "rotate");
// -- Actual changes
eventChecker.expect("modify modify");
numbers[0].value = 15;
text.setAttribute("rotate", "17 20 30");
// -- Redundant changes
eventChecker.expect("");
numbers[0].value = 17;
numbers[1].value = 20;
text.setAttribute("rotate", "17 20 30");
eventChecker.finish();

SimpleTest.finish();
}

window.addEventListener("load", run_tests, false);

]]>
</script>
</pre>
</body>
</html>

0 comments on commit 9b7dedb

Please sign in to comment.