Simply SVGSVGElement::getElementById method

Source/core/svg/SVGSVGElement.cpp

Index: Source/core/svg/SVGSVGElement.cpp
diff --git a/Source/core/svg/SVGSVGElement.cpp b/Source/core/svg/SVGSVGElement.cpp
index 30cf7fa27e2e365a56436721ca0275a31bc85b6a..9d701fc76d015771c73f01323e514c0bc8cc333f 100644
--- a/Source/core/svg/SVGSVGElement.cpp
+++ b/Source/core/svg/SVGSVGElement.cpp
@@ -767,20 +767,14 @@ void SVGSVGElement::inheritViewAttributes(SVGViewElement* viewElement)
 // See http://www.w3.org/TR/SVG11/struct.html#InterfaceSVGSVGElement
 Element* SVGSVGElement::getElementById(const AtomicString& id) const
 {
-    Element* element = treeScope().getElementById(id);
-    if (element && element->isDescendantOf(this))
-        return element;
-
-    // Fall back to traversing our subtree. Duplicate ids are allowed, the first found will
-    // be returned.
-    for (Node* node = firstChild(); node; node = NodeTraversal::next(*node, this)) {
-        if (!node->isElementNode())
-            continue;
-
-        Element* element = toElement(node);
-        if (element->getIdAttribute() == id)
+    Vector<Element*> elements = treeScope().getAllElementsById(id);

[Inactive, 2014/02/24 20:12:42]

This may cause a performance regression in the non-duplicate id case, which is
the most common case. Your CL causes some unnecessary overhead in the single-id
case (Vector creation, some checks in getAllElementsById).

It would be safer to use TreeScope::containsMultipleElementsWithId() and use
getElementById() if it returns true (fast path). Then fallback to using
getAllElementsById() instead of traversing.

+
+    for (Vector<Element*>::iterator it = elements.begin(); it != elements.end(); ++it) {

[pdr., 2014/02/24 18:39:43]

I'm not sure how much it matters here, but I typically see the end() moved into
a local variable like so:

Vector<Element*>::iterator end = elements.end();
for (Vector<Element*>::iterator it = elements.begin(); it != end; ++it) {

I don't feel strongly about this though.

+        Element* element = *it;
+        if (element->isDescendantOf(this))
             return element;
     }
+
     return 0;
 }