From fe7e46933cf1e647849e6b5269b63c155be19116 Mon Sep 17 00:00:00 2001 From: Aaron Leventhal <aleventhal@google.com> Date: Tue, 5 Nov 2024 16:59:52 +0000 Subject: [PATCH] M131: [A11y] Handle when document is marked dirty late in the process This condition actually occurs only on Mac, because it is the only platform that needs to use MarkDocumentDirty() when aria-modal dialogs are created or destroyed. There is no real additional cost to keep the code for all platforms. Will do test as a follow-up. Repro is to load pepboys.com and continually click on "Your store" on the right side of the toolbar. (cherry picked from commit f2eb83b50beff612ec481f600cb0d9cfe05dedfd) Fixed: 372508699,375669433 Change-Id: I524479e035d27e6118a53fdccef77fc9ad5d7dcd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5979112 Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com> Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1376030} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5981914 Cr-Commit-Position: refs/branch-heads/6778@{#1719} Cr-Branched-From: b21671ca172dcfd1566d41a770b2808e7fa7cd88-refs/heads/main@{#1368529} --- .../accessibility/ax_object_cache_impl.cc | 22 ++++++++----------- .../accessibility/ax_object_cache_impl.h | 5 ----- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc index 4a48797cb6468..325a0e4952666 100644 --- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc +++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc @@ -3123,10 +3123,6 @@ void AXObjectCacheImpl::CommitAXUpdates(Document& document, bool force) { // Upon exiting this function, listen for tree updates again. absl::Cleanup lifecycle_returns_to_queueing_updates = [this] { lifecycle_.EnsureStateAtMost(AXObjectCacheLifecycle::kDeferTreeUpdates); -#if DCHECK_IS_ON() - // TODO(https://crbug.com/372508699): Remove after bug fixed. - can_mark_all_dirty_ = true; -#endif }; SCOPED_DISALLOW_LIFECYCLE_TRANSITION(); @@ -3192,11 +3188,6 @@ void AXObjectCacheImpl::CommitAXUpdates(Document& document, bool force) { #endif mark_all_dirty_ = false; -#if DCHECK_IS_ON() - // TODO(https://crbug.com/372508699): Remove after bug fixed. - // Do not mark document dirty after the point that we expect to. - can_mark_all_dirty_ = false; -#endif // All tree updates have been processed. DUMP_WILL_BE_CHECK(!IsMainDocumentDirty()); @@ -3211,6 +3202,15 @@ void AXObjectCacheImpl::CommitAXUpdates(Document& document, bool force) { EnsureFocusedObject(); + if (mark_all_dirty_) { + // In some cases, EnsureFocusedObject() causes bad aria-hidden subtrees + // to be removed, if they contained the focus. This can in turn lead to + // marking the entire document dirty if a modal dialog or focus within + // the modal dialog is removed. + MarkDocumentDirtyWithCleanLayout(); + mark_all_dirty_ = false; + } + CHECK(tree_update_callback_queue_main_.empty()); CHECK(tree_update_callback_queue_popup_.empty()); CHECK(nodes_with_pending_children_changed_.empty()); @@ -4988,10 +4988,6 @@ void AXObjectCacheImpl::MarkSubtreeDirty(Node* node) { void AXObjectCacheImpl::MarkDocumentDirty() { CHECK(!IsFrozen()); - // TODO(https://crbug.com/372508699): Remove after bug fixed. -#if DCHECK_IS_ON() - CHECK(can_mark_all_dirty_); -#endif mark_all_dirty_ = true; diff --git a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h index 08acfd34a19df..cf98b3d19623c 100644 --- a/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h +++ b/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h @@ -668,11 +668,6 @@ class MODULES_EXPORT AXObjectCacheImpl : public AXObjectCacheBase { } HeapHashMap<AXID, Member<AXObject>>& GetObjects() { return objects_; } -#if DCHECK_IS_ON() - // TODO(https://crbug.com/372508699): Remove after bug fixed. - bool can_mark_all_dirty_ = true; -#endif - // Used to turn on accessibility checks for internal Web UI, e.g. history, // preferences, etc. Will trigger DCHECKS so that WebUI with basic a11y errors // fail tests. -- GitLab