commit 523cc106f153796ab624f1a1c86d979d8f7ffb77
author Evelynn Kaplan <evelynn.kaplan@microsoft.com> Fri Aug 09 18:46:39 2024
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Aug 09 18:46:39 2024
tree b233782ffcdd1d3fca5943c00a8458cd379d4c68
parent eb807d6ce4940f3f6d12fd6aed491dd3fb6d5bd8

[a11y] Check if the queue is flushing before firing a new event

This CL adds a new variable, `g_is_flushing`, to track when an
`AXPlatformNodeDelegate`'s event queue is being flushed. The value of
`g_is_flushing` is checked in
`ViewAXPlatformNodeDelegate::FireNativeEvent()` and will cause that
function to return early before firing or queueing a new event.

Firing a new event while flushing the event queue is not a valid
behavior. However, it's currently possible for this to occur if an
event that is fired once it is flushed from the queue has a call stack
that will cause another new event to get fired. An example of this can
be seen in SavedTabGroupButton::GetAccessibleNodeData(): https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/ui/views/bookmarks/saved_tab_groups/saved_tab_group_button.cc;l=177;drc=ae99ae281ffc2fe19024f86c5c64939efc1b2e87.

This has been noticed in Linux after some adjustments made to the
`GetAccessibleNodeData()` function for a handful of Views. These
adjustments put calls to functions such as
`GetViewAccessibility().SetName()` into the `GetAccessibleNodeData()`
function. `GetAccessibleNodeData()` is called when an assistive
technology is requesting accessibility information about a View, and
is not where a View's accessibility properties change (change in
a property would cause a new event to fire).

However, when `AXPlatformNodeAuraLinux::NotifyAccessibilityEvent()` is
called, perhaps as part of flushing the queue, it calls
`GetOrCreateAtkObject()`, which then calls `GetRole()`, which then
calls the View's `GetAccessibleNodeData()`. If a function such as
`SetName()` is then called for the first time or if the value has
actually changed, it is interpreted as a change in name, not
a new name. And such, a call to fire a new event is made, and that is
how we can end up with new events firing during queue flushing.

Using `g_is_flushing` to block new events from firing will not stop
`FireNativeEvent()` from emplacing new events in the queue if
`g_is_queueing_events` is true, because while `g_is_queueing_events` is
set before posting the task to flush, the flushing itself is done at a
later point, asynchronously. `g_is_flushing` can be `false` while
`g_is_queueing_events` is `true`.

Bug: 358404368
Change-Id: I2e67c51c402fdee60204fab232edd9fcc9a8b257
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5770923
Reviewed-by: Jacques Newman <janewman@microsoft.com>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Commit-Queue: Evelynn Kaplan <evelynn.kaplan@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1339770}