- Revision
- 103140
- Author
- [email protected]
- Date
- 2011-12-16 20:36:34 -0800 (Fri, 16 Dec 2011)
Log Message
Consolidate before-advice regarding attribute modification into a single method
https://bugs.webkit.org/show_bug.cgi?id=74752
Reviewed by Ryosuke Niwa.
Adds a willModifyAttribute method to Element, meant to be called
before an attribute on that Element is added/removed/changed.
Replace most calls to Element::updateId and all calls to
Element::enqueueAttributesMutationRecordIfRequested with calls to
willModifyAttribute. Moreover, enqueueAttributesMutation... can now
be private since its only caller is willModifyAttribute.
The only remaining direct calls to updateId are in cases the entire
NamedNodeMap is being replaced. These are implementation details of
WebCore that shouldn't be exposed via MutationObservers.
No new tests, no expected change in behavior.
* dom/Attr.cpp:
(WebCore::Attr::setValue):
(WebCore::Attr::childrenChanged): Besides the above change, use a
StringBuilder to build up value, and only do String -> AtomicString
conversion once.
* dom/Element.cpp:
(WebCore::Element::setAttributeInternal):
* dom/Element.h:
(WebCore::Element::willModifyAttribute):
* dom/NamedNodeMap.cpp:
(WebCore::NamedNodeMap::setNamedItem):
(WebCore::NamedNodeMap::removeNamedItem):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (103139 => 103140)
--- trunk/Source/WebCore/ChangeLog 2011-12-17 03:46:17 UTC (rev 103139)
+++ trunk/Source/WebCore/ChangeLog 2011-12-17 04:36:34 UTC (rev 103140)
@@ -1,3 +1,37 @@
+2011-12-16 Adam Klein <[email protected]>
+
+ Consolidate before-advice regarding attribute modification into a single method
+ https://bugs.webkit.org/show_bug.cgi?id=74752
+
+ Reviewed by Ryosuke Niwa.
+
+ Adds a willModifyAttribute method to Element, meant to be called
+ before an attribute on that Element is added/removed/changed.
+
+ Replace most calls to Element::updateId and all calls to
+ Element::enqueueAttributesMutationRecordIfRequested with calls to
+ willModifyAttribute. Moreover, enqueueAttributesMutation... can now
+ be private since its only caller is willModifyAttribute.
+
+ The only remaining direct calls to updateId are in cases the entire
+ NamedNodeMap is being replaced. These are implementation details of
+ WebCore that shouldn't be exposed via MutationObservers.
+
+ No new tests, no expected change in behavior.
+
+ * dom/Attr.cpp:
+ (WebCore::Attr::setValue):
+ (WebCore::Attr::childrenChanged): Besides the above change, use a
+ StringBuilder to build up value, and only do String -> AtomicString
+ conversion once.
+ * dom/Element.cpp:
+ (WebCore::Element::setAttributeInternal):
+ * dom/Element.h:
+ (WebCore::Element::willModifyAttribute):
+ * dom/NamedNodeMap.cpp:
+ (WebCore::NamedNodeMap::setNamedItem):
+ (WebCore::NamedNodeMap::removeNamedItem):
+
2011-12-16 James Robinson <[email protected]>
[chromium] CCLayerDelegate and WebLayerClient do not need notifySyncRequired
Modified: trunk/Source/WebCore/dom/Attr.cpp (103139 => 103140)
--- trunk/Source/WebCore/dom/Attr.cpp 2011-12-17 03:46:17 UTC (rev 103139)
+++ trunk/Source/WebCore/dom/Attr.cpp 2011-12-17 04:36:34 UTC (rev 103140)
@@ -29,6 +29,8 @@
#include "ScopedEventQueue.h"
#include "Text.h"
#include "XMLNSNames.h"
+#include <wtf/text/AtomicString.h>
+#include <wtf/text/StringBuilder.h>
namespace WebCore {
@@ -132,14 +134,9 @@
void Attr::setValue(const AtomicString& value, ExceptionCode&)
{
-#if ENABLE(MUTATION_OBSERVERS)
if (m_element)
- m_element->enqueueAttributesMutationRecordIfRequested(m_attribute->name(), m_attribute->value());
-#endif
+ m_element->willModifyAttribute(m_attribute->name(), m_attribute->value(), value);
- if (m_element && m_element->isIdAttributeName(m_attribute->name()))
- m_element->updateId(m_element->getIdAttribute(), value);
-
setValue(value);
if (m_element)
@@ -175,27 +172,23 @@
if (m_ignoreChildrenChanged > 0)
return;
-#if ENABLE(MUTATION_OBSERVERS)
- if (m_element)
- m_element->enqueueAttributesMutationRecordIfRequested(m_attribute->name(), m_attribute->value());
-#endif
-
Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
invalidateNodeListsCacheAfterAttributeChanged(m_attribute->name());
// FIXME: We should include entity references in the value
- String val = "";
+ StringBuilder valueBuilder;
for (Node *n = firstChild(); n; n = n->nextSibling()) {
if (n->isTextNode())
- val += static_cast<Text *>(n)->data();
+ valueBuilder.append(static_cast<Text*>(n)->data());
}
- if (m_element && m_element->isIdAttributeName(m_attribute->name()))
- m_element->updateId(m_attribute->value(), val);
+ AtomicString newValue = valueBuilder.toString();
+ if (m_element)
+ m_element->willModifyAttribute(m_attribute->name(), m_attribute->value(), newValue);
- m_attribute->setValue(val.impl());
+ m_attribute->setValue(newValue);
if (m_element)
m_element->attributeChanged(m_attribute.get());
}
Modified: trunk/Source/WebCore/dom/Element.cpp (103139 => 103140)
--- trunk/Source/WebCore/dom/Element.cpp 2011-12-17 03:46:17 UTC (rev 103139)
+++ trunk/Source/WebCore/dom/Element.cpp 2011-12-17 04:36:34 UTC (rev 103140)
@@ -655,14 +655,9 @@
InspectorInstrumentation::willModifyDOMAttr(document(), this);
#endif
-#if ENABLE(MUTATION_OBSERVERS)
- enqueueAttributesMutationRecordIfRequested(name, old ? old->value() : nullAtom);
-#endif
-
document()->incDOMTreeVersion();
- if (isIdAttributeName(name))
- updateId(old ? old->value() : nullAtom, value);
+ willModifyAttribute(name, old ? old->value() : nullAtom, value);
if (old && value.isNull())
m_attributeMap->removeAttribute(name);
Modified: trunk/Source/WebCore/dom/Element.h (103139 => 103140)
--- trunk/Source/WebCore/dom/Element.h 2011-12-17 03:46:17 UTC (rev 103139)
+++ trunk/Source/WebCore/dom/Element.h 2011-12-17 04:36:34 UTC (rev 103140)
@@ -266,6 +266,7 @@
virtual String title() const;
void updateId(const AtomicString& oldId, const AtomicString& newId);
+ void willModifyAttribute(const QualifiedName&, const AtomicString& oldValue, const AtomicString& newValue);
LayoutSize minimumSizeForResizing() const;
void setMinimumSizeForResizing(const LayoutSize&);
@@ -364,10 +365,6 @@
PassRefPtr<RenderStyle> styleForRenderer();
-#if ENABLE(MUTATION_OBSERVERS)
- void enqueueAttributesMutationRecordIfRequested(const QualifiedName&, const AtomicString& oldValue);
-#endif
-
protected:
Element(const QualifiedName& tagName, Document* document, ConstructionType type)
: ContainerNode(document, type)
@@ -436,6 +433,10 @@
SpellcheckAttributeState spellcheckAttributeState() const;
+#if ENABLE(MUTATION_OBSERVERS)
+ void enqueueAttributesMutationRecordIfRequested(const QualifiedName&, const AtomicString& oldValue);
+#endif
+
private:
mutable RefPtr<NamedNodeMap> m_attributeMap;
};
@@ -511,6 +512,18 @@
scope->addElementById(newId, this);
}
+inline void Element::willModifyAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue)
+{
+ if (isIdAttributeName(name))
+ updateId(oldValue, newValue);
+
+ // FIXME: Should probably call InspectorInstrumentation::willModifyDOMAttr here.
+
+#if ENABLE(MUTATION_OBSERVERS)
+ enqueueAttributesMutationRecordIfRequested(name, oldValue);
+#endif
+}
+
inline bool Element::fastHasAttribute(const QualifiedName& name) const
{
ASSERT(fastAttributeLookupAllowed(name));
Modified: trunk/Source/WebCore/dom/NamedNodeMap.cpp (103139 => 103140)
--- trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-17 03:46:17 UTC (rev 103139)
+++ trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-17 04:36:34 UTC (rev 103140)
@@ -119,13 +119,8 @@
return 0;
}
-#if ENABLE(MUTATION_OBSERVERS)
- m_element->enqueueAttributesMutationRecordIfRequested(attribute->name(), oldAttribute ? oldAttribute->value() : nullAtom);
-#endif
+ m_element->willModifyAttribute(attribute->name(), oldAttribute ? oldAttribute->value() : nullAtom, attribute->value());
- if (attr->isId())
- m_element->updateId(oldAttribute ? oldAttribute->value() : nullAtom, attribute->value());
-
// ### slightly inefficient - resizes attribute array twice.
RefPtr<Attr> oldAttr;
if (oldAttribute) {
@@ -147,21 +142,17 @@
// because of removeNamedItem, removeNamedItemNS, and removeAttributeNode.
PassRefPtr<Node> NamedNodeMap::removeNamedItem(const QualifiedName& name, ExceptionCode& ec)
{
+ ASSERT(m_element);
+
Attribute* attribute = getAttributeItem(name);
if (!attribute) {
ec = NOT_FOUND_ERR;
return 0;
}
-#if ENABLE(MUTATION_OBSERVERS)
- if (m_element)
- m_element->enqueueAttributesMutationRecordIfRequested(attribute->name(), attribute->value());
-#endif
-
RefPtr<Attr> attr = attribute->createAttrIfNeeded(m_element);
- if (attr->isId())
- m_element->updateId(attribute->value(), nullAtom);
+ m_element->willModifyAttribute(attribute->name(), attribute->value(), nullAtom);
removeAttribute(name);
return attr.release();
