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

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
This commit is contained in:
James Teh 2023-12-20 21:59:41 +00:00
parent 6014a16340
commit 06eb0ca294
3 changed files with 11 additions and 7 deletions

View File

@ -995,7 +995,7 @@ void NotificationController::WillRefresh(mozilla::TimeStamp aTime) {
} }
if (mDocument) { if (mDocument) {
mDocument->ClearMovedAccessibles(); mDocument->ClearMutationData();
} }
if (ipc::ProcessChild::ExpectingShutdown()) { if (ipc::ProcessChild::ExpectingShutdown()) {

View File

@ -1016,9 +1016,6 @@ void DocAccessible::ContentRemoved(nsIContent* aChildNode,
logging::MsgEnd(); logging::MsgEnd();
} }
#endif #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); ContentRemoved(aChildNode);
} }
@ -2176,6 +2173,10 @@ void DocAccessible::ContentRemoved(LocalAccessible* aChild) {
} }
void DocAccessible::ContentRemoved(nsIContent* aContentNode) { void DocAccessible::ContentRemoved(nsIContent* aContentNode) {
if (!mRemovedNodes.EnsureInserted(aContentNode)) {
return;
}
// If child node is not accessible then look for its accessible children. // If child node is not accessible then look for its accessible children.
LocalAccessible* acc = GetAccessible(aContentNode); LocalAccessible* acc = GetAccessible(aContentNode);
if (acc) { if (acc) {

View File

@ -538,12 +538,12 @@ class DocAccessible : public HyperTextAccessible,
/** /**
* Called from NotificationController after all mutation events have been * Called from NotificationController after all mutation events have been
* processed to clear our data about Accessibles that were moved during this * processed to clear our data about mutations during this tick.
* tick.
*/ */
void ClearMovedAccessibles() { void ClearMutationData() {
mMovedAccessibles.Clear(); mMovedAccessibles.Clear();
mInsertedAccessibles.Clear(); mInsertedAccessibles.Clear();
mRemovedNodes.Clear();
} }
/** /**
@ -808,6 +808,9 @@ class DocAccessible : public HyperTextAccessible,
// processes. This is needed to prevent insertions + moves of the same // processes. This is needed to prevent insertions + moves of the same
// Accessible in the same tick from being tracked as moves. // Accessible in the same tick from being tracked as moves.
nsTHashSet<RefPtr<LocalAccessible>> mInsertedAccessibles; 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() { inline DocAccessible* LocalAccessible::AsDoc() {