bug 1052122 - derecursify TreeWalker::NextChild r=surkov

accessible/base/TreeWalker.cpp

@@ -12,36 +12,17 @@
 #include "mozilla/dom/ChildIterator.h"
 #include "mozilla/dom/Element.h"
 
+using namespace mozilla;
 using namespace mozilla::a11y;
 
-////////////////////////////////////////////////////////////////////////////////
-// WalkState
-////////////////////////////////////////////////////////////////////////////////
-
-namespace mozilla {
-namespace a11y {
-
-struct WalkState
-{
-  WalkState(nsIContent *aContent, uint32_t aFilter) :
-    content(aContent), prevState(nullptr), iter(aContent, aFilter) {}
-
-  nsCOMPtr<nsIContent> content;
-  WalkState *prevState;
-  dom::AllChildrenIterator iter;
-};
-
-} // namespace a11y
-} // namespace mozilla
-
 ////////////////////////////////////////////////////////////////////////////////
 // TreeWalker
 ////////////////////////////////////////////////////////////////////////////////
 
 TreeWalker::
   TreeWalker(Accessible* aContext, nsIContent* aContent, uint32_t aFlags) :
-  mDoc(aContext->Document()), mContext(aContext),
-  mFlags(aFlags), mState(nullptr)
+  mDoc(aContext->Document()), mContext(aContext), mAnchorNode(aContent),
+  mFlags(aFlags)
 {
   NS_ASSERTION(aContent, "No node for the accessible tree walker!");
 
@@ -50,17 +31,13 @@ TreeWalker::
   mChildFilter |= nsIContent::eSkipPlaceholderContent;
 
   if (aContent)
-    mState = new WalkState(aContent, mChildFilter);
+    PushState(aContent);
 
   MOZ_COUNT_CTOR(TreeWalker);
 }
 
 TreeWalker::~TreeWalker()
 {
-  // Clear state stack from memory
-  while (mState)
-    PopState();
-
   MOZ_COUNT_DTOR(TreeWalker);
 }
 
@@ -68,74 +45,66 @@ TreeWalker::~TreeWalker()
 // TreeWalker: private
 
 Accessible*
-TreeWalker::NextChildInternal(bool aNoWalkUp)
+TreeWalker::NextChild()
 {
-  if (!mState || !mState->content)
+  if (mStateStack.IsEmpty())
     return nullptr;
 
-  while (nsIContent* childNode = mState->iter.GetNextChild()) {
-    bool isSubtreeHidden = false;
-    Accessible* accessible = mFlags & eWalkCache ?
-      mDoc->GetAccessible(childNode) :
-      GetAccService()->GetOrCreateAccessible(childNode, mContext,
-                                             &isSubtreeHidden);
+  dom::AllChildrenIterator* top = &mStateStack[mStateStack.Length() - 1];
+  while (top) {
+    while (nsIContent* childNode = top->GetNextChild()) {
+      bool isSubtreeHidden = false;
+      Accessible* accessible = mFlags & eWalkCache ?
+        mDoc->GetAccessible(childNode) :
+        GetAccService()->GetOrCreateAccessible(childNode, mContext,
+                                               &isSubtreeHidden);
 
-    if (accessible)
-      return accessible;
-
-    // Walk down into subtree to find accessibles.
-    if (!isSubtreeHidden && childNode->IsElement()) {
-      PushState(childNode);
-      accessible = NextChildInternal(true);
       if (accessible)
         return accessible;
+
+      // Walk down into subtree to find accessibles.
+      if (!isSubtreeHidden && childNode->IsElement())
+        top = PushState(childNode);
     }
+
+    top = PopState();
   }
 
-  // No more children, get back to the parent.
-  nsIContent* anchorNode = mState->content;
-  PopState();
-  if (aNoWalkUp)
-    return nullptr;
-
-  if (mState)
-    return NextChildInternal(false);
-
   // If we traversed the whole subtree of the anchor node. Move to next node
   // relative anchor node within the context subtree if possible.
   if (mFlags != eWalkContextTree)
     return nullptr;
 
-  while (anchorNode != mContext->GetNode()) {
-    nsINode* parentNode = anchorNode->GetFlattenedTreeParent();
+  nsINode* contextNode = mContext->GetNode();
+  while (mAnchorNode != contextNode) {
+    nsINode* parentNode = mAnchorNode->GetFlattenedTreeParent();
     if (!parentNode || !parentNode->IsElement())
       return nullptr;
 
-    PushState(parentNode->AsElement());
-    while (nsIContent* childNode = mState->iter.GetNextChild()) {
-      if (childNode == anchorNode)
-        return NextChildInternal(false);
+    nsIContent* parent = parentNode->AsElement();
+    top = mStateStack.AppendElement(dom::AllChildrenIterator(parent,
+                                                             mChildFilter));
+    while (nsIContent* childNode = top->GetNextChild()) {
+      if (childNode == mAnchorNode) {
+        mAnchorNode = parent;
+        return NextChild();
+      }
     }
-    PopState();
 
-    anchorNode = parentNode->AsElement();
+    // XXX We really should never get here, it means we're trying to find an
+    // accessible for a dom node where iterating over its parent's children
+    // doesn't return it. However this sometimes happens when we're asked for
+    // the nearest accessible to place holder content which we ignore.
+    mAnchorNode = parent;
   }
 
   return nullptr;
 }
 
-void
+dom::AllChildrenIterator*
 TreeWalker::PopState()
 {
-  WalkState* prevToLastState = mState->prevState;
-  delete mState;
-  mState = prevToLastState;
-}
-
-void
-TreeWalker::PushState(nsIContent* aContent)
-{
-  WalkState* nextToLastState = new WalkState(aContent, mChildFilter);
-  nextToLastState->prevState = mState;
-  mState = nextToLastState;
+  size_t length = mStateStack.Length();
+  mStateStack.RemoveElementAt(length - 1);
+  return mStateStack.IsEmpty() ? nullptr : &mStateStack[mStateStack.Length() - 1];
 }

accessible/base/TreeWalker.h

@@ -8,6 +8,8 @@
 
 #include "mozilla/Attributes.h"
 #include <stdint.h>
+#include "mozilla/dom/ChildIterator.h"
+#include "nsCOMPtr.h"
 
 class nsIContent;
 
@@ -17,8 +19,6 @@ namespace a11y {
 class Accessible;
 class DocAccessible;
 
-struct WalkState;
-
 /**
  * This class is used to walk the DOM tree to create accessible tree.
  */
@@ -50,43 +50,36 @@ public:
    *       rejected during tree creation then the caller should be unbind it
    *       from the document.
    */
-  Accessible* NextChild()
-  {
-    return NextChildInternal(false);
-  }
+  Accessible* NextChild();
 
 private:
   TreeWalker();
   TreeWalker(const TreeWalker&);
   TreeWalker& operator =(const TreeWalker&);
 
-  /**
-   * Return the next child accessible.
-   *
-   * @param  aNoWalkUp  [in] specifies the walk direction, true means we
-   *                     shouldn't go up through the tree if we failed find
-   *                     accessible children.
-   */
-  Accessible* NextChildInternal(bool aNoWalkUp);
-
   /**
    * Create new state for the given node and push it on top of stack.
    *
    * @note State stack is used to navigate up/down the DOM subtree during
    *        accessible children search.
    */
-  void PushState(nsIContent* aNode);
+  dom::AllChildrenIterator* PushState(nsIContent* aContent)
+  {
+    return mStateStack.AppendElement(dom::AllChildrenIterator(aContent,
+                                                              mChildFilter));
+  }
 
   /**
    * Pop state from stack.
    */
-  void PopState();
+  dom::AllChildrenIterator* PopState();
 
   DocAccessible* mDoc;
   Accessible* mContext;
+  nsIContent* mAnchorNode;
+  nsAutoTArray<dom::AllChildrenIterator, 20> mStateStack;
   int32_t mChildFilter;
   uint32_t mFlags;
-  WalkState* mState;
 };
 
 } // namespace a11y

accessible/tests/mochitest/text/test_words.html

@@ -92,6 +92,8 @@
       testWordAt("div17", 3, "you", kOk);
       testWordAt("div17", 4, "here", kTodo);
 
+      testWords("input_1", ["foo", "bar"]);
+
       SimpleTest.finish();
     }
 
@@ -125,5 +127,7 @@
   <div id="div16">3+4*5=23</div>
   <div id="div17">Hello. Friend, are you here?!</div>
   </pre>
+  <input id="input_1" type="text" value="foo bar" placeholder="something or other">
+
 </body>
 </html>