Bug 646406 - AccTreeWalker::NextChild should return raw pointer, r=trevor
authorAlexander Surkov <surkov.alexander@gmail.com>
Thu, 31 Mar 2011 18:30:58 +0900
changeset 64506 6fa8fbd811fa39d603889e4d4d7c01ad4bc7c67a
parent 64505 51118920ebe17985ffced51b8aa6000e04d50413
child 64507 6b17244c57959ecae020ac04ca91883c088c1be5
push idunknown
push userunknown
push dateunknown
reviewerstrevor
bugs646406
milestone2.2a1pre
Bug 646406 - AccTreeWalker::NextChild should return raw pointer, r=trevor
accessible/src/base/nsAccTreeWalker.cpp
accessible/src/base/nsAccTreeWalker.h
accessible/src/base/nsAccessibilityService.cpp
accessible/src/base/nsAccessibilityService.h
accessible/src/base/nsAccessible.cpp
accessible/src/base/nsDocAccessible.cpp
accessible/src/html/nsHTMLTableAccessible.cpp
accessible/src/xforms/nsXFormsAccessible.cpp
accessible/src/xul/nsXULColorPickerAccessible.cpp
accessible/src/xul/nsXULFormControlAccessible.cpp
--- a/accessible/src/base/nsAccTreeWalker.cpp
+++ b/accessible/src/base/nsAccTreeWalker.cpp
@@ -89,18 +89,18 @@ nsAccTreeWalker::~nsAccTreeWalker()
     PopState();
 
   MOZ_COUNT_DTOR(nsAccTreeWalker);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsAccTreeWalker: private
 
-already_AddRefed<nsAccessible>
-nsAccTreeWalker::GetNextChildInternal(PRBool aNoWalkUp)
+nsAccessible*
+nsAccTreeWalker::NextChildInternal(bool aNoWalkUp)
 {
   if (!mState || !mState->content)
     return nsnull;
 
   if (!mState->childList)
     mState->childList = mState->content->GetChildren(mChildFilter);
 
   nsCOMPtr<nsIPresShell> presShell(do_QueryReferent(mWeakShell));
@@ -109,38 +109,38 @@ nsAccTreeWalker::GetNextChildInternal(PR
   if (mState->childList)
     mState->childList->GetLength(&length);
 
   while (mState->childIdx < length) {
     nsIContent* childNode = mState->childList->GetNodeAt(mState->childIdx);
     mState->childIdx++;
 
     bool isSubtreeHidden = false;
-    nsRefPtr<nsAccessible> accessible =
+    nsAccessible* accessible =
       GetAccService()->GetOrCreateAccessible(childNode, presShell, mWeakShell,
                                              &isSubtreeHidden);
 
     if (accessible)
-      return accessible.forget();
+      return accessible;
 
     // Walk down into subtree to find accessibles.
     if (!isSubtreeHidden) {
       if (!PushState(childNode))
         break;
 
-      accessible = GetNextChildInternal(PR_TRUE);
+      accessible = NextChildInternal(true);
       if (accessible)
-        return accessible.forget();
+        return accessible;
     }
   }
 
   // No more children, get back to the parent.
   PopState();
 
-  return aNoWalkUp ? nsnull : GetNextChildInternal(PR_FALSE);
+  return aNoWalkUp ? nsnull : NextChildInternal(false);
 }
 
 void
 nsAccTreeWalker::PopState()
 {
   WalkState* prevToLastState = mState->prevState;
   delete mState;
   mState = prevToLastState;
--- a/accessible/src/base/nsAccTreeWalker.h
+++ b/accessible/src/base/nsAccTreeWalker.h
@@ -54,33 +54,36 @@ class nsAccTreeWalker
 {
 public:
   nsAccTreeWalker(nsIWeakReference *aShell, nsIContent *aNode, 
                   PRBool aWalkAnonymousContent);
   virtual ~nsAccTreeWalker();
 
   /**
    * Return the next child accessible.
+   *
+   * @note Returned accessible is bound to the document, if the accessible is
+   *       rejected during tree creation then the caller should be unbind it
+   *       from the document.
    */
-  already_AddRefed<nsAccessible> GetNextChild()
+  inline nsAccessible* NextChild()
   {
-    return GetNextChildInternal(PR_FALSE);
+    return NextChildInternal(false);
   }
 
 private:
 
   /**
    * 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.
    */
-  already_AddRefed<nsAccessible>
-    GetNextChildInternal(PRBool aNoWalkUp = PR_FALSE);
+  nsAccessible* 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.
    */
   PRBool PushState(nsIContent *aNode);
--- a/accessible/src/base/nsAccessibilityService.cpp
+++ b/accessible/src/base/nsAccessibilityService.cpp
@@ -853,44 +853,40 @@ static PRBool HasRelatedContent(nsIConte
         // ancestor has activedescendant property, this content could be active
       return PR_TRUE;
     }
   }
 
   return PR_FALSE;
 }
 
-already_AddRefed<nsAccessible>
+nsAccessible*
 nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
                                               nsIPresShell* aPresShell,
                                               nsIWeakReference* aWeakShell,
                                               bool* aIsSubtreeHidden)
 {
   if (!aPresShell || !aWeakShell || !aNode || gIsShutdown)
     return nsnull;
 
   if (aIsSubtreeHidden)
     *aIsSubtreeHidden = false;
 
   // Check to see if we already have an accessible for this node in the cache.
   nsAccessible* cachedAccessible = GetAccessibleInWeakShell(aNode, aWeakShell);
-  if (cachedAccessible) {
-    NS_ADDREF(cachedAccessible);
+  if (cachedAccessible)
     return cachedAccessible;
-  }
 
   // No cache entry, so we must create the accessible.
 
   if (aNode->IsNodeOfType(nsINode::eDOCUMENT)) {
     // If it's document node then ask accessible document loader for
     // document accessible, otherwise return null.
     nsCOMPtr<nsIDocument> document(do_QueryInterface(aNode));
-    nsAccessible *accessible = GetDocAccessible(document);
-    NS_IF_ADDREF(accessible);
-    return accessible;
+    return GetDocAccessible(document);
   }
 
   // We have a content node.
   if (!aNode->IsInDoc()) {
     NS_WARNING("Creating accessible for node with no document");
     return nsnull;
   }
 
@@ -919,26 +915,23 @@ nsAccessibilityService::GetOrCreateAcces
   }
 
   if (weakFrame.GetFrame()->GetContent() != content) {
     // Not the main content for this frame. This happens because <area>
     // elements return the image frame as their primary frame. The main content
     // for the image frame is the image content. If the frame is not an image
     // frame or the node is not an area element then null is returned.
     // This setup will change when bug 135040 is fixed.
-    nsAccessible* areaAcc = GetAreaAccessible(weakFrame.GetFrame(),
-                                              aNode, aWeakShell);
-    NS_IF_ADDREF(areaAcc);
-    return areaAcc;
+    return GetAreaAccessible(weakFrame.GetFrame(), aNode, aWeakShell);
   }
 
   nsDocAccessible* docAcc =
     GetAccService()->GetDocAccessible(aNode->GetOwnerDoc());
   if (!docAcc) {
-    NS_NOTREACHED("No document for accessible being created!");
+    NS_NOTREACHED("Node has no host document accessible!");
     return nsnull;
   }
 
   // Attempt to create an accessible based on what we know.
   nsRefPtr<nsAccessible> newAcc;
 
   // Create accessible for visible text frames.
   if (content->IsNodeOfType(nsINode::eTEXT)) {
@@ -949,17 +942,17 @@ nsAccessibilityService::GetOrCreateAcces
         *aIsSubtreeHidden = true;
 
       return nsnull;
     }
 
     newAcc = weakFrame->CreateAccessible();
     if (docAcc->BindToDocument(newAcc, nsnull)) {
       newAcc->AsTextLeaf()->SetText(text);
-      return newAcc.forget();
+      return newAcc;
     }
 
     return nsnull;
   }
 
   PRBool isHTML = content->IsHTML();
   if (isHTML && content->Tag() == nsAccessibilityAtoms::map) {
     // Create hyper text accessible for HTML map if it is used to group links
@@ -976,17 +969,17 @@ nsAccessibilityService::GetOrCreateAcces
       if (aIsSubtreeHidden)
         *aIsSubtreeHidden = true;
 
       return nsnull;
     }
 
     newAcc = new nsHyperTextAccessibleWrap(content, aWeakShell);
     if (docAcc->BindToDocument(newAcc, nsAccUtils::GetRoleMapEntry(aNode)))
-      return newAcc.forget();
+      return newAcc;
     return nsnull;
   }
 
   nsRoleMapEntry *roleMapEntry = nsAccUtils::GetRoleMapEntry(aNode);
   if (roleMapEntry && !nsCRT::strcmp(roleMapEntry->roleString, "presentation") &&
       !content->IsFocusable()) { // For presentation only
     // Only create accessible for role of "presentation" if it is focusable --
     // in that case we need an accessible in case it gets focused, we
@@ -1162,19 +1155,17 @@ nsAccessibilityService::GetOrCreateAcces
       newAcc = new nsHyperTextAccessibleWrap(content, aWeakShell);
     }
     else {  // XUL, SVG, MathML etc.
       // Interesting generic non-HTML container
       newAcc = new nsAccessibleWrap(content, aWeakShell);
     }
   }
 
-  if (docAcc->BindToDocument(newAcc, roleMapEntry))
-    return newAcc.forget();
-  return nsnull;
+  return docAcc->BindToDocument(newAcc, roleMapEntry) ? newAcc : nsnull;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsAccessibilityService private
 
 PRBool
 nsAccessibilityService::Init()
 {
--- a/accessible/src/base/nsAccessibilityService.h
+++ b/accessible/src/base/nsAccessibilityService.h
@@ -153,20 +153,19 @@ public:
    * one.
    *
    * @param  aNode             [in] the given node
    * @param  aPresShell        [in] the pres shell of the node
    * @param  aWeakShell        [in] the weak shell for the pres shell
    * @param  aIsSubtreeHidden  [out, optional] indicates whether the node's
    *                             frame and its subtree is hidden
    */
-  already_AddRefed<nsAccessible>
-    GetOrCreateAccessible(nsINode* aNode, nsIPresShell* aPresShell,
-                          nsIWeakReference* aWeakShell,
-                          bool* aIsSubtreeHidden = nsnull);
+  nsAccessible* GetOrCreateAccessible(nsINode* aNode, nsIPresShell* aPresShell,
+                                      nsIWeakReference* aWeakShell,
+                                      bool* aIsSubtreeHidden = nsnull);
 
   /**
    * Return an accessible for the given DOM node.
    */
   nsAccessible* GetAccessible(nsINode* aNode);
 
   /**
    * Return an accessible for a DOM node in the given presshell.
--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@@ -3117,18 +3117,18 @@ nsAccessible::UnselectAll()
 ////////////////////////////////////////////////////////////////////////////////
 // nsAccessible protected methods
 
 void
 nsAccessible::CacheChildren()
 {
   nsAccTreeWalker walker(mWeakShell, mContent, GetAllowsAnonChildAccessibles());
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild()) && AppendChild(child));
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild()) && AppendChild(child));
 }
 
 void
 nsAccessible::TestChildCache(nsAccessible* aCachedChild) const
 {
 #ifdef DEBUG
   PRInt32 childCount = mChildren.Length();
   if (childCount == 0) {
--- a/accessible/src/base/nsDocAccessible.cpp
+++ b/accessible/src/base/nsDocAccessible.cpp
@@ -1491,18 +1491,18 @@ nsDocAccessible::NotifyOfCachingEnd(nsAc
 void
 nsDocAccessible::CacheChildren()
 {
   // Search for accessible children starting from the document element since
   // some web pages tend to insert elements under it rather than document body.
   nsAccTreeWalker walker(mWeakShell, mDocument->GetRootElement(),
                          GetAllowsAnonChildAccessibles());
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild()) && AppendChild(child));
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild()) && AppendChild(child));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Protected members
 
 void
 nsDocAccessible::NotifyOfInitialUpdate()
 {
--- a/accessible/src/html/nsHTMLTableAccessible.cpp
+++ b/accessible/src/html/nsHTMLTableAccessible.cpp
@@ -434,21 +434,21 @@ void
 nsHTMLTableAccessible::CacheChildren()
 {
   // Move caption accessible so that it's the first child. Check for the first
   // caption only, because nsAccessibilityService ensures we don't create
   // accessibles for the other captions, since only the first is actually
   // visible.
   nsAccTreeWalker walker(mWeakShell, mContent, GetAllowsAnonChildAccessibles());
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild())) {
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild())) {
     if (child->Role() == nsIAccessibleRole::ROLE_CAPTION) {
       InsertChildAt(0, child);
-      while ((child = walker.GetNextChild()) && AppendChild(child));
+      while ((child = walker.NextChild()) && AppendChild(child));
       break;
     }
     AppendChild(child);
   }
 }
 
 PRUint32
 nsHTMLTableAccessible::NativeRole()
--- a/accessible/src/xforms/nsXFormsAccessible.cpp
+++ b/accessible/src/xforms/nsXFormsAccessible.cpp
@@ -125,17 +125,17 @@ nsXFormsAccessible::CacheSelectChildren(
 
   for (PRUint32 index = 0; index < length; index++) {
     nsCOMPtr<nsIDOMNode> DOMChild;
     children->Item(index, getter_AddRefs(DOMChild));
     if (!DOMChild)
       continue;
 
     nsCOMPtr<nsIContent> child(do_QueryInterface(DOMChild));
-    nsRefPtr<nsAccessible> accessible =
+    nsAccessible* accessible =
       GetAccService()->GetOrCreateAccessible(child, presShell, mWeakShell);
     if (!accessible)
       continue;
 
     AppendChild(accessible);
   }
 }
 
--- a/accessible/src/xul/nsXULColorPickerAccessible.cpp
+++ b/accessible/src/xul/nsXULColorPickerAccessible.cpp
@@ -158,21 +158,21 @@ nsXULColorPickerAccessible::NativeRole()
 ////////////////////////////////////////////////////////////////////////////////
 // nsXULColorPickerAccessible: protected nsAccessible
 
 void
 nsXULColorPickerAccessible::CacheChildren()
 {
   nsAccTreeWalker walker(mWeakShell, mContent, PR_TRUE);
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild())) {
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild())) {
     PRUint32 role = child->Role();
 
-    // Get an accessbile for menupopup or panel elements.
+    // Get an accessible for menupopup or panel elements.
     if (role == nsIAccessibleRole::ROLE_ALERT) {
       AppendChild(child);
       return;
     }
 
     // Unbind rejected accessibles from the document.
     GetDocAccessible()->UnbindFromDocument(child);
   }
--- a/accessible/src/xul/nsXULFormControlAccessible.cpp
+++ b/accessible/src/xul/nsXULFormControlAccessible.cpp
@@ -200,47 +200,47 @@ nsXULButtonAccessible::CacheChildren()
   PRBool isMenuButton = isMenu ?
     PR_FALSE :
     mContent->AttrValueIs(kNameSpaceID_None, nsAccessibilityAtoms::type,
                           nsAccessibilityAtoms::menuButton, eCaseMatters);
 
   if (!isMenu && !isMenuButton)
     return;
 
-  nsRefPtr<nsAccessible> menupopupAccessible;
-  nsRefPtr<nsAccessible> buttonAccessible;
+  nsAccessible* menupopup = nsnull;
+  nsAccessible* button = nsnull;
 
   nsAccTreeWalker walker(mWeakShell, mContent, PR_TRUE);
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild())) {
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild())) {
     PRUint32 role = child->Role();
 
     if (role == nsIAccessibleRole::ROLE_MENUPOPUP) {
-      // Get an accessbile for menupopup or panel elements.
-      menupopupAccessible.swap(child);
+      // Get an accessible for menupopup or panel elements.
+      menupopup = child;
 
     } else if (isMenuButton && role == nsIAccessibleRole::ROLE_PUSHBUTTON) {
       // Button type="menu-button" contains a real button. Get an accessible
-      // for it. Ignore dropmarker button what is placed as a last child.
-      buttonAccessible.swap(child);
+      // for it. Ignore dropmarker button which is placed as a last child.
+      button = child;
       break;
 
     } else {
       // Unbind rejected accessible from document.
       GetDocAccessible()->UnbindFromDocument(child);
     }
   }
 
-  if (!menupopupAccessible)
+  if (!menupopup)
     return;
 
-  AppendChild(menupopupAccessible);
-  if (buttonAccessible)
-    AppendChild(buttonAccessible);
+  AppendChild(menupopup);
+  if (button)
+    AppendChild(button);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsXULButtonAccessible protected
 
 PRBool
 nsXULButtonAccessible::ContainsMenu()
 {
@@ -1040,18 +1040,18 @@ nsXULTextFieldAccessible::CacheChildren(
   // Create child accessibles for native anonymous content of underlying HTML
   // input element.
   nsCOMPtr<nsIContent> inputContent(GetInputField());
   if (!inputContent)
     return;
 
   nsAccTreeWalker walker(mWeakShell, inputContent, PR_FALSE);
 
-  nsRefPtr<nsAccessible> child;
-  while ((child = walker.GetNextChild()) && AppendChild(child));
+  nsAccessible* child = nsnull;
+  while ((child = walker.NextChild()) && AppendChild(child));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsXULTextFieldAccessible protected
 
 already_AddRefed<nsIContent>
 nsXULTextFieldAccessible::GetInputField() const
 {