From 408b51a92e5ccbaf4d6da2562651893186834e4c Mon Sep 17 00:00:00 2001
From: Aaron Leventhal <aleventhal@google.com>
Date: Mon, 5 Feb 2024 18:37:11 +0000
Subject: [PATCH] [A11y] Invalidate AXIDs when those objects are removed
After CL:4185636, an AXObject may have been detached and recreated
with the same DOM node, and therefore has the same id (the dom node id).
This breaks the accessibility object caching mechanism used for web
tests, which assumes that if it sees the same ID, it can assume the
object is still the same one. The fix is to notify the caching
mechanism of detached AXObjects so that it can invalidate the
corresponding entries.
Bug: none
Change-Id: I152538644f9dc24d4e7fbfe68057deb4ab4d34aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5262560
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1256301}
---
.../web_test/renderer/accessibility_controller.cc | 6 ++++++
content/web_test/renderer/accessibility_controller.h | 2 ++
content/web_test/renderer/web_ax_object_proxy.h | 1 +
content/web_test/renderer/web_frame_test_proxy.cc | 4 ++++
content/web_test/renderer/web_frame_test_proxy.h | 1 +
.../blink/public/web/web_local_frame_client.h | 5 +++++
.../modules/accessibility/ax_object_cache_impl.cc | 12 ++++++++++--
7 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/content/web_test/renderer/accessibility_controller.cc b/content/web_test/renderer/accessibility_controller.cc
index d7b011dd07fd9..e73fc949caf47 100644
--- a/content/web_test/renderer/accessibility_controller.cc
+++ b/content/web_test/renderer/accessibility_controller.cc
@@ -343,4 +343,10 @@ blink::WebAXObject AccessibilityController::GetAccessibilityObjectForMainFrame()
web_view()->MainFrame()->ToWebLocalFrame()->GetDocument());
}
+void AccessibilityController::Remove(unsigned axid) {
+ if (IsInstalled()) {
+ elements_->Remove(axid);
+ }
+}
+
} // namespace content
diff --git a/content/web_test/renderer/accessibility_controller.h b/content/web_test/renderer/accessibility_controller.h
index 8433ea7dbcc57..b5ce3ddb3668b 100644
--- a/content/web_test/renderer/accessibility_controller.h
+++ b/content/web_test/renderer/accessibility_controller.h
@@ -51,6 +51,8 @@ class AccessibilityController {
const std::string& notification_name,
const std::vector<ui::AXEventIntent>& event_intents);
+ void Remove(unsigned axid);
+
private:
friend class AccessibilityControllerBindings;
diff --git a/content/web_test/renderer/web_ax_object_proxy.h b/content/web_test/renderer/web_ax_object_proxy.h
index 0c9b1ed3c6270..97f5889a717fa 100644
--- a/content/web_test/renderer/web_ax_object_proxy.h
+++ b/content/web_test/renderer/web_ax_object_proxy.h
@@ -267,6 +267,7 @@ class WebAXObjectProxyList : public WebAXObjectProxy::Factory {
~WebAXObjectProxyList() override;
void Clear();
+ void Remove(unsigned axid) { ax_objects_.erase(axid); }
v8::Local<v8::Object> GetOrCreate(const blink::WebAXObject&) override;
blink::WebAXContext* GetAXContext() override;
diff --git a/content/web_test/renderer/web_frame_test_proxy.cc b/content/web_test/renderer/web_frame_test_proxy.cc
index 95f25fe629a4a..ac7dd5356af30 100644
--- a/content/web_test/renderer/web_frame_test_proxy.cc
+++ b/content/web_test/renderer/web_frame_test_proxy.cc
@@ -561,6 +561,10 @@ void WebFrameTestProxy::PostAccessibilityEvent(const ui::AXEvent& event) {
RenderFrameImpl::PostAccessibilityEvent(event);
}
+void WebFrameTestProxy::HandleAXObjectDetachedForTest(unsigned axid) {
+ accessibility_controller_.Remove(axid);
+}
+
void WebFrameTestProxy::HandleWebAccessibilityEventForTest(
const blink::WebAXObject& object,
const char* event_name,
diff --git a/content/web_test/renderer/web_frame_test_proxy.h b/content/web_test/renderer/web_frame_test_proxy.h
index fda17ec1f51d0..91506d3091f71 100644
--- a/content/web_test/renderer/web_frame_test_proxy.h
+++ b/content/web_test/renderer/web_frame_test_proxy.h
@@ -89,6 +89,7 @@ class WebFrameTestProxy : public RenderFrameImpl,
bool should_reset_browser_interface_broker,
const blink::ParsedPermissionsPolicy& permissions_policy_header,
const blink::DocumentPolicyFeatureState& document_policy_header) override;
+ void HandleAXObjectDetachedForTest(unsigned axid) override;
void HandleWebAccessibilityEventForTest(
const blink::WebAXObject& object,
const char* event_name,
diff --git a/third_party/blink/public/web/web_local_frame_client.h b/third_party/blink/public/web/web_local_frame_client.h
index 763db453aed44..91e7590e8c04c 100644
--- a/third_party/blink/public/web/web_local_frame_client.h
+++ b/third_party/blink/public/web/web_local_frame_client.h
@@ -818,6 +818,11 @@ class BLINK_EXPORT WebLocalFrameClient {
// a window.print() call.
virtual void ScriptedPrint() {}
+ // This method is ONLY for web tests and is not supposed to be overridden in
+ // classes other than web_frame_test_proxy. It's called from accessibility and
+ // is used as a way to notify that an accessibility object has been destroyed.
+ virtual void HandleAXObjectDetachedForTest(unsigned axid) {}
+
// This method is ONLY for web tests and is not supposed to be overridden in
// classes other than web_frame_test_proxy. It's called from accessibility and
// is used as a way to tunnel events to the accessibility_controller in web
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 09f936a123c36..fdcecfec8cd05 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
@@ -1775,8 +1775,16 @@ void AXObjectCacheImpl::Remove(AXID ax_id, bool notify_parent) {
}
#endif
- if (notify_parent && !has_been_disposed_) {
- ChildrenChangedOnAncestorOf(obj);
+ if (!has_been_disposed_) {
+ if (notify_parent) {
+ ChildrenChangedOnAncestorOf(obj);
+ }
+ // TODO(aleventhal) This is for web tests only, in order to record MarkDirty
+ // events. Is there a way to avoid these calls for normal browsing?
+ // Maybe we should use dependency injection from AccessibilityController.
+ if (auto* client = GetWebLocalFrameClient()) {
+ client->HandleAXObjectDetachedForTest(ax_id);
+ }
}
obj->Detach();