[webkit-reviews] review granted: [Bug 19002] Optimize id selectors in querySelector/querySelectorAll : [Attachment 21940] Now with tests and less bugs

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted David Smith <catfish.man at gmail.com>'s
request for review:
Bug 19002: Optimize id selectors in querySelector/querySelectorAll
https://bugs.webkit.org/show_bug.cgi?id=19002

Attachment 21940: Now with tests and less bugs
https://bugs.webkit.org/attachment.cgi?id=21940&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
Please include the speedup and on what test you tested it on.

You can probably put this with the other Id related methods (getElementById,
and hasElementWithId).
+    bool containsMultipleElementsWithId(const AtomicString& elementId) {
return m_duplicateIds.contains(elementId.impl()); }

The parens around (querySelector->next()) are not needed.
+    if (!(querySelector->next()) && querySelector->m_match == CSSSelector::Id)
{

Here too.
+    if (!(querySelector->next()) && querySelector->m_match == CSSSelector::Id
&& !document->containsMultipleElementsWithId(selectorValue)) {

You can remove the else here and and just return 0.
+	 if (element && (isDocumentNode() || element->isDescendantOf(this)))
+	     return element;
+    } else {
+	 // FIXME: We can speed this up by implementing caching similar to the
one use by getElementById


r=me