Bug 1530207 - fix crash [@ InvalidArrayIndex_CRASH | mozilla::a11y::Accessible::InsertChildAt]. r=surkov a=jorgk DONTBUILD THUNDERBIRD670b2_2019042501_RELBRANCH
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 18 Apr 2019 09:39:59 +0000
branchTHUNDERBIRD670b2_2019042501_RELBRANCH
changeset 523352 7243480d39dc3a57e6f81d3b6bf8cc4f6cf062d5
parent 523339 151cf24aa0fe45af946282b275e30deb908af841
push id11157
push usermozilla@jorgk.com
push dateThu, 25 Apr 2019 10:01:37 +0000
treeherdermozilla-beta@7243480d39dc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov, jorgk
bugs1530207
milestone67.0
Bug 1530207 - fix crash [@ InvalidArrayIndex_CRASH | mozilla::a11y::Accessible::InsertChildAt]. r=surkov a=jorgk DONTBUILD For XULTreeAccessible, the ChildCount() is not only the mChildren, so check mChildren directly to make sure we stay within bounds Differential Revision: https://phabricator.services.mozilla.com/D27553
accessible/generic/DocAccessible.cpp
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -2063,18 +2063,21 @@ void DocAccessible::PutChildrenBack(nsTA
 
   aChildren->RemoveElementsAt(aStartIdx, aChildren->Length() - aStartIdx);
 }
 
 bool DocAccessible::MoveChild(Accessible* aChild, Accessible* aNewParent,
                               int32_t aIdxInParent) {
   MOZ_ASSERT(aChild, "No child");
   MOZ_ASSERT(aChild->Parent(), "No parent");
-  MOZ_ASSERT(aIdxInParent <= static_cast<int32_t>(aNewParent->ChildCount()),
-             "Wrong insertion point for a moving child");
+  // We can't guarantee MoveChild works correctly for accessibilities storing
+  // children outside mChildren.
+  MOZ_ASSERT(
+      aIdxInParent <= static_cast<int32_t>(aNewParent->mChildren.Length()),
+      "Wrong insertion point for a moving child");
 
   Accessible* curParent = aChild->Parent();
 
   if (!aNewParent->IsAcceptableChild(aChild->GetContent())) {
     return false;
   }
 
 #ifdef A11Y_LOG
@@ -2102,24 +2105,21 @@ bool DocAccessible::MoveChild(Accessible
 
 #ifdef A11Y_LOG
     logging::TreeInfo("move child: parent tree after", logging::eVerbose,
                       curParent);
 #endif
     return true;
   }
 
-  MOZ_ASSERT(aIdxInParent <= static_cast<int32_t>(aNewParent->ChildCount()),
-             "Wrong insertion point for a moving child");
-
   // If the child cannot be re-inserted into the tree, then make sure to remove
   // it from its present parent and then shutdown it.
   bool hasInsertionPoint =
-      (aIdxInParent != -1) ||
-      (aIdxInParent <= static_cast<int32_t>(aNewParent->ChildCount()));
+      (aIdxInParent >= 0) &&
+      (aIdxInParent <= static_cast<int32_t>(aNewParent->mChildren.Length()));
 
   TreeMutation rmut(curParent);
   rmut.BeforeRemoval(aChild, hasInsertionPoint && TreeMutation::kNoShutdown);
   curParent->RemoveChild(aChild);
   rmut.Done();
 
   // No insertion point for the child.
   if (!hasInsertionPoint) {
@@ -2150,17 +2150,17 @@ void DocAccessible::CacheChildrenInSubtr
     *aFocusedAcc = aRoot;
 
   Accessible* root = aRoot->IsHTMLCombobox() ? aRoot->FirstChild() : aRoot;
   if (root->KidsFromDOM()) {
     TreeMutation mt(root, TreeMutation::kNoEvents);
     TreeWalker walker(root);
     while (Accessible* child = walker.Next()) {
       if (child->IsBoundToParent()) {
-        MoveChild(child, root, root->ChildCount());
+        MoveChild(child, root, root->mChildren.Length());
         continue;
       }
 
       root->AppendChild(child);
       mt.AfterInsertion(child);
 
       CacheChildrenInSubtree(child, aFocusedAcc);
     }