From 7290c55f965cc12be36583ac8fe78ff760585d98 Mon Sep 17 00:00:00 2001
From: Alexander Surkov <surkov.alexander@gmail.com>
Date: Thu, 10 Mar 2016 15:50:37 -0500
Subject: [PATCH] Bug 1254989 - extend TreeWalker::Next to accept a stopper
 node, r=yzen

---
 accessible/base/TreeWalker.cpp                | 68 +++++++++++--------
 accessible/base/TreeWalker.h                  |  8 ++-
 .../mochitest/events/test_aria_menu.html      |  1 +
 3 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/accessible/base/TreeWalker.cpp b/accessible/base/TreeWalker.cpp
index 3740b2d808f9..3983fe1ea0f0 100644
--- a/accessible/base/TreeWalker.cpp
+++ b/accessible/base/TreeWalker.cpp
@@ -110,7 +110,7 @@ TreeWalker::Seek(nsIContent* aChildNode)
 }
 
 Accessible*
-TreeWalker::Next()
+TreeWalker::Next(nsIContent* aStopNode)
 {
   if (mStateStack.IsEmpty()) {
     if (mPhase == eAtEnd) {
@@ -139,30 +139,25 @@ TreeWalker::Next()
 
   dom::AllChildrenIterator* top = &mStateStack[mStateStack.Length() - 1];
   while (top) {
+    if (aStopNode && top->Get() == aStopNode) {
+      return nullptr;
+    }
+
     while (nsIContent* childNode = top->GetNextChild()) {
       bool skipSubtree = false;
-      Accessible* child = nullptr;
-      if (mFlags & eWalkCache) {
-        child = mDoc->GetAccessible(childNode);
-      }
-      else if (mContext->IsAcceptableChild(childNode)) {
-        child = GetAccService()->
-          GetOrCreateAccessible(childNode, mContext, &skipSubtree);
-      }
-
-      // Ignore the accessible and its subtree if it was repositioned by means
-      // of aria-owns.
+      Accessible* child = AccessibleFor(childNode, mFlags, &skipSubtree);
       if (child) {
-        if (child->IsRelocated()) {
-          continue;
-        }
         return child;
       }
 
-      // Walk down into subtree to find accessibles.
+      // Walk down the subtree if allowed, otherwise check if we have reached
+      // a stop node.
       if (!skipSubtree && childNode->IsElement()) {
         top = PushState(childNode, true);
       }
+      else if (childNode == aStopNode) {
+        return nullptr;
+      }
     }
     top = PopState();
   }
@@ -176,7 +171,7 @@ TreeWalker::Next()
       mPhase = eAtEnd;
       return nullptr;
     }
-    return Next();
+    return Next(aStopNode);
   }
 
   nsINode* contextNode = mContext->GetNode();
@@ -189,7 +184,7 @@ TreeWalker::Next()
     top = PushState(parent, true);
     if (top->Seek(mAnchorNode)) {
       mAnchorNode = parent;
-      return Next();
+      return Next(aStopNode);
     }
 
     // XXX We really should never get here, it means we're trying to find an
@@ -199,7 +194,7 @@ TreeWalker::Next()
     mAnchorNode = parent;
   }
 
-  return Next();
+  return Next(aStopNode);
 }
 
 Accessible*
@@ -234,17 +229,10 @@ TreeWalker::Prev()
   dom::AllChildrenIterator* top = &mStateStack[mStateStack.Length() - 1];
   while (top) {
     while (nsIContent* childNode = top->GetPreviousChild()) {
-      bool skipSubtree = false;
-
       // No accessible creation on the way back.
-      Accessible* child = mDoc->GetAccessible(childNode);
-
-      // Ignore the accessible and its subtree if it was repositioned by means of
-      // aria-owns.
+      bool skipSubtree = false;
+      Accessible* child = AccessibleFor(childNode, eWalkCache, &skipSubtree);
       if (child) {
-        if (child->IsRelocated()) {
-          continue;
-        }
         return child;
       }
 
@@ -284,10 +272,32 @@ TreeWalker::Prev()
   return nullptr;
 }
 
+Accessible*
+TreeWalker::AccessibleFor(nsIContent* aNode, uint32_t aFlags, bool* aSkipSubtree)
+{
+  Accessible* child = nullptr;
+  if (aFlags & eWalkCache) {
+    child = mDoc->GetAccessible(aNode);
+  }
+  else if (mContext->IsAcceptableChild(aNode)) {
+    child = GetAccService()->
+      GetOrCreateAccessible(aNode, mContext, aSkipSubtree);
+  }
+
+  // Ignore the accessible and its subtree if it was repositioned by means
+  // of aria-owns.
+  if (child && child->IsRelocated()) {
+    *aSkipSubtree = true;
+    return nullptr;
+  }
+
+  return child;
+}
+
 dom::AllChildrenIterator*
 TreeWalker::PopState()
 {
   size_t length = mStateStack.Length();
   mStateStack.RemoveElementAt(length - 1);
-  return mStateStack.IsEmpty() ? nullptr : &mStateStack[mStateStack.Length() - 1];
+  return mStateStack.IsEmpty() ? nullptr : &mStateStack.LastElement();
 }
diff --git a/accessible/base/TreeWalker.h b/accessible/base/TreeWalker.h
index 07dab8c734ac..afce61789f2f 100644
--- a/accessible/base/TreeWalker.h
+++ b/accessible/base/TreeWalker.h
@@ -62,7 +62,7 @@ public:
    *       rejected during tree creation then the caller should be unbind it
    *       from the document.
    */
-  Accessible* Next();
+  Accessible* Next(nsIContent* aStopNode = nullptr);
   Accessible* Prev();
 
   Accessible* Context() const { return mContext; }
@@ -73,6 +73,12 @@ private:
   TreeWalker(const TreeWalker&);
   TreeWalker& operator =(const TreeWalker&);
 
+  /**
+   * Return an accessible for the given node if any.
+   */
+  Accessible* AccessibleFor(nsIContent* aNode, uint32_t aFlags,
+                            bool* aSkipSubtree);
+
   /**
    * Create new state for the given node and push it on top of stack / at bottom
    * of stack.
diff --git a/accessible/tests/mochitest/events/test_aria_menu.html b/accessible/tests/mochitest/events/test_aria_menu.html
index d6b99b937659..2dd00478e5a3 100644
--- a/accessible/tests/mochitest/events/test_aria_menu.html
+++ b/accessible/tests/mochitest/events/test_aria_menu.html
@@ -153,6 +153,7 @@
     var gQueue = null;
 
     //gA11yEventDumpToConsole = true; // debuging
+    //enableLogging("tree,verbose");
 
     function doTests()
     {