Fix bottleneck in computing win accessibility attributes

When a node is updated, on the browser side we need to
go through a three-phase process to update the hypertext
and other computed Windows attributes.

In some cases we also need to update the parent node's
attributes too - specifically when the original node is
text that contributes to its parent's hypertext, or
in an editable text region where things like spelling
markers need to propagate.

The previous algorithm was essentially that while updating
a node, it'd force its parent to update too. This led to
an inefficiency if you have a very flat document with
thousands of nodes that are all children of a single
parent. Every node that's updated triggers updating its
parent, causing the parent to be updated thousands of
times redundantly.

The fix is to first precompute a set of all nodes that need
to be updated, including following parents, and then
update them all with no more recursion.

This dramatically improves the load time of documents with
thousands of lines of just flat text and line breaks.

Bug: 921789

Change-Id: I5246ea321fda8a6ddce45221377010a4b5543801
Reviewed-on: https://chromium-review.googlesource.com/c/1455858
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629898}

content/browser/accessibility/browser_accessibility_com_win.cc

@@ -1799,16 +1799,6 @@ void BrowserAccessibilityComWin::UpdateStep3FireEvents(
         FireNativeEvent(IA2_EVENT_TEXT_INSERTED);
       }
     }
-
-    // Changing a static text node can affect the IA2 hypertext of its parent
-    // and, if the node is in a simple text control, the hypertext of the text
-    // control itself.
-    BrowserAccessibilityComWin* parent =
-        ToBrowserAccessibilityComWin(owner()->PlatformGetParent());
-    if (parent && (parent->owner()->HasState(ax::mojom::State::kEditable) ||
-                   owner()->IsTextOnlyObject())) {
-      parent->owner()->UpdatePlatformAttributes();
-    }
   }
 
   old_win_attributes_.reset(nullptr);

content/browser/accessibility/browser_accessibility_manager.cc

@@ -390,7 +390,7 @@ void BrowserAccessibilityManager::OnAccessibilityEvents(
     // Fire the native event.
     BrowserAccessibility* event_target = GetFromID(event.id);
     if (!event_target || !event_target->CanFireEvents())
-      return;
+      continue;
 
     if (event.event_type == ax::mojom::Event::kHover)
       GetRootManager()->CacheHitTestResult(event_target);

content/browser/accessibility/browser_accessibility_manager_win.cc

@@ -15,6 +15,7 @@
 #include "content/browser/accessibility/browser_accessibility_win.h"
 #include "content/browser/renderer_host/legacy_render_widget_host_win.h"
 #include "content/common/accessibility_messages.h"
+#include "ui/accessibility/ax_role_properties.h"
 #include "ui/base/win/atl_module.h"
 
 namespace content {
@@ -253,29 +254,63 @@ void BrowserAccessibilityManagerWin::OnAtomicUpdateFinished(
 
   // Do a sequence of Windows-specific updates on each node. Each one is
   // done in a single pass that must complete before the next step starts.
-  // The first step moves win_attributes_ to old_win_attributes_ and then
-  // recomputes all of win_attributes_ other than IAccessibleText.
+  // The nodes that need to be updated are all of the nodes that were changed,
+  // plus some parents.
+  std::map<BrowserAccessibilityComWin*, bool /* is_subtree_created */>
+      objs_to_update;
   for (const auto& change : changes) {
     const ui::AXNode* changed_node = change.node;
     DCHECK(changed_node);
+
+    bool is_subtree_created = change.type == AXTreeObserver::SUBTREE_CREATED;
     BrowserAccessibility* obj = GetFromAXNode(changed_node);
     if (obj && obj->IsNative()) {
-      ToBrowserAccessibilityWin(obj)
-          ->GetCOM()
-          ->UpdateStep1ComputeWinAttributes();
+      objs_to_update[ToBrowserAccessibilityWin(obj)->GetCOM()] =
+          is_subtree_created;
+    }
+
+    // When a node is a text node or line break, update its parent, because
+    // its text is part of its hypertext.
+    const ui::AXNode* parent = changed_node->parent();
+    if (!parent)
+      continue;
+    if (ui::IsTextOrLineBreak(changed_node->data().role)) {
+      BrowserAccessibility* parent_obj = GetFromAXNode(parent);
+      if (parent_obj && parent_obj->IsNative()) {
+        BrowserAccessibilityComWin* parent_com_obj =
+            ToBrowserAccessibilityWin(parent_obj)->GetCOM();
+        if (objs_to_update.find(parent_com_obj) == objs_to_update.end())
+          objs_to_update[parent_com_obj] = false;
+      }
+    }
+
+    // When a node is editable, update the editable root too.
+    if (!changed_node->data().HasState(ax::mojom::State::kEditable))
+      continue;
+    const ui::AXNode* editable_root = changed_node;
+    while (editable_root->parent() && editable_root->parent()->data().HasState(
+                                          ax::mojom::State::kEditable)) {
+      editable_root = editable_root->parent();
+    }
+    BrowserAccessibility* editable_root_obj = GetFromAXNode(editable_root);
+    if (editable_root_obj && editable_root_obj->IsNative()) {
+      BrowserAccessibilityComWin* editable_root_com_obj =
+          ToBrowserAccessibilityWin(editable_root_obj)->GetCOM();
+      if (objs_to_update.find(editable_root_com_obj) == objs_to_update.end())
+        objs_to_update[editable_root_com_obj] = false;
     }
   }
 
+  // The first step moves win_attributes_ to old_win_attributes_ and then
+  // recomputes all of win_attributes_ other than IAccessibleText.
+  for (auto& key_value : objs_to_update)
+    key_value.first->UpdateStep1ComputeWinAttributes();
+
   // The next step updates the hypertext of each node, which is a
   // concatenation of all of its child text nodes, so it can't run until
   // the text of all of the nodes was computed in the previous step.
-  for (const auto& change : changes) {
-    const ui::AXNode* changed_node = change.node;
-    DCHECK(changed_node);
-    BrowserAccessibility* obj = GetFromAXNode(changed_node);
-    if (obj && obj->IsNative())
-      ToBrowserAccessibilityWin(obj)->GetCOM()->UpdateStep2ComputeHypertext();
-  }
+  for (auto& key_value : objs_to_update)
+    key_value.first->UpdateStep2ComputeHypertext();
 
   // The third step fires events on nodes based on what's changed - like
   // if the name, value, or description changed, or if the hypertext had
@@ -285,14 +320,10 @@ void BrowserAccessibilityManagerWin::OnAtomicUpdateFinished(
   // client may walk the tree when it receives any of these events.
   // At the end, it deletes old_win_attributes_ since they're not needed
   // anymore.
-  for (const auto& change : changes) {
-    const ui::AXNode* changed_node = change.node;
-    DCHECK(changed_node);
-    BrowserAccessibility* obj = GetFromAXNode(changed_node);
-    if (obj && obj->IsNative()) {
-      ToBrowserAccessibilityWin(obj)->GetCOM()->UpdateStep3FireEvents(
-          change.type == AXTreeObserver::SUBTREE_CREATED);
-    }
+  for (auto& key_value : objs_to_update) {
+    BrowserAccessibilityComWin* obj = key_value.first;
+    bool is_subtree_created = key_value.second;
+    obj->UpdateStep3FireEvents(is_subtree_created);
   }
 }
 

ui/accessibility/ax_role_properties.cc

@@ -343,6 +343,16 @@ bool IsTableRow(ax::mojom::Role role) {
   }
 }
 
+bool IsTextOrLineBreak(ax::mojom::Role role) {
+  switch (role) {
+    case ax::mojom::Role::kLineBreak:
+    case ax::mojom::Role::kStaticText:
+      return true;
+    default:
+      return false;
+  }
+}
+
 bool SupportsExpandCollapse(const ax::mojom::Role role) {
   switch (role) {
     case ax::mojom::Role::kComboBoxGrouping:

ui/accessibility/ax_role_properties.h

@@ -86,6 +86,9 @@ AX_EXPORT bool IsTableLike(const ax::mojom::Role role);
 // table is not used for layout purposes.
 AX_EXPORT bool IsTableRow(ax::mojom::Role role);
 
+// Returns true if it's a text or line break node.
+AX_EXPORT bool IsTextOrLineBreak(ax::mojom::Role role);
+
 // Returns true if the provided role supports expand/collapse.
 AX_EXPORT bool SupportsExpandCollapse(const ax::mojom::Role role);