Optimize computed bounds in AXTreeSourceArc

This change takes the O(n^2) computation of a node's bounds down to O(n) by caching previously computed bounds.

It also solves a crash which seemed to result from corruption of the incoming node data. This was observed while using NY Times with webviews containing many nodes and firing many events.

Change-Id: I43c9295864fdcaf3775348eb17f55a4cf66da281
Reviewed-on: https://chromium-review.googlesource.com/956413
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542708}

chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc

@@ -339,11 +339,18 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(
     mojom::AccessibilityEventData* event_data) {
   tree_map_.clear();
   parent_map_.clear();
+  cached_computed_bounds_.clear();
   root_id_ = -1;
 
   window_id_ = event_data->window_id;
   is_notification_ = event_data->notification_key.has_value();
 
+  // The following loops perform caching to prepare for AXTreeSerializer.
+  // First, we need to cache parent links, which are implied by a node's child
+  // ids.
+  // Next, we cache the nodes by id. During this process, we can detect the root
+  // node based upon the parent links we cached above.
+  // Finally, we cache each node's computed bounds, based on its descendants.
   for (size_t i = 0; i < event_data->node_data.size(); ++i) {
     if (!event_data->node_data[i]->int_list_properties)
       continue;
@@ -357,18 +364,27 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(
 
   for (size_t i = 0; i < event_data->node_data.size(); ++i) {
     int32_t id = event_data->node_data[i]->id;
-    tree_map_[id] = event_data->node_data[i].get();
+    mojom::AccessibilityNodeInfoData* node = event_data->node_data[i].get();
+    tree_map_[id] = node;
     if (parent_map_.find(id) == parent_map_.end()) {
       CHECK_EQ(-1, root_id_) << "Duplicated root";
       root_id_ = id;
     }
 
-    if (GetBooleanProperty(event_data->node_data[i].get(),
+    if (GetBooleanProperty(node,
                            arc::mojom::AccessibilityBooleanProperty::FOCUSED)) {
       focused_node_id_ = id;
     }
   }
 
+  // Assuming |nodeData| is in pre-order, compute cached bounds in post-order to
+  // avoid an O(n^2) amount of work as the computed bounds uses descendant
+  // bounds.
+  for (int i = event_data->node_data.size() - 1; i >= 0; --i) {
+    mojom::AccessibilityNodeInfoData* node = event_data->node_data[i].get();
+    cached_computed_bounds_[node] = ComputeEnclosingBounds(node);
+  }
+
   ExtensionMsg_AccessibilityEventParams params;
   params.event_type = ToAXEvent(event_data->event_type);
 
@@ -696,6 +712,12 @@ gfx::Rect AXTreeSourceArc::ComputeEnclosingBounds(
 void AXTreeSourceArc::ComputeEnclosingBoundsInternal(
     mojom::AccessibilityNodeInfoData* node,
     gfx::Rect& computed_bounds) const {
+  auto cached_bounds = cached_computed_bounds_.find(node);
+  if (cached_bounds != cached_computed_bounds_.end()) {
+    computed_bounds.Union(cached_bounds->second);
+    return;
+  }
+
   // Only consider nodes that can possibly be accessibility focused. In Chrome,
   // this amounts to nodes with a non-generic container role.
   ui::AXNodeData data;
@@ -731,6 +753,7 @@ void AXTreeSourceArc::PerformAction(const ui::AXActionData& data) {
 void AXTreeSourceArc::Reset() {
   tree_map_.clear();
   parent_map_.clear();
+  cached_computed_bounds_.clear();
   current_tree_serializer_.reset(new AXTreeArcSerializer(this));
   root_id_ = -1;
   focused_node_id_ = -1;

chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h

@@ -119,6 +119,8 @@ class AXTreeSourceArc
   const Delegate* const delegate_;
   std::unique_ptr<FocusStealer> focus_stealer_;
   std::string package_name_;
+  std::map<mojom::AccessibilityNodeInfoData*, gfx::Rect>
+      cached_computed_bounds_;
 
   DISALLOW_COPY_AND_ASSIGN(AXTreeSourceArc);
 };