Bug 1353094 - simplify the logic of accessible subtrees removal, r=davidb

accessible/base/NotificationController.cpp

@@ -676,7 +676,7 @@ NotificationController::WillRefresh(mozilla::TimeStamp aTime)
         }
   #endif
 
-        mDocument->ContentRemoved(containerElm, textNode);
+        mDocument->ContentRemoved(textAcc);
         continue;
       }
 

accessible/base/TreeWalker.cpp

@@ -52,6 +52,18 @@ TreeWalker::
   MOZ_COUNT_CTOR(TreeWalker);
 }
 
+TreeWalker::
+  TreeWalker(DocAccessible* aDocument, nsIContent* aAnchorNode) :
+  mDoc(aDocument), mContext(nullptr), mAnchorNode(aAnchorNode),
+  mARIAOwnsIdx(0),
+  mChildFilter(nsIContent::eSkipPlaceholderContent | nsIContent::eAllChildren),
+  mFlags(eWalkCache),
+  mPhase(eAtStart)
+{
+  MOZ_ASSERT(aAnchorNode, "No anchor node for the accessible tree walker");
+  MOZ_COUNT_CTOR(TreeWalker);
+}
+
 TreeWalker::~TreeWalker()
 {
   MOZ_COUNT_DTOR(TreeWalker);

accessible/base/TreeWalker.h

@@ -47,6 +47,11 @@ public:
    */
   TreeWalker(Accessible* aContext, nsIContent* aAnchorNode, uint32_t aFlags = eWalkCache);
 
+  /**
+   * Navigates the accessible children within the anchor node subtree.
+   */
+  TreeWalker(DocAccessible* aDocument, nsIContent* aAnchorNode);
+
   ~TreeWalker();
 
   /**

accessible/base/nsAccessibilityService.cpp

@@ -524,7 +524,7 @@ nsAccessibilityService::DeckPanelSwitched(nsIPresShell* aPresShell,
     }
 #endif
 
-    document->ContentRemoved(aDeckNode, panelNode);
+    document->ContentRemoved(panelNode);
   }
 
   if (aCurrentBoxFrame) {
@@ -582,26 +582,7 @@ nsAccessibilityService::ContentRemoved(nsIPresShell* aPresShell,
 #endif
 
   if (document) {
-    // Flatten hierarchy may be broken at this point so we cannot get a true
-    // container by traversing up the DOM tree. Find a parent of first accessible
-    // from the subtree of the given DOM node, that'll be a container. If no
-    // accessibles in subtree then we don't care about the change.
-    Accessible* child = document->GetAccessible(aChildNode);
-    if (!child) {
-      Accessible* container = document->GetContainerAccessible(aChildNode);
-      a11y::TreeWalker walker(container ? container : document, aChildNode,
-                              a11y::TreeWalker::eWalkCache);
-      child = walker.Next();
-    }
-
-    if (child) {
-      MOZ_DIAGNOSTIC_ASSERT(child->Parent(), "Unattached accessible from tree");
-      document->ContentRemoved(child->Parent(), aChildNode);
-#ifdef A11Y_LOG
-      if (logging::IsEnabled(logging::eTree))
-        logging::AccessibleNNode("real container", child->Parent());
-#endif
-    }
+    document->ContentRemoved(aChildNode);
   }
 
 #ifdef A11Y_LOG

accessible/generic/DocAccessible.cpp

@@ -1170,10 +1170,7 @@ DocAccessible::ContentRemoved(nsIDocument* aDocument,
   // This one and content removal notification from layout may result in
   // double processing of same subtrees. If it pops up in profiling, then
   // consider reusing a document node cache to reject these notifications early.
-  Accessible* container = GetAccessibleOrContainer(aContainerNode);
-  if (container) {
-    UpdateTreeOnRemoval(container, aChildNode);
-  }
+  ContentRemoved(aChildNode);
 }
 
 void
@@ -1382,7 +1379,7 @@ DocAccessible::RecreateAccessible(nsIContent* aContent)
   // should be coalesced with normal show/hide events.
 
   nsIContent* parent = aContent->GetFlattenedTreeParent();
-  ContentRemoved(parent, aContent);
+  ContentRemoved(aContent);
   ContentInserted(parent, aContent, aContent->GetNextSibling());
 }
 
@@ -1972,35 +1969,38 @@ DocAccessible::FireEventsOnInsertion(Accessible* aContainer)
 }
 
 void
-DocAccessible::UpdateTreeOnRemoval(Accessible* aContainer, nsIContent* aChildNode)
+DocAccessible::ContentRemoved(Accessible* aContent)
 {
-  // If child node is not accessible then look for its accessible children.
-  Accessible* child = GetAccessible(aChildNode);
+  MOZ_DIAGNOSTIC_ASSERT(aContent->Parent(), "Unattached accessible from tree");
+
 #ifdef A11Y_LOG
   logging::TreeInfo("process content removal", 0,
-                    "container", aContainer, "child", aChildNode);
+                    "container", aContent->Parent(), "child", aContent, nullptr);
 #endif
 
-  TreeMutation mt(aContainer);
-  if (child) {
-    mt.BeforeRemoval(child);
-    MOZ_ASSERT(aContainer == child->Parent(), "Wrong parent");
-    aContainer->RemoveChild(child);
-    UncacheChildrenInSubtree(child);
-    mt.Done();
-    return;
-  }
-
-  TreeWalker walker(aContainer, aChildNode, TreeWalker::eWalkCache);
-  while (Accessible* child = walker.Next()) {
-    mt.BeforeRemoval(child);
-    MOZ_ASSERT(aContainer == child->Parent(), "Wrong parent");
-    aContainer->RemoveChild(child);
-    UncacheChildrenInSubtree(child);
-  }
+  TreeMutation mt(aContent->Parent());
+  mt.BeforeRemoval(aContent);
+  aContent->Parent()->RemoveChild(aContent);
+  UncacheChildrenInSubtree(aContent);
   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);
+    }
+  }
+}
+
 bool
 DocAccessible::RelocateARIAOwnedIfNeeded(nsIContent* aElement)
 {
@@ -2051,7 +2051,7 @@ DocAccessible::ValidateARIAOwned()
 
       // If DOM node doesn't have a frame anymore then shutdown its accessible.
       if (child->Parent() && !child->GetFrame()) {
-        UpdateTreeOnRemoval(child->Parent(), child->GetContent());
+        ContentRemoved(child);
         children->RemoveElementAt(idx);
         idx--;
         continue;

accessible/generic/DocAccessible.h

@@ -342,18 +342,10 @@ public:
                        nsIContent* aEndChildNode);
 
   /**
-   * Notify the document accessible that content was removed.
+   * Update the tree on content removal.
    */
-  void ContentRemoved(Accessible* aContainer, nsIContent* aChildNode)
-  {
-    // Update the whole tree of this document accessible when the container is
-    // null (document element is removed).
-    UpdateTreeOnRemoval((aContainer ? aContainer : this), aChildNode);
-  }
-  void ContentRemoved(nsIContent* aContainerNode, nsIContent* aChildNode)
-  {
-    ContentRemoved(AccessibleOrTrueContainer(aContainerNode), aChildNode);
-  }
+  void ContentRemoved(Accessible* aContent);
+  void ContentRemoved(nsIContent* aContentNode);
 
   /**
    * Updates accessible tree when rendered text is changed.
@@ -512,11 +504,6 @@ protected:
    */
   void ProcessInvalidationList();
 
-  /**
-   * Update the accessible tree for content removal.
-   */
-  void UpdateTreeOnRemoval(Accessible* aContainer, nsIContent* aChildNode);
-
   /**
    * Validates all aria-owns connections and updates the tree accordingly.
    */