WontFix: Add fast path for id/name/style/class in getAttribute/setAttribute bindings.

Source/core/dom/Element.cpp

Index: Source/core/dom/Element.cpp
diff --git a/Source/core/dom/Element.cpp b/Source/core/dom/Element.cpp
index bb3e0aad6f5ef42a05593dfba29040649ea60d5c..29029690d8e897d2178a7f669d672c7acd400fe3 100644
--- a/Source/core/dom/Element.cpp
+++ b/Source/core/dom/Element.cpp
@@ -908,6 +908,60 @@ IntRect Element::screenRect() const
     return document().view()->contentsToScreen(renderer()->absoluteBoundingBoxRectIgnoringTransforms());
 }
 
+static inline const QualifiedName& fastAttributeNameFromString(const Element& element, const AtomicString& localName)
+{
+    unsigned length = localName.length();
+    if (!length)
+        return nullQName();
+    const StringImpl& name = *localName.impl();
+
+    if (length == 2 && name[0] == 'i' && name[1] == 'd')

[Inactive, 2014/03/11 13:48:33]

if localName if an AtomicString, checking its characters no longer makes sense
as you could simply compare AtomicStrings (pointer comparison).

+        return HTMLNames::idAttr;
+    if (length == 4 && name[0] == 'n' && name[1] == 'a' && name[2] == 'm' && name[3] == 'e')
+        return HTMLNames::nameAttr;
+    if (!element.isSVGElement() && length == 5 && name[0] == 'c' && name[1] == 'l' && name[2] == 'a' && name[3] == 's' && name[4] == 's')
+        return HTMLNames::classAttr;
+
+    return nullQName();
+}
+
+const AtomicString& Element::bindingsGetAttribute(const AtomicString& localName) const

[Inactive, 2014/03/11 13:48:33]

Based on the Changelog on WebKit side, part of the optimization was to avoid
AtomicString lookup for the commonly used attribute names. However, now that you
are using AtomicString type for localName, this optimization does not make sense
anymore (you already did the AtomicString lookup).

+{
+    const QualifiedName& fastName = fastAttributeNameFromString(*this, localName);
+    if (fastName != nullQName())
+        return fastGetAttribute(fastName);
+    return getAttribute(localName);
+}
+
+static inline const QualifiedName& principalAttributeNameFromString(const AtomicString& localName)

[Inactive, 2014/03/11 13:48:33]

Sorry, I don't like the fact that we now have 2 functions doing almost the same
thing and thus a bit of code duplication. I understand you are trying to handle
the attribute setting case more efficiently since we don't have a
fastSetAttribute() on Blink-side. However, we need to find a better way to
achieve this.

+{
+    unsigned length = localName.length();
+    if (!length)
+        return nullQName();
+    const StringImpl& name = *localName.impl();
+
+    if (length == 2 && name[0] == 'i' && name[1] == 'd')
+        return HTMLNames::idAttr;
+    if (length == 4 && name[0] == 'n' && name[1] == 'a' && name[2] == 'm' && name[3] == 'e')
+        return HTMLNames::nameAttr;
+    if (length == 5 && name[0] == 'c' && name[1] == 'l' && name[2] == 'a' && name[3] == 's' && name[4] == 's')
+        return HTMLNames::classAttr;
+    if (length == 5 && name[0] == 's' && name[1] == 't' && name[2] == 'y' && name[3] == 'l' && name[4] == 'e')
+        return HTMLNames::styleAttr;
+
+    return nullQName();
+}
+
+void Element::bindingsSetAttribute(const AtomicString& localName, const AtomicString& newValue, ExceptionState& exceptionState)

[Inactive, 2014/03/11 13:48:33]

localName should be a String. I only asked for newValue to be an AtomicString
and I explicitly said that localName should remain a String.

+{
+    const QualifiedName& principalAttributeName = principalAttributeNameFromString(localName);
+    if (principalAttributeName != nullQName()) {
+        setAttribute(principalAttributeName, AtomicString(newValue));

[Inactive, 2014/03/11 13:48:33]

Useless AtomicString().

Note that WebKit has a fastSetAttribute() as well. We may consider porting this
patch as well if we see an impact on perf tests.

+        return;
+    }
+    setAttribute(localName, newValue, exceptionState);
+}
+
 const AtomicString& Element::getAttribute(const AtomicString& localName) const
 {
     if (!elementData())