Commit 2489f004 authored by James Teh's avatar James Teh
Browse files

Bug 1834718: Don't process a duplicate DocAccessible::ContentRemoved call for... 

Bug 1834718: Don't process a duplicate DocAccessible::ContentRemoved call for a DOM node we've already processed. r=morgan, a=dmeehan

ContentRemoved recursively walks both AllChildrenIterator and direct DOM children.
In addition, we might get duplicate notifications from DOM and layout, plus PruneOrInsertSubtree might do a recursive walk and it too calls ContentRemoved.
To avoid this duplicate processing, keep a set of removed DOM nodes on the DocAccessible which we clear after mutation events are processed.

Differential Revision: https://phabricator.services.mozilla.com/D196707
parent ca59820e

accessible/base/NotificationController.cpp

+1 −1
Original line number Diff line number Diff line
@@ -946,7 +946,7 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) { 
  }



  if (mDocument) {

    mDocument->ClearMovedAccessibles();

    mDocument->ClearMutationData();

  }



  if (ipc::ProcessChild::ExpectingShutdown()) {

accessible/generic/DocAccessible.cpp

+4 −3
Original line number Diff line number Diff line
@@ -1035,9 +1035,6 @@ void DocAccessible::ContentRemoved(nsIContent* aChildNode, 
    logging::MsgEnd();

  }

#endif

  // 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.

  ContentRemoved(aChildNode);

}
@@ -2165,6 +2162,10 @@ void DocAccessible::ContentRemoved(LocalAccessible* aChild) { 
}



void DocAccessible::ContentRemoved(nsIContent* aContentNode) {

  if (!mRemovedNodes.EnsureInserted(aContentNode)) {

    return;

  }



  // If child node is not accessible then look for its accessible children.

  LocalAccessible* acc = GetAccessible(aContentNode);

  if (acc) {

accessible/generic/DocAccessible.h

+6 −3
Original line number Diff line number Diff line
@@ -550,12 +550,12 @@ class DocAccessible : public HyperTextAccessibleWrap, 


  /**

   * Called from NotificationController after all mutation events have been

   * processed to clear our data about Accessibles that were moved during this

   * tick.

   * processed to clear our data about mutations during this tick.

   */

  void ClearMovedAccessibles() {

  void ClearMutationData() {

    mMovedAccessibles.Clear();

    mInsertedAccessibles.Clear();

    mRemovedNodes.Clear();

  }



  /**
@@ -815,6 +815,9 @@ class DocAccessible : public HyperTextAccessibleWrap, 
  // processes. This is needed to prevent insertions + moves of the same

  // Accessible in the same tick from being tracked as moves.

  nsTHashSet<RefPtr<LocalAccessible>> mInsertedAccessibles;

  // A set of DOM nodes removed during this tick. This avoids a lot of pointless

  // recursive DOM traversals.

  nsTHashSet<nsIContent*> mRemovedNodes;

};



inline DocAccessible* LocalAccessible::AsDoc() {