Bug 1362063 - replace delayed ValidateARIAOwned on straightforward DOM tree traversal, r=davidb
authorAlexander Surkov <surkov.alexander@gmail.com>
Thu, 04 May 2017 13:21:17 -0400
changeset 572843 6669f26d7f1366221b9edd3d82ac699e98101dc7
parent 572842 9630a51eca0776e055e04797614d15f02aaf65f5
child 572844 a7894c708b6fb1f02dbf71baedfdedd4e7abd7ec
push id57195
push userbmo:rbarker@mozilla.com
push dateThu, 04 May 2017 20:08:56 +0000
reviewersdavidb
bugs1362063
milestone55.0a1
Bug 1362063 - replace delayed ValidateARIAOwned on straightforward DOM tree traversal, r=davidb
accessible/base/NotificationController.cpp
accessible/generic/DocAccessible.cpp
accessible/generic/DocAccessible.h
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -790,20 +790,16 @@ NotificationController::WillRefresh(mozi
     if (!mDocument)
       return;
   }
 
   // Process invalidation list of the document after all accessible tree
   // modification are done.
   mDocument->ProcessInvalidationList();
 
-  // We cannot rely on DOM tree to keep aria-owns relations updated. Make
-  // a validation to remove dead links.
-  mDocument->ValidateARIAOwned();
-
   // Process relocation list.
   for (uint32_t idx = 0; idx < mRelocations.Length(); idx++) {
     if (mRelocations[idx]->IsInDocument()) {
       mDocument->DoARIAOwnsRelocation(mRelocations[idx]);
     }
   }
   mRelocations.Clear();
 
--- a/accessible/generic/DocAccessible.cpp
+++ b/accessible/generic/DocAccessible.cpp
@@ -1967,45 +1967,56 @@ DocAccessible::FireEventsOnInsertion(Acc
         break;
       }
     }
     while ((ancestor = ancestor->Parent()));
   }
 }
 
 void
-DocAccessible::ContentRemoved(Accessible* aContent)
+DocAccessible::ContentRemoved(Accessible* aChild)
 {
-  MOZ_DIAGNOSTIC_ASSERT(aContent->Parent(), "Unattached accessible from tree");
+  Accessible* parent = aChild->Parent();
+  MOZ_DIAGNOSTIC_ASSERT(parent, "Unattached accessible from tree");
 
 #ifdef A11Y_LOG
   logging::TreeInfo("process content removal", 0,
-                    "container", aContent->Parent(), "child", aContent, nullptr);
+                    "container", parent, "child", aChild, nullptr);
 #endif
 
-  TreeMutation mt(aContent->Parent());
-  mt.BeforeRemoval(aContent);
-  aContent->Parent()->RemoveChild(aContent);
-  UncacheChildrenInSubtree(aContent);
+  TreeMutation mt(parent);
+  mt.BeforeRemoval(aChild);
+
+  if (aChild->IsRelocated()) {
+    nsTArray<RefPtr<Accessible> >* owned = mARIAOwnsHash.Get(parent);
+    MOZ_ASSERT(owned, "IsRelocated flag is out of sync with mARIAOwnsHash");
+    owned->RemoveElement(aChild);
+    if (owned->Length() == 0) {
+      mARIAOwnsHash.Remove(parent);
+    }
+  }
+  parent->RemoveChild(aChild);
+  UncacheChildrenInSubtree(aChild);
+
   mt.Done();
 }
 
 void
 DocAccessible::ContentRemoved(nsIContent* aContentNode)
 {
   // If child node is not accessible then look for its accessible children.
   Accessible* acc = GetAccessible(aContentNode);
   if (acc) {
     ContentRemoved(acc);
   }
-  else {
-    TreeWalker walker(this, aContentNode);
-    while (Accessible* acc = walker.Next()) {
-      ContentRemoved(acc);
-    }
+
+  dom::AllChildrenIterator iter =
+    dom::AllChildrenIterator(aContentNode, nsIContent::eAllChildren, true);
+  while (nsIContent* childNode = iter.GetNextChild()) {
+    ContentRemoved(childNode);
   }
 }
 
 bool
 DocAccessible::RelocateARIAOwnedIfNeeded(nsIContent* aElement)
 {
   if (!aElement->HasID())
     return false;
@@ -2023,60 +2034,16 @@ DocAccessible::RelocateARIAOwnedIfNeeded
       }
     }
   }
 
   return false;
 }
 
 void
-DocAccessible::ValidateARIAOwned()
-{
-  for (auto it = mARIAOwnsHash.Iter(); !it.Done(); it.Next()) {
-    Accessible* owner = it.Key();
-    nsTArray<RefPtr<Accessible> >* children = it.UserData();
-
-    // Owner is about to die, put children back if applicable.
-    if (owner != this &&
-        (!mAccessibleCache.GetWeak(reinterpret_cast<void*>(owner)) ||
-         !owner->IsInDocument())) {
-      PutChildrenBack(children, 0);
-      it.Remove();
-      continue;
-    }
-
-    for (uint32_t idx = 0; idx < children->Length(); idx++) {
-      Accessible* child = children->ElementAt(idx);
-      if (!child->IsInDocument()) {
-        children->RemoveElementAt(idx);
-        idx--;
-        continue;
-      }
-
-      NS_ASSERTION(child->Parent(), "No parent for ARIA owned?");
-
-      // If DOM node doesn't have a frame anymore then shutdown its accessible.
-      if (child->Parent() && !child->GetFrame()) {
-        ContentRemoved(child);
-        children->RemoveElementAt(idx);
-        idx--;
-        continue;
-      }
-
-      NS_ASSERTION(child->Parent() == owner,
-                   "Illigally stolen ARIA owned child!");
-    }
-
-    if (children->Length() == 0) {
-      it.Remove();
-    }
-  }
-}
-
-void
 DocAccessible::DoARIAOwnsRelocation(Accessible* aOwner)
 {
   nsTArray<RefPtr<Accessible> >* children = mARIAOwnsHash.LookupOrAdd(aOwner);
 
   MOZ_ASSERT(aOwner, "aOwner must be a valid pointer");
   MOZ_ASSERT(aOwner->Elm(), "aOwner->Elm() must be a valid pointer");
 
 #ifdef A11Y_LOG
@@ -2189,17 +2156,17 @@ DocAccessible::PutChildrenBack(nsTArray<
     int32_t idxInParent = -1;
     Accessible* origContainer = GetContainerAccessible(child->GetContent());
     if (origContainer) {
       TreeWalker walker(origContainer);
       if (walker.Seek(child->GetContent())) {
         Accessible* prevChild = walker.Prev();
         if (prevChild) {
           idxInParent = prevChild->IndexInParent() + 1;
-          MOZ_ASSERT(origContainer == prevChild->Parent(), "Broken tree");
+          MOZ_DIAGNOSTIC_ASSERT(origContainer == prevChild->Parent(), "Broken tree");
           origContainer = prevChild->Parent();
         }
         else {
           idxInParent = 0;
         }
       }
     }
     MoveChild(child, origContainer, idxInParent);
@@ -2220,18 +2187,22 @@ DocAccessible::MoveChild(Accessible* aCh
 #ifdef A11Y_LOG
   logging::TreeInfo("move child", 0,
                     "old parent", curParent, "new parent", aNewParent,
                     "child", aChild, nullptr);
 #endif
 
   // If the child was taken from from an ARIA owns element.
   if (aChild->IsRelocated()) {
-    nsTArray<RefPtr<Accessible> >* children = mARIAOwnsHash.Get(curParent);
-    children->RemoveElement(aChild);
+    nsTArray<RefPtr<Accessible> >* owned = mARIAOwnsHash.Get(curParent);
+    MOZ_ASSERT(owned, "IsRelocated flag is out of sync with mARIAOwnsHash");
+    owned->RemoveElement(aChild);
+    if (owned->Length() == 0) {
+      mARIAOwnsHash.Remove(curParent);
+    }
   }
 
   NotificationController::MoveGuard mguard(mNotificationController);
 
   if (curParent == aNewParent) {
     MOZ_ASSERT(aChild->IndexInParent() != aIdxInParent, "No move case");
     curParent->MoveChild(aIdxInParent, aChild);
 
@@ -2322,20 +2293,29 @@ DocAccessible::CacheChildrenInSubtree(Ac
 }
 
 void
 DocAccessible::UncacheChildrenInSubtree(Accessible* aRoot)
 {
   aRoot->mStateFlags |= eIsNotInDocument;
   RemoveDependentIDsFor(aRoot);
 
+  nsTArray<RefPtr<Accessible> >* owned = mARIAOwnsHash.Get(aRoot);
   uint32_t count = aRoot->ContentChildCount();
   for (uint32_t idx = 0; idx < count; idx++) {
     Accessible* child = aRoot->ContentChildAt(idx);
 
+    if (child->IsRelocated()) {
+      MOZ_ASSERT(owned, "IsRelocated flag is out of sync with mARIAOwnsHash");
+      owned->RemoveElement(child);
+      if (owned->Length() == 0) {
+        mARIAOwnsHash.Remove(aRoot);
+      }
+    }
+
     // Removing this accessible from the document doesn't mean anything about
     // accessibles for subdocuments, so skip removing those from the tree.
     if (!child->IsDoc()) {
       UncacheChildrenInSubtree(child);
     }
   }
 
   if (aRoot->IsNodeMapEntry() &&
--- a/accessible/generic/DocAccessible.h
+++ b/accessible/generic/DocAccessible.h
@@ -339,17 +339,17 @@ public:
    */
   void ContentInserted(nsIContent* aContainerNode,
                        nsIContent* aStartChildNode,
                        nsIContent* aEndChildNode);
 
   /**
    * Update the tree on content removal.
    */
-  void ContentRemoved(Accessible* aContent);
+  void ContentRemoved(Accessible* aAccessible);
   void ContentRemoved(nsIContent* aContentNode);
 
   /**
    * Updates accessible tree when rendered text is changed.
    */
   void UpdateText(nsIContent* aTextNode);
 
   /**
@@ -500,21 +500,16 @@ protected:
    *
    * While children are cached we may encounter the case there's no accessible
    * for referred content by related accessible. Store these related nodes to
    * invalidate their containers later.
    */
   void ProcessInvalidationList();
 
   /**
-   * Validates all aria-owns connections and updates the tree accordingly.
-   */
-  void ValidateARIAOwned();
-
-  /**
    * Steals or puts back accessible subtrees.
    */
   void DoARIAOwnsRelocation(Accessible* aOwner);
 
   /**
    * Moves children back under their original parents.
    */
   void PutChildrenBack(nsTArray<RefPtr<Accessible> >* aChildren,