Bug 1530207 - fix crash [@ InvalidArrayIndex_CRASH | mozilla::a11y::Accessible::InsertChildAt] . r=surkov
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 18 Apr 2019 09:39:59 +0000
changeset 470043 5982eef0c2cb7ca74ab2461b5601cad68caf49c4
parent 470042 51423343879f6ea29b96b2c3506cc2fb3fd16e60
child 470044 313aab6b8a05c0cfd63bc11e4c514ff39f92ad13
push id112839
push userapavel@mozilla.com
push dateThu, 18 Apr 2019 21:50:57 +0000
treeherdermozilla-inbound@e0a826fcd85b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssurkov
bugs1530207
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1530207 - fix crash [@ InvalidArrayIndex_CRASH | mozilla::a11y::Accessible::InsertChildAt] . r=surkov 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
@@ -2085,18 +2085,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
@@ -2124,24 +2127,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) {
@@ -2172,17 +2172,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);
     }