Commit f9b60b64 authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

Android a11y: Avoid firing too many WINDOW_CONTENT_CHANGED events.

This is a quick fix for a performance issue that we've seen in several cases
but is particularly bad for one Android app on Android TV.

Whenever a node in the accessibility tree changes, we fire a
TYPE_WINDOW_CONTENT_CHANGED accessibility event, which is needed for
Android to clear its accessibility cache. However, when a major change
to a web page happens and a lot of events are fired on different nodes,
the overhead of firing an event on every single node is much too high
and just ends up slowing the device to a crawl or causing an ANR. The
alternative is to just fire TYPE_WINDOW_CONTENT_CHANGED on the root,
invalidating the whole WebView. This is inefficient compared to
invalidating just one node, but much more efficient than individually
updating hundreds of nodes.

What this patch does is add some really simple and quick logic to
count how many WINDOW_CONTENT_CHANGED events we're firing within a single
accessibility tree atomic update, and when we reach some maximum it
fires an event on the root and then suppresses further events of this
type until the next update.

It might be cleaner to move this logic to AccessibilityEventDispatcher
instead in the future, but for now this is the most straightforward
fix and it results in a big performance improvement in practice.

is changing

Bug: b/159131746
Change-Id: Ic04f5e87a205e80f460715de14f99de70a0cf90e
AX-Relnotes: improves performance on some Android pages when lots of content
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552897
Auto-Submit: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarMark Schillaci <mschillaci@google.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/master@{#829900}
parent 1c33106a
...@@ -15,6 +15,12 @@ ...@@ -15,6 +15,12 @@
namespace content { namespace content {
namespace {
// The maximum number of TYPE_WINDOW_CONTENT_CHANGED events to fire in one
// atomic update before we give up and fire it on the root node instead.
constexpr int kMaxContentChangedEventsToFire = 5;
} // namespace
// static // static
BrowserAccessibilityManager* BrowserAccessibilityManager::Create( BrowserAccessibilityManager* BrowserAccessibilityManager::Create(
const ui::AXTreeUpdate& initial_tree, const ui::AXTreeUpdate& initial_tree,
...@@ -195,9 +201,27 @@ void BrowserAccessibilityManagerAndroid::FireGeneratedEvent( ...@@ -195,9 +201,27 @@ void BrowserAccessibilityManagerAndroid::FireGeneratedEvent(
// Always send AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED to notify // Always send AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED to notify
// the Android system that the accessibility hierarchy rooted at this // the Android system that the accessibility hierarchy rooted at this
// node has changed. // node has changed. However, if there are a large number of changes
if (event_type != ui::AXEventGenerator::Event::SUBTREE_CREATED) // it's too expensive to fire all of them, so we just fire one
wcax->HandleContentChanged(android_node->unique_id()); // on the root instead.
if (event_type != ui::AXEventGenerator::Event::SUBTREE_CREATED) {
content_changed_events_++;
if (content_changed_events_ < kMaxContentChangedEventsToFire) {
// If it's less than the max event count, fire the event on the specific
// node that changed.
wcax->HandleContentChanged(android_node->unique_id());
} else if (content_changed_events_ == kMaxContentChangedEventsToFire) {
// If it's equal to the max event count, fire the event on the
// root instead.
BrowserAccessibilityManager* root_manager = GetRootManager();
if (root_manager) {
auto* root_node =
static_cast<BrowserAccessibilityAndroid*>(root_manager->GetRoot());
if (root_node)
wcax->HandleContentChanged(root_node->unique_id());
}
}
}
switch (event_type) { switch (event_type) {
case ui::AXEventGenerator::Event::ALERT: { case ui::AXEventGenerator::Event::ALERT: {
...@@ -499,6 +523,9 @@ void BrowserAccessibilityManagerAndroid::OnAtomicUpdateFinished( ...@@ -499,6 +523,9 @@ void BrowserAccessibilityManagerAndroid::OnAtomicUpdateFinished(
ui::AXTree* tree, ui::AXTree* tree,
bool root_changed, bool root_changed,
const std::vector<ui::AXTreeObserver::Change>& changes) { const std::vector<ui::AXTreeObserver::Change>& changes) {
// Reset this every time we get an atomic update.
content_changed_events_ = 0;
BrowserAccessibilityManager::OnAtomicUpdateFinished(tree, root_changed, BrowserAccessibilityManager::OnAtomicUpdateFinished(tree, root_changed,
changes); changes);
......
...@@ -134,6 +134,10 @@ class CONTENT_EXPORT BrowserAccessibilityManagerAndroid ...@@ -134,6 +134,10 @@ class CONTENT_EXPORT BrowserAccessibilityManagerAndroid
// See docs for set_prune_tree_for_screen_reader, above. // See docs for set_prune_tree_for_screen_reader, above.
bool prune_tree_for_screen_reader_; bool prune_tree_for_screen_reader_;
// A count of the number of TYPE_WINDOW_CONTENT_CHANGED events we've
// fired during a single atomic update.
int content_changed_events_ = 0;
DISALLOW_COPY_AND_ASSIGN(BrowserAccessibilityManagerAndroid); DISALLOW_COPY_AND_ASSIGN(BrowserAccessibilityManagerAndroid);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment