From 7cce18a3b298aaa78e785c4c42f73d03b5c751ef Mon Sep 17 00:00:00 2001 From: Trevor Saunders Date: Tue, 12 Aug 2014 02:02:28 -0400 Subject: [PATCH] bug 1052122 - derecursify TreeWalker::NextChild r=surkov --- accessible/base/TreeWalker.cpp | 111 +++++++----------- accessible/base/TreeWalker.h | 29 ++--- .../tests/mochitest/text/test_words.html | 4 + 3 files changed, 55 insertions(+), 89 deletions(-) diff --git a/accessible/base/TreeWalker.cpp b/accessible/base/TreeWalker.cpp index 8e97e38c7a6..4ac1ee27b5f 100644 --- a/accessible/base/TreeWalker.cpp +++ b/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 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]; } diff --git a/accessible/base/TreeWalker.h b/accessible/base/TreeWalker.h index aa993b1679b..d34e5e9bb2e 100644 --- a/accessible/base/TreeWalker.h +++ b/accessible/base/TreeWalker.h @@ -8,6 +8,8 @@ #include "mozilla/Attributes.h" #include +#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 mStateStack; int32_t mChildFilter; uint32_t mFlags; - WalkState* mState; }; } // namespace a11y diff --git a/accessible/tests/mochitest/text/test_words.html b/accessible/tests/mochitest/text/test_words.html index f282a062fee..dff90bfeab8 100644 --- a/accessible/tests/mochitest/text/test_words.html +++ b/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 @@
3+4*5=23
Hello. Friend, are you here?!
+ +