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

Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc. All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 890 matching lines @@
 }
 
 IntRect Element::screenRect() const
 {
     if (!renderer())
         return IntRect();
     // FIXME: this should probably respect transforms
     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())
         return nullAtom;
     synchronizeAttribute(localName);
     if (const Attribute* attribute = elementData()->getAttributeItem(localName, shouldIgnoreAttributeCase()))
         return attribute->value();
     return nullAtom;
 }
 
@@ skipping 2619 matching lines @@
     // Before doing so, we need to resolve issues in HTMLSelectElement::recalcListItems
     // and RenderMenuList::setText. See also https://bugs.webkit.org/show_bug.cgi?id=88405
     if (hasTagName(optionTag) || hasTagName(optgroupTag))
         return false;
     if (FullscreenElementStack::isActiveFullScreenElement(this))
         return false;
     return true;
 }
 
 } // namespace WebCore