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 String& localName)

[Inactive, 2014/03/10 19:56:23]

add "const Element& element" argument.

+{
+    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')

[Inactive, 2014/03/10 19:56:23]

add "!element.isSVGElement() && " condition.

+        return HTMLNames::classAttr;

[Inactive, 2014/03/10 18:58:26]

Technically, I believe we cannot do a fast lookup for the "class" attribute for
SVG elements either, because it is animated:
Source/core/svg/SVGElement.idl:    [MeasureAs=SVGClassName] readonly attribute
SVGAnimatedString className;

I believe you are likely breaking getAttribute("class") on SVG elements.

+
+    return nullQName();
+}
+
+const AtomicString& Element::bindingsGetAttribute(const String& localName) const
+{
+    const QualifiedName& fastName = fastAttributeNameFromString(localName);

[Inactive, 2014/03/10 19:56:23]

fastAttributeNameFromString(*this, localName);

+    if (fastName != nullQName())
+        return fastGetAttribute(fastName);
+    return getAttribute(AtomicString(localName));
+}
+
+void Element::bindingsSetAttribute(const String& localName, const String& newValue, ExceptionState& exceptionState)

[Inactive, 2014/03/10 18:46:45]

I think I made this comment before but newValue should really be an const
AtomicString& here, not a const String& as we always use it as an AtomicString.
It makes little sense for the bindings to construct a String if it will anyway
be converted into an AtomicString internally.

[Inactive, 2014/03/10 20:04:15]

To be clear, I only want newValue to be an AtomicString. Keeping localName as a
String is fine. Unlike WebKit, our AtomicString(const String&) constructor is
explicit so it is more obvious when we are converting a String to an
AtomicString.

+{
+    const QualifiedName& fastName = fastAttributeNameFromString(localName);

[Inactive, 2014/03/10 19:56:23]

fastAttributeNameFromString(*this, localName);

+    if (fastName != nullQName()) {
+        setAttribute(fastName, AtomicString(newValue));
+        return;
+    }
+    setAttribute(AtomicString(localName), AtomicString(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 2004 matching lines @@
         }
     }
 
     return true;
 }
 
 #ifndef NDEBUG
 bool Element::fastAttributeLookupAllowed(const QualifiedName& name) const
 {
     if (name == HTMLNames::styleAttr)
         return false;

[dshwang, 2014/03/10 18:43:20]

Blink does not support styleAttr fast path.

 
     if (isSVGElement())
         return !toSVGElement(this)->isAnimatableAttribute(name);

[dshwang, 2014/03/10 19:48:59]

Thanks Chris. here is svg consideration.

How about this code? Can this address your concern?
const QualifiedName& fastName = fastAttributeNameFromString(localName);
     if (fastName != nullQName() && fastAttributeLookupAllowed(fastName ))
         return fastGetAttribute(fastName)

[Inactive, 2014/03/10 19:56:23]

No, fastAttributeLookupAllowed() is only available in debug builds and would be
too expensible to call in your fast path.

 
     return true;
 }
 #endif
 
 #ifdef DUMP_NODE_STATISTICS
 bool Element::hasNamedNodeMap() const
 {
     return hasRareData() && elementRareData()->attributeMap();
 }
@@ skipping 591 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