Bug 1596238, Make NodeIterator less AddRef/Release heavy by trying to avoid use of strong pointer when the node is skipped because of whatToShow, r=Ehsan
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Thu, 14 Nov 2019 17:29:27 +0000
changeset 502009 776f4bc4d6113d1b6e301761f3c77fe33fb8df25
parent 502008 24b134b55c5f70eab758d74682fc1f6ca829e0ff
child 502010 0de59487070db211af91074884a00564363c8d85
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1596238
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1596238, Make NodeIterator less AddRef/Release heavy by trying to avoid use of strong pointer when the node is skipped because of whatToShow, r=Ehsan TreeWalker could use some similar changes, but that is a different bug. TreeWalker does use TestNode method too, which is why the new argument is optional. A new bug will be filed for TreeWalker. Differential Revision: https://phabricator.services.mozilla.com/D53016
dom/base/NodeIterator.cpp
dom/base/nsTraversal.cpp
dom/base/nsTraversal.h
--- a/dom/base/NodeIterator.cpp
+++ b/dom/base/NodeIterator.cpp
@@ -166,18 +166,18 @@ already_AddRefed<nsINode> NodeIterator::
 
   struct AutoClear {
     NodePointer* mPtr;
     explicit AutoClear(NodePointer* ptr) : mPtr(ptr) {}
     ~AutoClear() { mPtr->Clear(); }
   } ac(&mWorkingPointer);
 
   while ((mWorkingPointer.*aMove)(mRoot)) {
-    nsCOMPtr<nsINode> testNode = mWorkingPointer.mNode;
-    int16_t filtered = TestNode(testNode, aResult);
+    nsCOMPtr<nsINode> testNode;
+    int16_t filtered = TestNode(mWorkingPointer.mNode, aResult, &testNode);
     if (aResult.Failed()) {
       return nullptr;
     }
 
     if (filtered == NodeFilter_Binding::FILTER_ACCEPT) {
       mPointer = mWorkingPointer;
       return testNode.forget();
     }
--- a/dom/base/nsTraversal.cpp
+++ b/dom/base/nsTraversal.cpp
@@ -30,28 +30,33 @@ nsTraversal::~nsTraversal() { /* destruc
 
 /*
  * Tests if and how a node should be filtered. Uses mWhatToShow and
  * mFilter to test the node.
  * @param aNode     Node to test
  * @param aResult   Whether we succeeded
  * @returns         Filtervalue. See NodeFilter.webidl
  */
-int16_t nsTraversal::TestNode(nsINode* aNode, mozilla::ErrorResult& aResult) {
+int16_t nsTraversal::TestNode(nsINode* aNode, mozilla::ErrorResult& aResult,
+                              nsCOMPtr<nsINode>* aUnskippedNode) {
   if (mInAcceptNode) {
     aResult.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return 0;
   }
 
   uint16_t nodeType = aNode->NodeType();
 
   if (nodeType <= 12 && !((1 << (nodeType - 1)) & mWhatToShow)) {
     return NodeFilter_Binding::FILTER_SKIP;
   }
 
+  if (aUnskippedNode) {
+    *aUnskippedNode = aNode;
+  }
+
   if (!mFilter) {
     // No filter, just accept
     return NodeFilter_Binding::FILTER_ACCEPT;
   }
 
   AutoRestore<bool> inAcceptNode(mInAcceptNode);
   mInAcceptNode = true;
   // No need to pass in an execution reason, since the generated default,
--- a/dom/base/nsTraversal.h
+++ b/dom/base/nsTraversal.h
@@ -28,16 +28,19 @@ class nsTraversal {
   nsCOMPtr<nsINode> mRoot;
   uint32_t mWhatToShow;
   RefPtr<mozilla::dom::NodeFilter> mFilter;
   bool mInAcceptNode;
 
   /*
    * Tests if and how a node should be filtered. Uses mWhatToShow and
    * mFilter to test the node.
-   * @param aNode     Node to test
-   * @param aResult   Whether we succeeded
-   * @returns         Filtervalue. See NodeFilter.webidl
+   * @param aNode          Node to test
+   * @param aResult        Whether we succeeded
+   * @param aUnskippedNode If non-null is passed, set to aNode if node isn't
+   *                       filtered out by mWhatToShow.
+   * @returns              Filtervalue. See NodeFilter.webidl
    */
-  int16_t TestNode(nsINode* aNode, mozilla::ErrorResult& aResult);
+  int16_t TestNode(nsINode* aNode, mozilla::ErrorResult& aResult,
+                   nsCOMPtr<nsINode>* aUnskippedNode = nullptr);
 };
 
 #endif