commit d1564869fee9301fce0674f44c01bee6d9be10cd
parent 81f86c8b2f7e9ac0b65eeb9e066176a35843a78a
Author: Emilio Cobos Álvarez <emilio@crisal.io>
Date: Tue, 30 Dec 2025 20:45:21 +0000
Bug 2007958 - Use SetParsedAttr for XUL prototype cache. r=smaug
This technically does a bit more work (calling AfterSetAttr), but no
more work than we do for HTML.
I got bit by this while trying to make some XUL attributes (checked /
disabled) work more like their HTML equivalent and their respective
states, because AfterSetAttr doesn't get called otherwise.
Rather than adding more and more special-cases here, I think we should
just take the simplification. I doubt the perf hit is particularly
meaningful nowadays, but I want to land this first to make sure we don't
regress any benchmark.
Declaration blocks don't need to be cloned anymore afaict (but also now
we disallow inline styles by CSP in virtually all the browser chrome).
Differential Revision: https://phabricator.services.mozilla.com/D277624
Diffstat:
3 files changed, 11 insertions(+), 70 deletions(-)
diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp
@@ -235,26 +235,7 @@ already_AddRefed<Element> nsXULElement::CreateFromPrototype(
}
nsXULElement* element = FromNode(baseElement);
-
- if (aPrototype->mHasIdAttribute) {
- element->SetHasID();
- }
- if (aPrototype->mHasClassAttribute) {
- element->SetMayHaveClass();
- }
- if (aPrototype->mHasStyleAttribute) {
- element->SetMayHaveStyle();
- }
-
element->MakeHeavyweight(aPrototype);
-
- // Check each attribute on the prototype to see if we need to do
- // any additional processing and hookup that would otherwise be
- // done 'automagically' by SetAttr().
- for (const auto& attribute : aPrototype->mAttributes) {
- element->AddListenerForAttributeIfNeeded(attribute.mName);
- }
-
return baseElement.forget();
}
@@ -520,12 +501,6 @@ void nsXULElement::AddListenerForAttributeIfNeeded(nsAtom* aLocalName) {
}
}
-void nsXULElement::AddListenerForAttributeIfNeeded(const nsAttrName& aName) {
- if (aName.IsAtom()) {
- AddListenerForAttributeIfNeeded(aName.Atom());
- }
-}
-
class XULInContentErrorReporter : public Runnable {
public:
explicit XULInContentErrorReporter(Document& aDocument)
@@ -1020,37 +995,12 @@ nsresult nsXULElement::AddPopupListener(nsAtom* aName) {
//----------------------------------------------------------------------
nsresult nsXULElement::MakeHeavyweight(nsXULPrototypeElement* aPrototype) {
- if (!aPrototype) {
- return NS_OK;
- }
-
- size_t i;
- nsresult rv;
- for (i = 0; i < aPrototype->mAttributes.Length(); ++i) {
- nsXULPrototypeAttribute* protoattr = &aPrototype->mAttributes[i];
- nsAttrValue attrValue;
-
- // Style rules need to be cloned.
- if (protoattr->mValue.Type() == nsAttrValue::eCSSDeclaration) {
- DeclarationBlock* decl = protoattr->mValue.GetCSSDeclarationValue();
- RefPtr<DeclarationBlock> declClone = decl->Clone();
-
- nsString stringValue;
- protoattr->mValue.ToString(stringValue);
-
- attrValue.SetTo(declClone.forget(), &stringValue);
- } else {
- attrValue.SetTo(protoattr->mValue);
- }
-
- bool oldValueSet;
- // XXX we might wanna have a SetAndTakeAttr that takes an nsAttrName
- if (protoattr->mName.IsAtom()) {
- rv = SetAndSwapAttr(protoattr->mName.Atom(), attrValue, &oldValueSet);
- } else {
- rv = SetAndSwapAttr(protoattr->mName.NodeInfo(), attrValue, &oldValueSet);
- }
- NS_ENSURE_SUCCESS(rv, rv);
+ MOZ_ASSERT(aPrototype);
+ for (const auto& protoattr : aPrototype->mAttributes) {
+ nsAttrValue value(protoattr.mValue);
+ MOZ_TRY(SetParsedAttr(
+ protoattr.mName.NamespaceID(), protoattr.mName.LocalName(),
+ protoattr.mName.GetPrefix(), value, /* aNotify = */ false));
}
return NS_OK;
}
@@ -1378,7 +1328,6 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
}
if (mAttributes[aPos].mName.Equals(nsGkAtoms::id) && !aValue.IsEmpty()) {
- mHasIdAttribute = true;
// Store id as atom.
// id="" means that the element has no id. Not that it has
// emptystring as id.
@@ -1396,13 +1345,11 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
return NS_OK;
} else if (mAttributes[aPos].mName.Equals(nsGkAtoms::_class)) {
- mHasClassAttribute = true;
// Compute the element's class list
mAttributes[aPos].mValue.ParseAtomArray(aValue);
return NS_OK;
} else if (mAttributes[aPos].mName.Equals(nsGkAtoms::style)) {
- mHasStyleAttribute = true;
// Parse the element's 'style' attribute
// This is basically duplicating what nsINode::NodePrincipal() does
@@ -1419,6 +1366,7 @@ nsresult nsXULPrototypeElement::SetAttrAt(uint32_t aPos,
aValue, data, eCompatibility_FullStandards, nullptr,
StyleCssRuleType::Style);
if (declaration) {
+ declaration->SetImmutable();
mAttributes[aPos].mValue.SetTo(declaration.forget(), &aValue);
return NS_OK;
diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h
@@ -168,9 +168,6 @@ class nsXULPrototypeElement : public nsXULPrototypeNode {
explicit nsXULPrototypeElement(mozilla::dom::NodeInfo* aNodeInfo = nullptr)
: nsXULPrototypeNode(eType_Element),
mNodeInfo(aNodeInfo),
- mHasIdAttribute(false),
- mHasClassAttribute(false),
- mHasStyleAttribute(false),
mIsAtom(nullptr) {}
private:
@@ -202,9 +199,6 @@ class nsXULPrototypeElement : public nsXULPrototypeNode {
RefPtr<mozilla::dom::NodeInfo> mNodeInfo;
- uint32_t mHasIdAttribute : 1;
- uint32_t mHasClassAttribute : 1;
- uint32_t mHasStyleAttribute : 1;
nsTArray<nsXULPrototypeAttribute> mAttributes; // [OWNER]
RefPtr<nsAtom> mIsAtom;
};
@@ -524,7 +518,6 @@ class nsXULElement : public nsStyledElement {
/**
* Add a listener for the specified attribute, if appropriate.
*/
- void AddListenerForAttributeIfNeeded(const nsAttrName& aName);
void AddListenerForAttributeIfNeeded(nsAtom* aLocalName);
protected:
diff --git a/dom/xul/nsXULPrototypeDocument.cpp b/dom/xul/nsXULPrototypeDocument.cpp
@@ -429,9 +429,6 @@ void nsXULPrototypeDocument::SetIsL10nCached(bool aIsCached) {
void nsXULPrototypeDocument::RebuildPrototypeFromElement(
nsXULPrototypeElement* aPrototype, Element* aElement, bool aDeep) {
- aPrototype->mHasIdAttribute = aElement->HasID();
- aPrototype->mHasClassAttribute = aElement->MayHaveClass();
- aPrototype->mHasStyleAttribute = aElement->MayHaveStyle();
NodeInfo* oldNodeInfo = aElement->NodeInfo();
RefPtr<NodeInfo> newNodeInfo = mNodeInfoManager->GetNodeInfo(
oldNodeInfo->NameAtom(), oldNodeInfo->GetPrefixAtom(),
@@ -457,7 +454,10 @@ void nsXULPrototypeDocument::RebuildPrototypeFromElement(
protoAttr->mName.SetTo(newNodeInfo);
}
protoAttr->mValue.SetTo(*attr.mValue);
-
+ if (protoAttr->mValue.Type() == nsAttrValue::eCSSDeclaration) {
+ // Ensure declarations get copied-on-write if needed.
+ protoAttr->mValue.GetCSSDeclarationValue()->SetImmutable();
+ }
protoAttr++;
}
