Simplify some arguments inside SelectorQuery.

third_party/WebKit/Source/core/dom/SelectorQuery.cpp

 /*
  * Copyright (C) 2011, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2014 Samsung Electronics. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 209 matching lines @@
     return false;
 
   for (Element* element = &toElement(rootNode); element;
        element = element->parentElement()) {
     if (element->hasClass() && element->classNames().contains(className))
       return true;
   }
   return false;
 }
 
-// If returns true, traversalRoots has the elements that may match the selector
-// query.
-//
-// If returns false, traversalRoots has the rootNode parameter or descendants of
-// rootNode representing the subtree for which we can limit the querySelector
-// traversal.
-//
-// The travseralRoots may be empty, regardless of the returned bool value, if
-// this method finds that the selectors won't match any element.

[esprehn, 2017/03/23 01:09:33]

This comment was ancient, from when we used to return the traverse roots and
some bools. The code doesn't work this way at all anymore so I removed it.

[sashab, 2017/03/23 05:40:21]

Wanna add an updated comment to explain how it works now? :)

[esprehn, 2017/03/23 05:48:51]

I'd prefer to do that in a separate patch.

 template <typename SelectorQueryTrait>
 void SelectorQuery::findTraverseRootsAndExecute(
     ContainerNode& rootNode,
     typename SelectorQueryTrait::OutputType& output) const {
   // We need to return the matches in document order. To use id lookup while
   // there is possiblity of multiple matches we would need to sort the
   // results. For now, just traverse the document in that case.
   DCHECK_EQ(m_selectors.size(), 1u);
 
   bool isRightmostSelector = true;
   bool startFromParent = false;
 
   for (const CSSSelector* selector = m_selectors[0]; selector;
        selector = selector->tagHistory()) {
     if (selector->match() == CSSSelector::Id && rootNode.isInTreeScope() &&
         !rootNode.containingTreeScope().containsMultipleElementsWithId(
             selector->value())) {
       Element* element =
           rootNode.containingTreeScope().getElementById(selector->value());
       ContainerNode* adjustedNode = &rootNode;
       if (element &&
           (isTreeScopeRoot(rootNode) || element->isDescendantOf(&rootNode)))
         adjustedNode = element;
       else if (!element || isRightmostSelector)
         adjustedNode = nullptr;
       if (isRightmostSelector) {
-        executeForTraverseRoot<SelectorQueryTrait>(
-            *m_selectors[0], adjustedNode, MatchesTraverseRoots, rootNode,
-            output);
+        if (!adjustedNode)
+          return;
+        element = toElement(adjustedNode);
+        if (selectorMatches(*m_selectors[0], *element, rootNode))
+          SelectorQueryTrait::appendElement(output, *element);

[sashab, 2017/03/23 06:26:32]

I don't really understand this part of the change, and why its equivalent to the
existing call to executeForTraverseRoot, but I'm assuming you do :)

[esprehn, 2017/03/23 06:31:58]

The code used to pass MatchedTraverseRoot as the enum value, which then went
down this branch:

  if (!traverseRoot)
    return;
  if (matchTraverseRoot) {
    if (selectorMatches(selector, toElement(*traverseRoot), rootNode))
      SelectorQueryTrait::appendElement(output, toElement(*traverseRoot));
    return;
  }

I inlined that code here.

[sashab, 2017/03/23 23:19:05]

Can you remove it from the function then? Do other callsites still need it?

[esprehn, 2017/03/24 00:02:31]

I did remove it from that function. There's a another function that's similarly
named: executeForTraverseRoots <-- plural with the "s" that still uses it
though.

         return;
       }
 
       if (startFromParent && adjustedNode)
         adjustedNode = adjustedNode->parentNode();
 
-      executeForTraverseRoot<SelectorQueryTrait>(*m_selectors[0], adjustedNode,
-                                                 DoesNotMatchTraverseRoots,
-                                                 rootNode, output);
+      executeForTraverseRoot<SelectorQueryTrait>(adjustedNode, rootNode,
+                                                 output);
       return;
     }
 
     // If we have both CSSSelector::Id and CSSSelector::Class at the same time,
     // we should use Id to find traverse root.
     if (!SelectorQueryTrait::shouldOnlyMatchFirstElement && !startFromParent &&
         selector->match() == CSSSelector::Class) {
       if (isRightmostSelector) {
         ClassElementList<AllElements> traverseRoots(rootNode,
                                                     selector->value());
         executeForTraverseRoots<SelectorQueryTrait>(
-            *m_selectors[0], traverseRoots, MatchesTraverseRoots, rootNode,
-            output);
+            traverseRoots, MatchesTraverseRoots, rootNode, output);
         return;
       }
       // Since there exists some ancestor element which has the class name, we
       // need to see all children of rootNode.
       if (ancestorHasClassName(rootNode, selector->value())) {
-        executeForTraverseRoot<SelectorQueryTrait>(*m_selectors[0], &rootNode,
-                                                   DoesNotMatchTraverseRoots,
-                                                   rootNode, output);
+        executeForTraverseRoot<SelectorQueryTrait>(&rootNode, rootNode, output);
         return;
       }
 
       ClassElementList<OnlyRoots> traverseRoots(rootNode, selector->value());
       executeForTraverseRoots<SelectorQueryTrait>(
-          *m_selectors[0], traverseRoots, DoesNotMatchTraverseRoots, rootNode,
-          output);
+          traverseRoots, DoesNotMatchTraverseRoots, rootNode, output);
       return;
     }
 
     if (selector->relation() == CSSSelector::SubSelector)
       continue;
     isRightmostSelector = false;
     if (selector->relation() == CSSSelector::DirectAdjacent ||
         selector->relation() == CSSSelector::IndirectAdjacent)
       startFromParent = true;
     else
       startFromParent = false;
   }
 
-  executeForTraverseRoot<SelectorQueryTrait>(
-      *m_selectors[0], &rootNode, DoesNotMatchTraverseRoots, rootNode, output);
+  executeForTraverseRoot<SelectorQueryTrait>(&rootNode, rootNode, output);
 }
 
 template <typename SelectorQueryTrait>
 void SelectorQuery::executeForTraverseRoot(
-    const CSSSelector& selector,
     ContainerNode* traverseRoot,
-    MatchTraverseRootState matchTraverseRoot,
     ContainerNode& rootNode,
     typename SelectorQueryTrait::OutputType& output) const {
+  DCHECK_EQ(m_selectors.size(), 1u);
+
   if (!traverseRoot)
     return;
-
-  if (matchTraverseRoot) {
-    if (selectorMatches(selector, toElement(*traverseRoot), rootNode))
-      SelectorQueryTrait::appendElement(output, toElement(*traverseRoot));
-    return;
-  }
+  const CSSSelector& selector = *m_selectors[0];
 
   for (Element& element : ElementTraversal::descendantsOf(*traverseRoot)) {
     if (selectorMatches(selector, element, rootNode)) {
       SelectorQueryTrait::appendElement(output, element);
       if (SelectorQueryTrait::shouldOnlyMatchFirstElement)
         return;
     }
   }
 }
 
 template <typename SelectorQueryTrait, typename SimpleElementListType>
 void SelectorQuery::executeForTraverseRoots(
-    const CSSSelector& selector,
     SimpleElementListType& traverseRoots,
     MatchTraverseRootState matchTraverseRoots,
     ContainerNode& rootNode,
     typename SelectorQueryTrait::OutputType& output) const {
+  DCHECK_EQ(m_selectors.size(), 1u);
+
   if (traverseRoots.isEmpty())
     return;
 
+  const CSSSelector& selector = *m_selectors[0];
+
   if (matchTraverseRoots) {
     while (!traverseRoots.isEmpty()) {
       Element& element = *traverseRoots.next();
       if (selectorMatches(selector, element, rootNode)) {
         SelectorQueryTrait::appendElement(output, element);
         if (SelectorQueryTrait::shouldOnlyMatchFirstElement)
           return;
       }
     }
     return;
@@ skipping 253 matching lines @@
   return m_entries
       .insert(selectors, SelectorQuery::adopt(std::move(selectorList)))
       .storedValue->value.get();
 }
 
 void SelectorQueryCache::invalidate() {
   m_entries.clear();
 }
 
 }  // namespace blink