Bug 1227224 - Create DepthFirstSearchPostOrder and extend ForEachNode functionality r?botond draft feature/APZ-generic-findtargetnode-and-getapzcatpoint
authorKevin Wern <kevin.m.wern@gmail.com>
Mon, 25 Jan 2016 04:04:13 -0800
branchfeature/APZ-generic-findtargetnode-and-getapzcatpoint
changeset 330283 3767c15db7091258f5caaf636635997d369dc756
parent 319164 9d6ffc7a08b6b47056eefe1e652710a3849adbf7
child 724539 76a1db0eb3d5074422993e0fac0c262aace472d0
push id10718
push userkevin.m.wern@gmail.com
push dateThu, 11 Feb 2016 06:18:35 +0000
reviewersbotond
bugs1227224
milestone46.0a1
Bug 1227224 - Create DepthFirstSearchPostOrder and extend ForEachNode functionality r?botond - Create internal version of ForEachNode that is recursive. Add parameter for post traversal action to outer functions. - Add flag TraversalFlag::Abort, which stops traversal immediately. - Overload ForEachNode to handle just one (pre-order) action for older callers. - Rewrite tests to reflect default traversal order. - Write DepthFirstSearch in terms of ForEachNode. - Refactor GetTargetNode and GetAPZCAtPoint using this functionality. MozReview-Commit-ID: 83Y7psjkUWG
gfx/layers/TreeTraversal.h
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/tests/gtest/TestTreeTraversal.cpp
--- a/gfx/layers/TreeTraversal.h
+++ b/gfx/layers/TreeTraversal.h
@@ -3,29 +3,139 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_layers_TreeTraversal_h
 #define mozilla_layers_TreeTraversal_h
 
 #include <queue>
-#include <stack>
 
 namespace mozilla {
 namespace layers {
 
 
 /*
- * Returned by |aAction| in ForEachNode. If the action returns
- * TraversalFlag::Skip, the node's children are not added to the traverrsal
- * stack. Otherwise, a return value of TraversalFlag::Continue indicates that
- * ForEachNode should traverse each of the node's children.
+ * Returned by |aPostAction| and |aPreAction| in ForEachNode, indicates
+ * the behavior to follow either action:
+ *
+ * TraversalFlag::Skip - the node's children are not traversed. If this
+ * flag is returned by |aPreAction|, |aPostAction| is skipped for the
+ * current node, as well.
+ * TraversalFlag::Continue - traversal continues normally.
+ * TraversalFlag::Abort - traversal stops immediately.
+ */
+enum class TraversalFlag { Skip, Continue, Abort };
+
+/*
+ * Do a depth-first traversal of the tree rooted at |aRoot|, performing
+ * |aPreAction| before traversal of children and |aPostAction| after.
+ *
+ * Returns true if traversal aborted, false if continued normally. If
+ * TraversalFlag::Skip is returned in |aPreAction|, then |aPostAction|
+ * is not performed.
+ */
+template <typename Node, typename PreAction, typename PostAction>
+static auto ForEachNode(Node* aRoot, const PreAction& aPreAction, const PostAction& aPostAction) ->
+typename EnableIf<IsSame<decltype(aPreAction(aRoot)), TraversalFlag>::value &&
+                  IsSame<decltype(aPostAction(aRoot)),TraversalFlag>::value, bool>::Type
+{
+  if (!aRoot) {
+    return false;
+  }
+
+  TraversalFlag result = aPreAction(aRoot);
+
+  if (result == TraversalFlag::Abort) {
+    return true;
+  }
+
+  if (result == TraversalFlag::Continue) {
+    for (Node* child = aRoot->GetLastChild();
+         child;
+         child = child->GetPrevSibling()) {
+      bool abort = ForEachNode(child, aPreAction, aPostAction);
+      if (abort) {
+        return true;
+      }
+    }
+
+    result = aPostAction(aRoot);
+
+    if (result == TraversalFlag::Abort) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+/*
+ * Do a depth-first traversal of the tree rooted at |aRoot|, performing
+ * |aPreAction| before traversal of children and |aPostAction| after.
  */
-enum class TraversalFlag { Skip, Continue };
+template <typename Node, typename PreAction, typename PostAction>
+static auto ForEachNode(Node* aRoot, const PreAction& aPreAction, const PostAction& aPostAction) ->
+typename EnableIf<IsSame<decltype(aPreAction(aRoot)), void>::value &&
+                  IsSame<decltype(aPostAction(aRoot)),void>::value, void>::Type
+{
+  if (!aRoot) {
+    return;
+  }
+
+  aPreAction(aRoot);
+
+  for (Node* child = aRoot->GetLastChild();
+       child;
+       child = child->GetPrevSibling()) {
+    ForEachNode(child, aPreAction, aPostAction);
+  }
+
+  aPostAction(aRoot);
+}
+
+/*
+ * ForEachNode pre-order traversal, using TraversalFlag.
+ */
+template <typename Node, typename PreAction>
+auto ForEachNode(Node* aRoot, const PreAction& aPreAction) ->
+typename EnableIf<IsSame<decltype(aPreAction(aRoot)), TraversalFlag>::value, bool>::Type
+{
+  return ForEachNode(aRoot, aPreAction, [](Node* aNode){ return TraversalFlag::Continue; });
+}
+
+/*
+ * ForEachNode pre-order, not using TraversalFlag.
+ */
+template <typename Node, typename PreAction>
+auto ForEachNode(Node* aRoot, const PreAction& aPreAction) ->
+typename EnableIf<IsSame<decltype(aPreAction(aRoot)), void>::value, void>::Type
+{
+  ForEachNode(aRoot, aPreAction, [](Node* aNode){});
+}
+
+/*
+ * ForEachNode post-order traversal, using TraversalFlag.
+ */
+template <typename Node, typename PostAction>
+auto ForEachNodePostOrder(Node* aRoot, const PostAction& aPostAction) ->
+typename EnableIf<IsSame<decltype(aPostAction(aRoot)), TraversalFlag>::value, bool>::Type
+{
+  return ForEachNode(aRoot, [](Node* aNode){ return TraversalFlag::Continue; }, aPostAction);
+}
+
+/*
+ * ForEachNode post-order, not using TraversalFlag.
+ */
+template <typename Node, typename PostAction>
+auto ForEachNodePostOrder(Node* aRoot, const PostAction& aPostAction) ->
+typename EnableIf<IsSame<decltype(aPostAction(aRoot)), void>::value, void>::Type
+{
+  ForEachNode(aRoot, [](Node* aNode){}, aPostAction);
+}
 
 /*
  * Do a breadth-first search of the tree rooted at |aRoot|, and return the
  * first visited node that satisfies |aCondition|, or nullptr if no such node
  * was found.
  *
  * |Node| should have methods GetLastChild() and GetPrevSibling().
  */
@@ -52,105 +162,59 @@ Node* BreadthFirstSearch(Node* aRoot, co
       queue.push(child);
     }
   }
 
   return nullptr;
 }
 
 /*
- * Do a depth-first search of the tree rooted at |aRoot|, and return the
- * first visited node that satisfies |aCondition|, or nullptr if no such node
- * was found.
+ * Do a pre-order, depth-first search of the tree rooted at |aRoot|, and
+ * return the first visited node that satisfies |aCondition|, or nullptr
+ * if no such node was found.
  *
  * |Node| should have methods GetLastChild() and GetPrevSibling().
  */
 template <typename Node, typename Condition>
 Node* DepthFirstSearch(Node* aRoot, const Condition& aCondition)
 {
-  if (!aRoot) {
-    return nullptr;
-  }
-
-  std::stack<Node*> stack;
-  stack.push(aRoot);
-  while (!stack.empty()) {
-    Node* node = stack.top();
-    stack.pop();
+  Node* result = nullptr;
 
-    if (aCondition(node)) {
-        return node;
-    }
+  ForEachNode(aRoot,
+      [&aCondition, &result](Node* aNode)
+      {
+        if (aCondition(aNode)) {
+          result = aNode;
+          return TraversalFlag::Abort;
+        }
 
-    for (Node* child = node->GetLastChild();
-         child;
-         child = child->GetPrevSibling()) {
-      stack.push(child);
-    }
-  }
+        return TraversalFlag::Continue;
+      });
 
-  return nullptr;
+  return result;
 }
 
 /*
- * Do a depth-first traversal of the tree rooted at |aRoot|, performing
- * |aAction| for each node.  |aAction| can return a TraversalFlag to determine
- * whether or not to omit the children of a particular node.
- *
- * If |aAction| does not return a TraversalFlag, it must return nothing.  There
- * is no ForEachNode instance handling types other than void or TraversalFlag.
+ * Perform a post-order, depth-first search starting at aRoot.
  */
-template <typename Node, typename Action>
-auto ForEachNode(Node* aRoot, const Action& aAction) ->
-typename EnableIf<IsSame<decltype(aAction(aRoot)), TraversalFlag>::value, void>::Type
+template <typename Node, typename Condition>
+Node* DepthFirstSearchPostOrder(Node* aRoot, const Condition& aCondition)
 {
-  if (!aRoot) {
-    return;
-  }
-
-  std::stack<Node*> stack;
-  stack.push(aRoot);
-
-  while (!stack.empty()) {
-    Node* node = stack.top();
-    stack.pop();
-
-    TraversalFlag result = aAction(node);
+  Node* result = nullptr;
 
-    if (result == TraversalFlag::Continue) {
-      for (Node* child = node->GetLastChild();
-           child;
-           child = child->GetPrevSibling()) {
-        stack.push(child);
-      }
-    }
-  }
-}
+  ForEachNodePostOrder(aRoot,
+      [&aCondition, &result](Node* aNode)
+      {
+        if (aCondition(aNode)) {
+          result = aNode;
+          return TraversalFlag::Abort;
+        }
 
-template <typename Node, typename Action>
-auto ForEachNode(Node* aRoot, const Action& aAction) ->
-typename EnableIf<IsSame<decltype(aAction(aRoot)), void>::value, void>::Type
-{
-  if (!aRoot) {
-    return;
-  }
-
-  std::stack<Node*> stack;
-  stack.push(aRoot);
+        return TraversalFlag::Continue;
+      });
 
-  while (!stack.empty()) {
-    Node* node = stack.top();
-    stack.pop();
-
-    aAction(node);
-
-    for (Node* child = node->GetLastChild();
-         child;
-         child = child->GetPrevSibling()) {
-      stack.push(child);
-    }
-  }
+  return result;
 }
 
 }
 }
 
 #endif // mozilla_layers_TreeTraversal_h
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -1,13 +1,14 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include <stack>
 #include "APZCTreeManager.h"
 #include "AsyncPanZoomController.h"
 #include "Compositor.h"                 // for Compositor
 #include "gfxPrefs.h"                   // for gfxPrefs
 #include "HitTestingTreeNode.h"         // for HitTestingTreeNode
 #include "InputBlockState.h"            // for InputBlockState
 #include "InputData.h"                  // for InputData, etc
 #include "Layers.h"                     // for Layer, etc
@@ -1487,17 +1488,30 @@ APZCTreeManager::GetTargetAPZC(const Scr
   return apzc.forget();
 }
 
 already_AddRefed<HitTestingTreeNode>
 APZCTreeManager::GetTargetNode(const ScrollableLayerGuid& aGuid,
                                GuidComparator aComparator)
 {
   mTreeLock.AssertCurrentThreadOwns();
-  RefPtr<HitTestingTreeNode> target = FindTargetNode(mRootNode, aGuid, aComparator);
+  RefPtr<HitTestingTreeNode> target = DepthFirstSearchPostOrder(mRootNode.get(),
+      [&aGuid, &aComparator](HitTestingTreeNode* node)
+      {
+        bool matches = false;
+        if (node->GetApzc()) {
+          if (aComparator) {
+            matches = aComparator(aGuid, node->GetApzc()->GetGuid());
+          } else {
+            matches = node->GetApzc()->Matches(aGuid);
+          }
+        }
+        return matches;
+      }
+  );
   return target.forget();
 }
 
 already_AddRefed<AsyncPanZoomController>
 APZCTreeManager::GetTargetAPZC(const ScreenPoint& aPoint, HitTestResult* aOutHitResult)
 {
   MonitorAutoLock lock(mTreeLock);
   HitTestResult hitResult = HitNothing;
@@ -1597,46 +1611,16 @@ APZCTreeManager::BuildOverscrollHandoffC
 
 /* static */ void
 APZCTreeManager::SetLongTapEnabled(bool aLongTapEnabled)
 {
   APZThreadUtils::RunOnControllerThread(
     NewRunnableFunction(GestureEventListener::SetLongTapEnabled, aLongTapEnabled));
 }
 
-HitTestingTreeNode*
-APZCTreeManager::FindTargetNode(HitTestingTreeNode* aNode,
-                                const ScrollableLayerGuid& aGuid,
-                                GuidComparator aComparator)
-{
-  mTreeLock.AssertCurrentThreadOwns();
-
-  // This walks the tree in depth-first, reverse order, so that it encounters
-  // APZCs front-to-back on the screen.
-  for (HitTestingTreeNode* node = aNode; node; node = node->GetPrevSibling()) {
-    HitTestingTreeNode* match = FindTargetNode(node->GetLastChild(), aGuid, aComparator);
-    if (match) {
-      return match;
-    }
-
-    bool matches = false;
-    if (node->GetApzc()) {
-      if (aComparator) {
-        matches = aComparator(aGuid, node->GetApzc()->GetGuid());
-      } else {
-        matches = node->GetApzc()->Matches(aGuid);
-      }
-    }
-    if (matches) {
-      return node;
-    }
-  }
-  return nullptr;
-}
-
 RefPtr<HitTestingTreeNode>
 APZCTreeManager::FindScrollNode(const AsyncDragMetrics& aDragMetrics)
 {
   MonitorAutoLock lock(mTreeLock);
 
   return DepthFirstSearch(mRootNode.get(),
       [&aDragMetrics](HitTestingTreeNode* aNode) {
         return aNode->MatchesScrollDragMetrics(aDragMetrics);
@@ -1647,61 +1631,70 @@ AsyncPanZoomController*
 APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
                                 const ParentLayerPoint& aHitTestPoint,
                                 HitTestResult* aOutHitResult)
 {
   mTreeLock.AssertCurrentThreadOwns();
 
   // This walks the tree in depth-first, reverse order, so that it encounters
   // APZCs front-to-back on the screen.
-  for (HitTestingTreeNode* node = aNode; node; node = node->GetPrevSibling()) {
-    if (node->IsOutsideClip(aHitTestPoint)) {
-      // If the point being tested is outside the clip region for this node
-      // then we don't need to test against this node or any of its children.
-      // Just skip it and move on.
-      APZCTM_LOG("Point %f %f outside clip for node %p\n",
-        aHitTestPoint.x, aHitTestPoint.y, node);
-      continue;
-    }
-
-    AsyncPanZoomController* result = nullptr;
-
-    // First check the subtree rooted at this node, because deeper nodes
-    // are more "in front".
-    Maybe<LayerPoint> hitTestPointForChildLayers = node->Untransform(aHitTestPoint);
-    if (hitTestPointForChildLayers) {
-      ParentLayerPoint childPoint = ViewAs<ParentLayerPixel>(hitTestPointForChildLayers.ref(),
-        PixelCastJustification::MovingDownToChildren);
-      result = GetAPZCAtPoint(node->GetLastChild(), childPoint, aOutHitResult);
-    }
+  HitTestingTreeNode* resultNode;
+  HitTestingTreeNode* root = aNode;
+  std::stack<ParentLayerPoint> hitTestPoints;
+  hitTestPoints.push(aHitTestPoint);
 
-    // If we didn't match anything in the subtree, check |node|.
-    if (*aOutHitResult == HitNothing) {
-      APZCTM_LOG("Testing ParentLayer point %s (Layer %s) against node %p\n",
-          Stringify(aHitTestPoint).c_str(),
-          hitTestPointForChildLayers ? Stringify(hitTestPointForChildLayers.ref()).c_str() : "nil",
-          node);
-      HitTestResult hitResult = node->HitTest(aHitTestPoint);
-      if (hitResult != HitTestResult::HitNothing) {
-        result = node->GetNearestContainingApzcWithSameLayersId();
-        if (!result) {
-          result = FindRootApzcForLayersId(node->GetLayersId());
-          MOZ_ASSERT(result);
+  ForEachNode(root,
+      [&hitTestPoints](HitTestingTreeNode* aNode) {
+        if (aNode->IsOutsideClip(hitTestPoints.top())) {
+          // If the point being tested is outside the clip region for this node
+          // then we don't need to test against this node or any of its children.
+          // Just skip it and move on.
+          APZCTM_LOG("Point %f %f outside clip for node %p\n",
+            hitTestPoints.top().x, hitTestPoints.top().y, aNode);
+          return TraversalFlag::Skip;
+        }
+        // First check the subtree rooted at this node, because deeper nodes
+        // are more "in front".
+        Maybe<LayerPoint> hitTestPointForChildLayers = aNode->Untransform(hitTestPoints.top());
+        APZCTM_LOG("Transformed ParentLayer point %s to layer %s\n",
+                Stringify(hitTestPoints.top()).c_str(),
+                hitTestPointForChildLayers ? Stringify(hitTestPointForChildLayers.ref()).c_str() : "nil");
+        if (!hitTestPointForChildLayers) {
+          return TraversalFlag::Skip;
         }
-        APZCTM_LOG("Successfully matched APZC %p via node %p (hit result %d)\n",
-             result, node, hitResult);
-        MOZ_ASSERT(hitResult == HitLayer || hitResult == HitDispatchToContentRegion);
-        // If event regions are disabled, *aOutHitResult will be HitLayer
-        *aOutHitResult = hitResult;
+        hitTestPoints.push(ViewAs<ParentLayerPixel>(hitTestPointForChildLayers.ref(),
+            PixelCastJustification::MovingDownToChildren));
+        return TraversalFlag::Continue;
+      },
+      [&resultNode, &hitTestPoints, &aOutHitResult](HitTestingTreeNode* aNode) {
+        hitTestPoints.pop();
+        HitTestResult hitResult = aNode->HitTest(hitTestPoints.top());
+        APZCTM_LOG("Testing ParentLayer point %s against node %p\n",
+                Stringify(hitTestPoints.top()).c_str(), aNode);
+        if (hitResult != HitTestResult::HitNothing) {
+          resultNode = aNode;
+          MOZ_ASSERT(hitResult == HitLayer || hitResult == HitDispatchToContentRegion);
+          // If event regions are disabled, *aOutHitResult will be HitLayer
+          *aOutHitResult = hitResult;
+          return TraversalFlag::Abort;
+        }
+        return TraversalFlag::Continue;
       }
-    }
+  );
 
-    if (*aOutHitResult != HitNothing) {
+  if (*aOutHitResult != HitNothing) {
+      MOZ_ASSERT(resultNode);
+      AsyncPanZoomController* result = resultNode->GetNearestContainingApzcWithSameLayersId();
+      if (!result) {
+        result = FindRootApzcForLayersId(resultNode->GetLayersId());
+        MOZ_ASSERT(result);
+      }
+      APZCTM_LOG("Successfully matched APZC %p via node %p (hit result %d)\n",
+          result, aNode, *aOutHitResult);
       return result;
-    }
   }
 
   return nullptr;
 }
 
 AsyncPanZoomController*
 APZCTreeManager::FindRootApzcForLayersId(uint64_t aLayersId) const
 {
--- a/gfx/tests/gtest/TestTreeTraversal.cpp
+++ b/gfx/tests/gtest/TestTreeTraversal.cpp
@@ -116,25 +116,25 @@ TEST(TreeTraversal, DepthFirstSearchValu
     } else if (i < expectedNeedleTraversalRank) {
       nodeList.push_back(new SearchTestNode(SearchNodeType::Hay, i));
     } else {
       nodeList.push_back(new SearchTestNode(SearchNodeType::Hay));
     }
   }
 
   RefPtr<SearchTestNode> root = nodeList[0];
+  nodeList[0]->AddChild(nodeList[4]);
   nodeList[0]->AddChild(nodeList[1]);
-  nodeList[0]->AddChild(nodeList[4]);
+  nodeList[1]->AddChild(nodeList[3]);
   nodeList[1]->AddChild(nodeList[2]);
-  nodeList[1]->AddChild(nodeList[3]);
+  nodeList[4]->AddChild(nodeList[6]);
   nodeList[4]->AddChild(nodeList[5]);
-  nodeList[4]->AddChild(nodeList[6]);
   nodeList[6]->AddChild(nodeList[7]);
+  nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
-  nodeList[7]->AddChild(nodeList[9]);
 
   RefPtr<SearchTestNode> foundNode = DepthFirstSearch(root.get(),
       [&visitCount](SearchTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
         visitCount++;
         return aNode->GetType() == SearchNodeType::Needle;
       });
@@ -180,43 +180,166 @@ TEST(TreeTraversal, DepthFirstSearchValu
   int visitCount = 0;
   std::vector<RefPtr<SearchTestNode>> nodeList;
   for (int i = 0; i < 10; i++)
   {
       nodeList.push_back(new SearchTestNode(SearchNodeType::Hay, i));
   }
 
   RefPtr<SearchTestNode> root = nodeList[0];
+  nodeList[0]->AddChild(nodeList[4]);
   nodeList[0]->AddChild(nodeList[1]);
-  nodeList[0]->AddChild(nodeList[4]);
+  nodeList[1]->AddChild(nodeList[3]);
   nodeList[1]->AddChild(nodeList[2]);
-  nodeList[1]->AddChild(nodeList[3]);
+  nodeList[4]->AddChild(nodeList[6]);
   nodeList[4]->AddChild(nodeList[5]);
-  nodeList[4]->AddChild(nodeList[6]);
   nodeList[6]->AddChild(nodeList[7]);
+  nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
-  nodeList[7]->AddChild(nodeList[9]);
 
 
   RefPtr<SearchTestNode> foundNode = DepthFirstSearch(root.get(),
       [&visitCount](SearchTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == SearchNodeType::Needle;
       });
 
   for (int i = 0; i < 10; i++)
   {
-    ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(), 
+    ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(),
+        nodeList[i]->GetActualTraversalRank())
+        << "Node at index " << i << " was hit out of order.";
+  }
+
+  ASSERT_EQ(foundNode.get(), nullptr)
+      << "Search found something that should not exist.";
+}
+
+TEST(TreeTraversal, DepthFirstSearchPostOrderNull)
+{
+  RefPtr<SearchTestNode> nullNode;
+  RefPtr<SearchTestNode> result = DepthFirstSearchPostOrder(nullNode.get(),
+      [](SearchTestNode* aNode)
+      {
+        return aNode->GetType() == SearchNodeType::Needle;
+      });
+  ASSERT_EQ(result.get(), nullptr) << "Null root did not return null search result.";
+}
+
+TEST(TreeTraversal, DepthFirstSearchPostOrderValueExists)
+{
+  int visitCount = 0;
+  size_t expectedNeedleTraversalRank = 7;
+  RefPtr<SearchTestNode> needleNode;
+  std::vector<RefPtr<SearchTestNode>> nodeList;
+  for (size_t i = 0; i < 10; i++)
+  {
+    if (i == expectedNeedleTraversalRank) {
+      needleNode = new SearchTestNode(SearchNodeType::Needle, i);
+      nodeList.push_back(needleNode);
+    } else if (i < expectedNeedleTraversalRank) {
+      nodeList.push_back(new SearchTestNode(SearchNodeType::Hay, i));
+    } else {
+      nodeList.push_back(new SearchTestNode(SearchNodeType::Hay));
+    }
+  }
+
+  RefPtr<SearchTestNode> root = nodeList[9];
+  nodeList[9]->AddChild(nodeList[8]);
+  nodeList[9]->AddChild(nodeList[2]);
+  nodeList[2]->AddChild(nodeList[1]);
+  nodeList[2]->AddChild(nodeList[0]);
+  nodeList[8]->AddChild(nodeList[7]);
+  nodeList[8]->AddChild(nodeList[6]);
+  nodeList[6]->AddChild(nodeList[5]);
+  nodeList[5]->AddChild(nodeList[4]);
+  nodeList[5]->AddChild(nodeList[3]);
+
+  RefPtr<SearchTestNode> foundNode = DepthFirstSearchPostOrder(root.get(),
+      [&visitCount](SearchTestNode* aNode)
+      {
+        aNode->SetActualTraversalRank(visitCount);
+        visitCount++;
+        return aNode->GetType() == SearchNodeType::Needle;
+      });
+
+  for (size_t i = 0; i < nodeList.size(); i++)
+  {
+    ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(),
         nodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
   }
 
-  ASSERT_EQ(foundNode.get(), nullptr) 
+  ASSERT_EQ(foundNode, needleNode) << "Search did not return expected node.";
+  ASSERT_EQ(foundNode->GetType(), SearchNodeType::Needle)
+      << "Returned node does not match expected value (something odd happened).";
+}
+
+TEST(TreeTraversal, DepthFirstSearchPostOrderRootIsNeedle)
+{
+  RefPtr<SearchTestNode> root = new SearchTestNode(SearchNodeType::Needle, 0);
+  RefPtr<SearchTestNode> childNode1= new SearchTestNode(SearchNodeType::Hay);
+  RefPtr<SearchTestNode> childNode2 = new SearchTestNode(SearchNodeType::Hay);
+  int visitCount = 0;
+  RefPtr<SearchTestNode> result = DepthFirstSearchPostOrder(root.get(),
+      [&visitCount](SearchTestNode* aNode)
+      {
+        aNode->SetActualTraversalRank(visitCount);
+        visitCount++;
+        return aNode->GetType() == SearchNodeType::Needle;
+      });
+  ASSERT_EQ(result, root) << "Search starting at needle did not return needle.";
+  ASSERT_EQ(root->GetExpectedTraversalRank(), root->GetActualTraversalRank())
+      << "Search starting at needle did not return needle.";
+  ASSERT_EQ(childNode1->GetExpectedTraversalRank(),
+      childNode1->GetActualTraversalRank())
+      << "Search starting at needle continued past needle.";
+  ASSERT_EQ(childNode2->GetExpectedTraversalRank(),
+      childNode2->GetActualTraversalRank())
+      << "Search starting at needle continued past needle.";
+}
+
+TEST(TreeTraversal, DepthFirstSearchPostOrderValueDoesNotExist)
+{
+  int visitCount = 0;
+  std::vector<RefPtr<SearchTestNode>> nodeList;
+  for (int i = 0; i < 10; i++)
+  {
+      nodeList.push_back(new SearchTestNode(SearchNodeType::Hay, i));
+  }
+
+  RefPtr<SearchTestNode> root = nodeList[9];
+  nodeList[9]->AddChild(nodeList[8]);
+  nodeList[9]->AddChild(nodeList[2]);
+  nodeList[2]->AddChild(nodeList[1]);
+  nodeList[2]->AddChild(nodeList[0]);
+  nodeList[8]->AddChild(nodeList[7]);
+  nodeList[8]->AddChild(nodeList[6]);
+  nodeList[6]->AddChild(nodeList[5]);
+  nodeList[5]->AddChild(nodeList[4]);
+  nodeList[5]->AddChild(nodeList[3]);
+
+  RefPtr<SearchTestNode> foundNode = DepthFirstSearchPostOrder(root.get(),
+      [&visitCount](SearchTestNode* aNode)
+      {
+        aNode->SetActualTraversalRank(visitCount);
+        visitCount++;
+        return aNode->GetType() == SearchNodeType::Needle;
+      });
+
+  for (int i = 0; i < 10; i++)
+  {
+    ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(),
+        nodeList[i]->GetActualTraversalRank())
+        << "Node at index " << i << " was hit out of order.";
+  }
+
+  ASSERT_EQ(foundNode.get(), nullptr)
       << "Search found something that should not exist.";
 }
 
 TEST(TreeTraversal, BreadthFirstSearchNull)
 {
   RefPtr<SearchTestNode> nullNode;
   RefPtr<SearchTestNode> result = BreadthFirstSearch(nullNode.get(),
       [](SearchTestNode* aNode)
@@ -238,20 +361,20 @@ TEST(TreeTraversal, BreadthFirstSearchRo
         aNode->SetActualTraversalRank(visitCount);
         visitCount++;
         return aNode->GetType() == SearchNodeType::Needle;
       });
   ASSERT_EQ(result, root) << "Search starting at needle did not return needle.";
   ASSERT_EQ(root->GetExpectedTraversalRank(), root->GetActualTraversalRank())
       << "Search starting at needle did not return needle.";
   ASSERT_EQ(childNode1->GetExpectedTraversalRank(),
-      childNode1->GetActualTraversalRank()) 
+      childNode1->GetActualTraversalRank())
       << "Search starting at needle continued past needle.";
   ASSERT_EQ(childNode2->GetExpectedTraversalRank(),
-      childNode2->GetActualTraversalRank()) 
+      childNode2->GetActualTraversalRank())
       << "Search starting at needle continued past needle.";
 }
 
 TEST(TreeTraversal, BreadthFirstSearchValueExists)
 {
   int visitCount = 0;
   size_t expectedNeedleTraversalRank = 7;
   RefPtr<SearchTestNode> needleNode;
@@ -278,17 +401,17 @@ TEST(TreeTraversal, BreadthFirstSearchVa
   nodeList[6]->AddChild(nodeList[7]);
   nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
 
   RefPtr<SearchTestNode> foundNode = BreadthFirstSearch(root.get(),
       [&visitCount](SearchTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == SearchNodeType::Needle;
       });
 
   for (size_t i = 0; i < nodeList.size(); i++)
   {
     ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(),
         nodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
@@ -319,17 +442,17 @@ TEST(TreeTraversal, BreadthFirstSearchVa
   nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
 
 
   RefPtr<SearchTestNode> foundNode = BreadthFirstSearch(root.get(),
       [&visitCount](SearchTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == SearchNodeType::Needle;
       });
 
   for (size_t i = 0; i < nodeList.size(); i++)
   {
       ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(), 
           nodeList[i]->GetActualTraversalRank())
           << "Node at index " << i << " was hit out of order.";
@@ -354,34 +477,34 @@ TEST(TreeTraversal, ForEachNodeAllEligib
   std::vector<RefPtr<ForEachTestNode>> nodeList;
   int visitCount = 0;
   for (int i = 0; i < 10; i++)
   {
     nodeList.push_back(new ForEachTestNode(ForEachNodeType::Continue,i));
   }
 
   RefPtr<ForEachTestNode> root = nodeList[0];
+  nodeList[0]->AddChild(nodeList[4]);
   nodeList[0]->AddChild(nodeList[1]);
-  nodeList[0]->AddChild(nodeList[4]);
+  nodeList[1]->AddChild(nodeList[3]);
   nodeList[1]->AddChild(nodeList[2]);
-  nodeList[1]->AddChild(nodeList[3]);
+  nodeList[4]->AddChild(nodeList[6]);
   nodeList[4]->AddChild(nodeList[5]);
-  nodeList[4]->AddChild(nodeList[6]);
   nodeList[6]->AddChild(nodeList[7]);
+  nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
-  nodeList[7]->AddChild(nodeList[9]);
 
 
   ForEachNode(root.get(),
       [&visitCount](ForEachTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == ForEachNodeType::Continue
-	    ? TraversalFlag::Continue : TraversalFlag::Skip;
+            ? TraversalFlag::Continue : TraversalFlag::Skip;
       });
 
   for (size_t i = 0; i < nodeList.size(); i++)
   {
     ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(), 
         nodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
   }
@@ -399,31 +522,31 @@ TEST(TreeTraversal, ForEachNodeSomeIneli
   expectedVisitedNodeList.push_back(new ForEachTestNode(ForEachNodeType::Skip, 3));
 
   expectedSkippedNodeList.push_back(new ForEachTestNode(ForEachNodeType::Continue));
   expectedSkippedNodeList.push_back(new ForEachTestNode(ForEachNodeType::Continue));
   expectedSkippedNodeList.push_back(new ForEachTestNode(ForEachNodeType::Skip));
   expectedSkippedNodeList.push_back(new ForEachTestNode(ForEachNodeType::Skip));
 
   RefPtr<ForEachTestNode> root = expectedVisitedNodeList[0];
+  expectedVisitedNodeList[0]->AddChild(expectedVisitedNodeList[2]);
   expectedVisitedNodeList[0]->AddChild(expectedVisitedNodeList[1]);
-  expectedVisitedNodeList[0]->AddChild(expectedVisitedNodeList[2]);
-  expectedVisitedNodeList[1]->AddChild(expectedSkippedNodeList[0]);
   expectedVisitedNodeList[1]->AddChild(expectedSkippedNodeList[1]);
+  expectedVisitedNodeList[1]->AddChild(expectedSkippedNodeList[0]);
   expectedVisitedNodeList[2]->AddChild(expectedVisitedNodeList[3]);
+  expectedVisitedNodeList[3]->AddChild(expectedSkippedNodeList[3]);
   expectedVisitedNodeList[3]->AddChild(expectedSkippedNodeList[2]);
-  expectedVisitedNodeList[3]->AddChild(expectedSkippedNodeList[3]);
 
   ForEachNode(root.get(),
       [&visitCount](ForEachTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == ForEachNodeType::Continue
-	    ? TraversalFlag::Continue : TraversalFlag::Skip;
+            ? TraversalFlag::Continue : TraversalFlag::Skip;
       });
 
   for (size_t i = 0; i < expectedVisitedNodeList.size(); i++)
   {
     ASSERT_EQ(expectedVisitedNodeList[i]->GetExpectedTraversalRank(), 
         expectedVisitedNodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
   }
@@ -443,19 +566,19 @@ TEST(TreeTraversal, ForEachNodeIneligibl
   RefPtr<ForEachTestNode> root = new ForEachTestNode(ForEachNodeType::Skip, 0);
   RefPtr<ForEachTestNode> childNode1 = new ForEachTestNode(ForEachNodeType::Continue);
   RefPtr<ForEachTestNode> chlidNode2 = new ForEachTestNode(ForEachNodeType::Skip);
 
   ForEachNode(root.get(),
       [&visitCount](ForEachTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == ForEachNodeType::Continue
-	    ? TraversalFlag::Continue : TraversalFlag::Skip;
+            ? TraversalFlag::Continue : TraversalFlag::Skip;
       });
 
   ASSERT_EQ(root->GetExpectedTraversalRank(), root->GetActualTraversalRank())
       << "Root was hit out of order.";
   ASSERT_EQ(childNode1->GetExpectedTraversalRank(), childNode1->GetActualTraversalRank())
       << "Eligible child was still hit.";
   ASSERT_EQ(chlidNode2->GetExpectedTraversalRank(), chlidNode2->GetActualTraversalRank())
       << "Ineligible child was still hit.";
@@ -471,33 +594,33 @@ TEST(TreeTraversal, ForEachNodeLeavesIne
     if (i == 1 || i == 9) {
       nodeList.push_back(new ForEachTestNode(ForEachNodeType::Skip, i));
     } else {
       nodeList.push_back(new ForEachTestNode(ForEachNodeType::Continue, i));
     }
   }
 
   RefPtr<ForEachTestNode> root = nodeList[0];
+  nodeList[0]->AddChild(nodeList[2]);
   nodeList[0]->AddChild(nodeList[1]);
-  nodeList[0]->AddChild(nodeList[2]);
+  nodeList[2]->AddChild(nodeList[4]);
   nodeList[2]->AddChild(nodeList[3]);
-  nodeList[2]->AddChild(nodeList[4]);
+  nodeList[4]->AddChild(nodeList[6]);
   nodeList[4]->AddChild(nodeList[5]);
-  nodeList[4]->AddChild(nodeList[6]);
   nodeList[6]->AddChild(nodeList[7]);
+  nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
-  nodeList[7]->AddChild(nodeList[9]);
 
   ForEachNode(root.get(),
       [&visitCount](ForEachTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
         return aNode->GetType() == ForEachNodeType::Continue
-	    ? TraversalFlag::Continue : TraversalFlag::Skip;
+            ? TraversalFlag::Continue : TraversalFlag::Skip;
       });
 
   for (size_t i = 0; i < nodeList.size(); i++)
   {
     ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(), 
         nodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
   }
@@ -508,32 +631,32 @@ TEST(TreeTraversal, ForEachNodeLambdaRet
   std::vector<RefPtr<ForEachTestNode>> nodeList;
   int visitCount = 0;
   for (int i = 0; i < 10; i++)
   {
     nodeList.push_back(new ForEachTestNode(ForEachNodeType::Continue,i));
   }
 
   RefPtr<ForEachTestNode> root = nodeList[0];
+  nodeList[0]->AddChild(nodeList[4]);
   nodeList[0]->AddChild(nodeList[1]);
-  nodeList[0]->AddChild(nodeList[4]);
+  nodeList[1]->AddChild(nodeList[3]);
   nodeList[1]->AddChild(nodeList[2]);
-  nodeList[1]->AddChild(nodeList[3]);
+  nodeList[4]->AddChild(nodeList[6]);
   nodeList[4]->AddChild(nodeList[5]);
-  nodeList[4]->AddChild(nodeList[6]);
   nodeList[6]->AddChild(nodeList[7]);
+  nodeList[7]->AddChild(nodeList[9]);
   nodeList[7]->AddChild(nodeList[8]);
-  nodeList[7]->AddChild(nodeList[9]);
 
 
   ForEachNode(root.get(),
       [&visitCount](ForEachTestNode* aNode)
       {
         aNode->SetActualTraversalRank(visitCount);
-	visitCount++;
+        visitCount++;
       });
 
   for (size_t i = 0; i < nodeList.size(); i++)
   {
     ASSERT_EQ(nodeList[i]->GetExpectedTraversalRank(),
         nodeList[i]->GetActualTraversalRank())
         << "Node at index " << i << " was hit out of order.";
   }