Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

This commit is contained in:
Robert O'Callahan 2015-07-25 17:57:13 +12:00
parent 5fc079e9ea
commit b7cc251745
6 changed files with 42 additions and 12 deletions

View File

@ -2258,9 +2258,9 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
// Copy aParsedValue for later use since it will be lost when we call
// SetAndSwapMappedAttr below
nsAttrValue aValueForAfterSetAttr;
nsAttrValue valueForAfterSetAttr;
if (aCallAfterSetAttr) {
aValueForAfterSetAttr.SetTo(aParsedValue);
valueForAfterSetAttr.SetTo(aParsedValue);
}
bool hadValidDir = false;
@ -2287,6 +2287,11 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
rv = mAttrsAndChildren.SetAndSwapAttr(ni, aParsedValue);
}
// If the old value owns its own data, we know it is OK to keep using it.
const nsAttrValue* oldValue =
aParsedValue.StoresOwnData() ? &aParsedValue : &aOldValue;
NS_ENSURE_SUCCESS(rv, rv);
if (document || HasFlag(NODE_FORCE_XBL_BINDINGS)) {
@ -2300,8 +2305,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
nsIDocument* ownerDoc = OwnerDoc();
if (ownerDoc && GetCustomElementData()) {
nsCOMPtr<nsIAtom> oldValueAtom = aOldValue.GetAsAtom();
nsCOMPtr<nsIAtom> newValueAtom = aValueForAfterSetAttr.GetAsAtom();
nsCOMPtr<nsIAtom> oldValueAtom = oldValue->GetAsAtom();
nsCOMPtr<nsIAtom> newValueAtom = valueForAfterSetAttr.GetAsAtom();
LifecycleCallbackArgs args = {
nsDependentAtomString(aName),
aModType == nsIDOMMutationEvent::ADDITION ?
@ -2313,11 +2318,11 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
}
if (aCallAfterSetAttr) {
rv = AfterSetAttr(aNamespaceID, aName, &aValueForAfterSetAttr, aNotify);
rv = AfterSetAttr(aNamespaceID, aName, &valueForAfterSetAttr, aNotify);
NS_ENSURE_SUCCESS(rv, rv);
if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) {
OnSetDirAttr(this, &aValueForAfterSetAttr,
OnSetDirAttr(this, &valueForAfterSetAttr,
hadValidDir, hadDirAuto, aNotify);
}
}
@ -2341,8 +2346,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
if (!newValue.IsEmpty()) {
mutation.mNewAttrValue = do_GetAtom(newValue);
}
if (!aOldValue.IsEmptyString()) {
mutation.mPrevAttrValue = aOldValue.GetAsAtom();
if (!oldValue->IsEmptyString()) {
mutation.mPrevAttrValue = oldValue->GetAsAtom();
}
mutation.mAttrChange = aModType;

View File

@ -477,6 +477,8 @@ public:
virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsIAtom* aPrefix,
const nsAString& aValue, bool aNotify) override;
// aParsedValue receives the old value of the attribute. That's useful if
// either the input or output value of aParsedValue is StoresOwnData.
nsresult SetParsedAttr(int32_t aNameSpaceID, nsIAtom* aName, nsIAtom* aPrefix,
nsAttrValue& aParsedValue, bool aNotify);
// GetAttr is not inlined on purpose, to keep down codesize from all
@ -1087,11 +1089,15 @@ protected:
* @param aNamespaceID namespace of attribute
* @param aAttribute local-name of attribute
* @param aPrefix aPrefix of attribute
* @param aOldValue previous value of attribute. Only needed if
* aFireMutation is true or if the element is a
* custom element (in web components).
* @param aOldValue The old value of the attribute to use as a fallback
* in the cases where the actual old value (i.e.
* its current value) is !StoresOwnData() --- in which
* case the current value is probably already useless.
* If the current value is StoresOwnData() (or absent),
* aOldValue will not be used.
* @param aParsedValue parsed new value of attribute. Replaced by the
* old value of the attribute.
* old value of the attribute. This old value is only
* useful if either it or the new value is StoresOwnData.
* @param aModType nsIDOMMutationEvent::MODIFICATION or ADDITION. Only
* needed if aFireMutation or aNotify is true.
* @param aFireMutation should mutation-events be fired?

View File

@ -88,6 +88,7 @@ public:
const nsAttrValue* GetAttr(const nsAString& aName,
nsCaseTreatment aCaseSensitive) const;
const nsAttrValue* AttrAt(uint32_t aPos) const;
// SetAndSwapAttr swaps the current attribute value with aValue.
nsresult SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue);
nsresult SetAndSwapAttr(mozilla::dom::NodeInfo* aName, nsAttrValue& aValue);

View File

@ -127,6 +127,12 @@ public:
static void Shutdown();
ValueType Type() const;
// Returns true when this value is self-contained and does not depend on
// the state of its associated element.
// Returns false when this value depends on the state of its associated
// element and may be invalid if that state has been changed by changes to
// that element state outside of attribute setting.
inline bool StoresOwnData() const;
void Reset();

View File

@ -191,6 +191,16 @@ nsAttrValue::IsSVGType(ValueType aType) const
return aType >= eSVGTypesBegin && aType <= eSVGTypesEnd;
}
inline bool
nsAttrValue::StoresOwnData() const
{
if (BaseType() != eOtherBase) {
return true;
}
ValueType t = Type();
return t != eCSSStyleRule && !IsSVGType(t);
}
inline void
nsAttrValue::SetPtrValueAndType(void* aValue, ValueBaseType aType)
{

View File

@ -347,6 +347,8 @@ protected:
mozilla::css::StyleRule* GetAnimatedContentStyleRule();
nsAttrValue WillChangeValue(nsIAtom* aName);
// aNewValue is set to the old value. This value may be invalid if
// !StoresOwnData.
void DidChangeValue(nsIAtom* aName, const nsAttrValue& aEmptyOrOldValue,
nsAttrValue& aNewValue);
void MaybeSerializeAttrBeforeRemoval(nsIAtom* aName, bool aNotify);