Bug 566298 - don't use nsIAccessible::GetNextSibling() to iterate children, r=davidb, sr=neil
authorAlexander Surkov <surkov.alexander@gmail.com>
Tue, 18 May 2010 23:03:56 +0900
changeset 42415 49ca6b8821f26a12fb3283dd58fb71847775ec1a
parent 42414 b4aa7d59c96e3b7f0098f7639e3ed4a78928d156
child 42416 6861c31f2ec6e3cb2f50fa90d54d959146b29ccb
push id13328
push usersurkov.alexander@gmail.com
push dateTue, 18 May 2010 14:04:57 +0000
treeherdermozilla-central@49ca6b8821f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdavidb, neil
bugs566298
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 566298 - don't use nsIAccessible::GetNextSibling() to iterate children, r=davidb, sr=neil
accessible/src/atk/nsAccessibleWrap.cpp
accessible/src/base/nsARIAGridAccessible.cpp
accessible/src/base/nsAccUtils.cpp
accessible/src/base/nsAccUtils.h
accessible/src/base/nsDocAccessible.cpp
accessible/src/base/nsTextEquivUtils.cpp
accessible/src/msaa/nsAccessibleWrap.cpp
accessible/src/xforms/nsXFormsAccessible.cpp
accessible/src/xul/nsXULListboxAccessible.cpp
--- a/accessible/src/atk/nsAccessibleWrap.cpp
+++ b/accessible/src/atk/nsAccessibleWrap.cpp
@@ -938,46 +938,36 @@ getIndexInParentCB(AtkObject *aAtkObj)
 {
     // We don't use nsIAccessible::GetIndexInParent() because
     // for ATK we don't want to include text leaf nodes as children
     nsAccessibleWrap *accWrap = GetAccessibleWrap(aAtkObj);
     if (!accWrap) {
         return -1;
     }
 
-    nsCOMPtr<nsIAccessible> parent;
-    accWrap->GetParent(getter_AddRefs(parent));
+    nsAccessible *parent = accWrap->GetParent();
     if (!parent) {
         return -1; // No parent
     }
 
-    nsCOMPtr<nsIAccessible> sibling;
-    parent->GetFirstChild(getter_AddRefs(sibling));
-    if (!sibling) {
-        return -1;  // Error, parent has no children
-    }
-
     PRInt32 currentIndex = 0;
 
-    while (sibling != static_cast<nsIAccessible*>(accWrap)) {
-      NS_ASSERTION(sibling, "Never ran into the same child that we started from");
-
-      if (!sibling) {
-          return -1;
-      }
-      if (nsAccUtils::IsEmbeddedObject(sibling)) {
-        ++ currentIndex;
+    PRInt32 childCount = parent->GetChildCount();
+    for (PRInt32 idx = 0; idx < childCount; idx++) {
+      nsAccessible *sibling = parent->GetChildAt(idx);
+      if (sibling == accWrap) {
+          return currentIndex;
       }
 
-      nsCOMPtr<nsIAccessible> tempAccessible;
-      sibling->GetNextSibling(getter_AddRefs(tempAccessible));
-      sibling.swap(tempAccessible);
+      if (nsAccUtils::IsEmbeddedObject(sibling)) {
+          ++ currentIndex;
+      }
     }
 
-    return currentIndex;
+    return -1;
 }
 
 static void TranslateStates(PRUint32 aState, const AtkStateMap *aStateMap,
                             AtkStateSet *aStateSet)
 {
   NS_ASSERTION(aStateSet, "Can't pass in null state set");
 
   // Convert every state to an entry in AtkStateMap
--- a/accessible/src/base/nsARIAGridAccessible.cpp
+++ b/accessible/src/base/nsARIAGridAccessible.cpp
@@ -1125,52 +1125,48 @@ nsARIAGridCellAccessible::GetAttributesI
   if (IsDefunct())
     return NS_ERROR_FAILURE;
   
   nsresult rv = nsHyperTextAccessibleWrap::GetAttributesInternal(aAttributes);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Expose "table-cell-index" attribute.
 
-  nsCOMPtr<nsIAccessible> thisRow;
-  GetParent(getter_AddRefs(thisRow));
+  nsAccessible *thisRow = GetParent();
   if (nsAccUtils::Role(thisRow) != nsIAccessibleRole::ROLE_ROW)
     return NS_OK;
 
   PRInt32 colIdx = 0, colCount = 0;
-  nsCOMPtr<nsIAccessible> child, nextChild;
-  thisRow->GetFirstChild(getter_AddRefs(child));
-  while (child) {
+  PRInt32 childCount = thisRow->GetChildCount();
+  for (PRInt32 childIdx = 0; childIdx < childCount; childIdx++) {
+    nsAccessible *child = thisRow->GetChildAt(childIdx);
     if (child == this)
       colIdx = colCount;
 
     PRUint32 role = nsAccUtils::Role(child);
     if (role == nsIAccessibleRole::ROLE_GRID_CELL ||
         role == nsIAccessibleRole::ROLE_ROWHEADER ||
         role == nsIAccessibleRole::ROLE_COLUMNHEADER)
       colCount++;
-
-    child->GetNextSibling(getter_AddRefs(nextChild));
-    child.swap(nextChild);
   }
 
-  nsCOMPtr<nsIAccessible> table;
-  thisRow->GetParent(getter_AddRefs(table));
+  nsAccessible *table = thisRow->GetParent();
   if (nsAccUtils::Role(table) != nsIAccessibleRole::ROLE_TABLE &&
       nsAccUtils::Role(table) != nsIAccessibleRole::ROLE_TREE_TABLE)
     return NS_OK;
 
   PRInt32 rowIdx = 0;
-  table->GetFirstChild(getter_AddRefs(child));
-  while (child && child != thisRow) {
+  childCount = table->GetChildCount();
+  for (PRInt32 childIdx = 0; childIdx < childCount; childIdx++) {
+    nsAccessible *child = table->GetChildAt(childIdx);
+    if (child == thisRow)
+      break;
+
     if (nsAccUtils::Role(child) == nsIAccessibleRole::ROLE_ROW)
       rowIdx++;
-
-    child->GetNextSibling(getter_AddRefs(nextChild));
-    child.swap(nextChild);
   }
 
   PRInt32 idx = rowIdx * colCount + colIdx;
 
   nsAutoString stringIdx;
   stringIdx.AppendInt(idx);
   nsAccUtils::SetAccAttr(aAttributes, nsAccessibilityAtoms::tableCellIndex,
                          stringIdx);
--- a/accessible/src/base/nsAccUtils.cpp
+++ b/accessible/src/base/nsAccUtils.cpp
@@ -784,41 +784,39 @@ nsAccUtils::GetLiveAttrValue(PRUint32 aR
   }
 
   return PR_FALSE;
 }
 
 #ifdef DEBUG_A11Y
 
 PRBool
-nsAccUtils::IsTextInterfaceSupportCorrect(nsIAccessible *aAccessible)
+nsAccUtils::IsTextInterfaceSupportCorrect(nsAccessible *aAccessible)
 {
   PRBool foundText = PR_FALSE;
   
-  nsCOMPtr<nsIAccessibleDocument> accDoc = do_QueryInterface(aAccessible);
+  nsCOMPtr<nsIAccessibleDocument> accDoc = do_QueryObject(aAccessible);
   if (accDoc) {
     // Don't test for accessible docs, it makes us create accessibles too
     // early and fire mutation events before we need to
     return PR_TRUE;
   }
 
-  nsCOMPtr<nsIAccessible> child, nextSibling;
-  aAccessible->GetFirstChild(getter_AddRefs(child));
-  while (child) {
+  PRInt32 childCount = aAccessible->GetChildCount();
+  for (PRint32 childIdx = 0; childIdx < childCount; childIdx++) {
+    nsAccessible *child = GetChildAt(childIdx);
     if (IsText(child)) {
       foundText = PR_TRUE;
       break;
     }
-    child->GetNextSibling(getter_AddRefs(nextSibling));
-    child.swap(nextSibling);
   }
 
   if (foundText) {
     // found text child node
-    nsCOMPtr<nsIAccessibleText> text = do_QueryInterface(aAccessible);
+    nsCOMPtr<nsIAccessibleText> text = do_QueryObject(aAccessible);
     if (!text)
       return PR_FALSE;
   }
 
   return PR_TRUE; 
 }
 #endif
 
--- a/accessible/src/base/nsAccUtils.h
+++ b/accessible/src/base/nsAccUtils.h
@@ -319,17 +319,17 @@ public:
    */
   static PRBool GetLiveAttrValue(PRUint32 aRule, nsAString& aValue);
 
 #ifdef DEBUG_A11Y
   /**
    * Detect whether the given accessible object implements nsIAccessibleText,
    * when it is text or has text child node.
    */
-  static PRBool IsTextInterfaceSupportCorrect(nsIAccessible *aAccessible);
+  static PRBool IsTextInterfaceSupportCorrect(nsAccessible *aAccessible);
 #endif
 
   /**
    * Return true if the given accessible has text role.
    */
   static PRBool IsText(nsIAccessible *aAcc)
   {
     PRUint32 role = Role(aAcc);
--- a/accessible/src/base/nsDocAccessible.cpp
+++ b/accessible/src/base/nsDocAccessible.cpp
@@ -1798,19 +1798,18 @@ void nsDocAccessible::RefreshNodes(nsIDO
                                 accessible);
       }
     }
 
     // We only need to shutdown the accessibles here if one of them has been
     // created.
     if (accessible->GetCachedFirstChild()) {
       nsCOMPtr<nsIArray> children;
-      // use GetChildren() to fetch children at one time, instead of using
-      // GetNextSibling(), because after we shutdown the first child,
-      // mNextSibling will be set null.
+      // use GetChildren() to fetch all children at once, because after shutdown
+      // the child references are cleared.
       accessible->GetChildren(getter_AddRefs(children));
       PRUint32 childCount =0;
       if (children)
         children->GetLength(&childCount);
       nsCOMPtr<nsIDOMNode> possibleAnonNode;
       for (PRUint32 index = 0; index < childCount; index++) {
         nsCOMPtr<nsIAccessNode> childAccessNode;
         children->QueryElementAt(index, NS_GET_IID(nsIAccessNode),
--- a/accessible/src/base/nsTextEquivUtils.cpp
+++ b/accessible/src/base/nsTextEquivUtils.cpp
@@ -231,26 +231,24 @@ nsTextEquivUtils::AppendTextEquivFromTex
 // nsTextEquivUtils. Private.
 
 nsCOMPtr<nsIAccessible> nsTextEquivUtils::gInitiatorAcc;
 
 nsresult
 nsTextEquivUtils::AppendFromAccessibleChildren(nsIAccessible *aAccessible,
                                                nsAString *aString)
 {
-  nsCOMPtr<nsIAccessible> accChild, accNextChild;
-  aAccessible->GetFirstChild(getter_AddRefs(accChild));
+  nsresult rv = NS_OK_NO_NAME_CLAUSE_HANDLED;
 
-  nsresult rv = NS_OK_NO_NAME_CLAUSE_HANDLED;
-  while (accChild) {
-    rv = AppendFromAccessible(accChild, aString);
+  nsRefPtr<nsAccessible> accessible(do_QueryObject(aAccessible));
+  PRInt32 childCount = accessible->GetChildCount();
+  for (PRInt32 childIdx = 0; childIdx < childCount; childIdx++) {
+    nsAccessible *child = accessible->GetChildAt(childIdx);
+    rv = AppendFromAccessible(child, aString);
     NS_ENSURE_SUCCESS(rv, rv);
-
-    accChild->GetNextSibling(getter_AddRefs(accNextChild));
-    accChild.swap(accNextChild);
   }
 
   return rv;
 }
 
 nsresult
 nsTextEquivUtils::AppendFromAccessible(nsIAccessible *aAccessible,
                                        nsAString *aString)
--- a/accessible/src/msaa/nsAccessibleWrap.cpp
+++ b/accessible/src/msaa/nsAccessibleWrap.cpp
@@ -385,36 +385,31 @@ nsAccessibleWrap::get_accDescription(VAR
     if (groupLevel > 0) {
       // XXX: How do we calculate the number of children? Now we append
       // " with [numChildren]c" for tree item. In the future we may need to
       // use the ARIA owns property to calculate that if it's present.
       PRInt32 numChildren = 0;
 
       PRUint32 currentRole = nsAccUtils::Role(xpAccessible);
       if (currentRole == nsIAccessibleRole::ROLE_OUTLINEITEM) {
-        nsCOMPtr<nsIAccessible> child;
-        xpAccessible->GetFirstChild(getter_AddRefs(child));
-        while (child) {
+        PRInt32 childCount = xpAccessible->GetChildCount();
+        for (PRInt32 childIdx = 0; childIdx < childCount; childIdx++) {
+          nsAccessible *child = xpAccessible->GetChildAt(childIdx);
           currentRole = nsAccUtils::Role(child);
           if (currentRole == nsIAccessibleRole::ROLE_GROUPING) {
-            nsCOMPtr<nsIAccessible> groupChild;
-            child->GetFirstChild(getter_AddRefs(groupChild));
-            while (groupChild) {
+            PRInt32 groupChildCount = child->GetChildCount();
+            for (PRInt32 groupChildIdx = 0; groupChildIdx < groupChildCount;
+                 groupChildIdx++) {
+              nsAccessible *groupChild = child->GetChildAt(groupChildIdx);
               currentRole = nsAccUtils::Role(groupChild);
               numChildren +=
                 (currentRole == nsIAccessibleRole::ROLE_OUTLINEITEM);
-              nsCOMPtr<nsIAccessible> nextGroupChild;
-              groupChild->GetNextSibling(getter_AddRefs(nextGroupChild));
-              groupChild.swap(nextGroupChild);
             }
             break;
           }
-          nsCOMPtr<nsIAccessible> nextChild;
-          child->GetNextSibling(getter_AddRefs(nextChild));
-          child.swap(nextChild);
         }
       }
 
       if (numChildren) {
         nsTextFormatter::ssprintf(description,
                                   NS_LITERAL_STRING("L%d, %d of %d with %d").get(),
                                   groupLevel, positionInGroup, itemsInGroup,
                                   numChildren);
--- a/accessible/src/xforms/nsXFormsAccessible.cpp
+++ b/accessible/src/xforms/nsXFormsAccessible.cpp
@@ -548,49 +548,42 @@ nsXFormsSelectableAccessible::SelectAllS
   *aMultipleSelection = PR_TRUE;
   return sXFormsService->SelectAllItemsForSelect(mDOMNode);
 }
 
 already_AddRefed<nsIDOMNode>
 nsXFormsSelectableAccessible::GetItemByIndex(PRInt32 *aIndex,
                                              nsIAccessible *aAccessible)
 {
-  nsCOMPtr<nsIAccessible> accessible(aAccessible ? aAccessible : this);
+  nsRefPtr<nsAccessible> accessible(do_QueryObject(aAccessible));
+  if (!accessible)
+    accessible = this;
 
-  nsCOMPtr<nsIAccessible> curAccChild;
-  accessible->GetFirstChild(getter_AddRefs(curAccChild));
+  PRInt32 childCount = accessible->GetChildCount();
+  for (PRInt32 childIdx = 0; childIdx < childCount; childIdx++) {
+    nsAccessible *child = accessible->GetChildAt(childIdx);
 
-  while (curAccChild) {
-    nsCOMPtr<nsIAccessNode> curAccNodeChild(do_QueryInterface(curAccChild));
-    if (curAccNodeChild) {
-      nsCOMPtr<nsIDOMNode> curChildNode;
-      curAccNodeChild->GetDOMNode(getter_AddRefs(curChildNode));
-      nsCOMPtr<nsIContent> curChildContent(do_QueryInterface(curChildNode));
-      if (curChildContent) {
-        nsCOMPtr<nsINodeInfo> nodeInfo = curChildContent->NodeInfo();
-        if (nodeInfo->NamespaceEquals(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS))) {
-          if (nodeInfo->Equals(nsAccessibilityAtoms::item)) {
-            if (!*aIndex) {
-              nsIDOMNode *itemNode = nsnull;
-              curChildNode.swap(itemNode);
-              return itemNode;
-            }
-            --*aIndex;
-          } else if (nodeInfo->Equals(nsAccessibilityAtoms::choices)) {
-            nsIDOMNode *itemNode = GetItemByIndex(aIndex, curAccChild).get();
-            if (itemNode)
-              return itemNode;
-          }
-        }
+    nsCOMPtr<nsIDOMNode> childNode(child->GetDOMNode());
+    nsCOMPtr<nsIContent> childContent(do_QueryInterface(childNode));
+    if (!childContent)
+      continue;
+
+    nsINodeInfo *nodeInfo = childContent->NodeInfo();
+    if (nodeInfo->NamespaceEquals(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS))) {
+      if (nodeInfo->Equals(nsAccessibilityAtoms::item)) {
+        if (!*aIndex)
+          return childNode.forget();
+
+        --*aIndex;
+      } else if (nodeInfo->Equals(nsAccessibilityAtoms::choices)) {
+        nsIDOMNode *itemNode = GetItemByIndex(aIndex, child).get();
+        if (itemNode)
+          return itemNode;
       }
     }
-
-    nsCOMPtr<nsIAccessible> nextAccChild;
-    curAccChild->GetNextSibling(getter_AddRefs(nextAccChild));
-    curAccChild.swap(nextAccChild);
   }
 
   return nsnull;
 }
 
 
 // nsXFormsSelectableItemAccessible
 
--- a/accessible/src/xul/nsXULListboxAccessible.cpp
+++ b/accessible/src/xul/nsXULListboxAccessible.cpp
@@ -1144,35 +1144,33 @@ nsXULListCellAccessible::GetColumnHeader
   if (IsDefunct())
     return NS_ERROR_FAILURE;
 
   nsCOMPtr<nsIAccessibleTable> table;
   GetTable(getter_AddRefs(table));
   NS_ENSURE_STATE(table); // we expect to be in a listbox (table)
 
   // Get column header cell from XUL listhead.
-  nsCOMPtr<nsIAccessible> tableAcc(do_QueryInterface(table));
+  nsAccessible *list = nsnull;
 
-  nsCOMPtr<nsIAccessible> list, nextChild;
-  tableAcc->GetFirstChild(getter_AddRefs(list));
-  while (list) {
-    if (nsAccUtils::Role(list) == nsIAccessibleRole::ROLE_LIST)
+  nsRefPtr<nsAccessible> tableAcc(do_QueryObject(table));
+  PRInt32 tableChildCount = tableAcc->GetChildCount();
+  for (PRInt32 childIdx = 0; childIdx < tableChildCount; childIdx++) {
+    nsAccessible *child = tableAcc->GetChildAt(childIdx);
+    if (nsAccUtils::Role(child) == nsIAccessibleRole::ROLE_LIST) {
+      list = child;
       break;
-
-    list->GetNextSibling(getter_AddRefs(nextChild));
-    nextChild.swap(list);
+    }
   }
 
   if (list) {
     PRInt32 colIdx = -1;
     GetColumnIndex(&colIdx);
 
-    nsCOMPtr<nsIAccessible> headerCell;
-    list->GetChildAt(colIdx, getter_AddRefs(headerCell));
-
+    nsIAccessible *headerCell = list->GetChildAt(colIdx);
     if (headerCell) {
       nsresult rv = NS_OK;
       nsCOMPtr<nsIMutableArray> headerCells =
         do_CreateInstance(NS_ARRAY_CONTRACTID, &rv);
       NS_ENSURE_SUCCESS(rv, rv);
 
       headerCells->AppendElement(headerCell, PR_FALSE);
       NS_ADDREF(*aHeaderCells = headerCells);