[Fuchsia] Add special handling of a11y root node

The root of the Fuchsia semantic tree always has root id 0. This isn't
a constraint in Chromium, so some special handling has been added to
ensure the semantic tree received by Fuchsia maintains a consistent,
valid state.

Test: Expanded AccessibilityBridgeTest.

Bug: fuchsia:42172
Change-Id: Ic1e2398e0a6793bcbfef36643af64b78f79fea22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954321
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726157}

fuchsia/engine/browser/accessibility_bridge.cc

@@ -22,25 +22,6 @@ constexpr uint32_t kSemanticNodeRootId = 0;
 // maximum sizes of serialized Semantic Nodes.
 constexpr size_t kMaxNodesPerUpdate = 16;
 
-// Template function to handle batching and sending of FIDL messages when
-// updating or deleting nodes.
-template <typename T>
-void SendBatches(std::vector<T> pending_items,
-                 base::RepeatingCallback<void(std::vector<T>)> callback) {
-  std::vector<T> nodes_to_send;
-  for (size_t i = 0; i < pending_items.size(); i++) {
-    nodes_to_send.push_back(std::move(pending_items.at(i)));
-    if (nodes_to_send.size() == kMaxNodesPerUpdate) {
-      callback.Run(std::move(nodes_to_send));
-      nodes_to_send.clear();
-    }
-  }
-
-  if (!nodes_to_send.empty()) {
-    callback.Run(std::move(nodes_to_send));
-  }
-}
-
 }  // namespace
 
 AccessibilityBridge::AccessibilityBridge(
@@ -63,35 +44,59 @@ AccessibilityBridge::AccessibilityBridge(
 AccessibilityBridge::~AccessibilityBridge() = default;
 
 void AccessibilityBridge::TryCommit() {
-  if (commit_inflight_ || (to_send_.empty() && to_delete_.empty()))
+  if (commit_inflight_ || to_send_.empty())
     return;
 
-  if (!to_send_.empty()) {
-    SendBatches<fuchsia::accessibility::semantics::Node>(
-        std::move(to_send_),
-        base::BindRepeating(
-            [](SemanticTree* tree,
-               std::vector<fuchsia::accessibility::semantics::Node> nodes) {
-              tree->UpdateSemanticNodes(std::move(nodes));
-            },
-            base::Unretained(tree_ptr_.get())));
+  SemanticUpdateOrDelete::Type current = to_send_.at(0).type;
+  int range_start = 0;
+  for (size_t i = 1; i < to_send_.size(); i++) {
+    if (to_send_.at(i).type == current &&
+        (i - range_start < kMaxNodesPerUpdate)) {
+      continue;
+    } else {
+      DispatchSemanticsMessages(range_start, i - range_start);
+      current = to_send_.at(i).type;
+      range_start = i;
     }
-
-  if (!to_delete_.empty()) {
-    SendBatches<uint32_t>(
-        std::move(to_delete_),
-        base::BindRepeating(
-            [](SemanticTree* tree, std::vector<uint32_t> nodes) {
-              tree->DeleteSemanticNodes(std::move(nodes));
-            },
-            base::Unretained(tree_ptr_.get())));
   }
+  DispatchSemanticsMessages(range_start, to_send_.size() - range_start);
 
   tree_ptr_->CommitUpdates(
       fit::bind_member(this, &AccessibilityBridge::OnCommitComplete));
   commit_inflight_ = true;
+  to_send_.clear();
 }
 
+void AccessibilityBridge::DispatchSemanticsMessages(size_t start, size_t size) {
+  if (to_send_.at(start).type == SemanticUpdateOrDelete::Type::UPDATE) {
+    std::vector<fuchsia::accessibility::semantics::Node> updates;
+    for (size_t i = start; i < start + size; i++) {
+      DCHECK(to_send_.at(i).type == SemanticUpdateOrDelete::Type::UPDATE);
+      updates.push_back(std::move(to_send_.at(i).update_node));
+    }
+    tree_ptr_->UpdateSemanticNodes(std::move(updates));
+  } else if (to_send_.at(start).type == SemanticUpdateOrDelete::Type::DELETE) {
+    std::vector<uint32_t> deletes;
+    for (size_t i = start; i < start + size; i++) {
+      DCHECK(to_send_.at(i).type == SemanticUpdateOrDelete::Type::DELETE);
+      deletes.push_back(to_send_.at(i).id_to_delete);
+    }
+    tree_ptr_->DeleteSemanticNodes(deletes);
+  }
+}
+
+AccessibilityBridge::SemanticUpdateOrDelete::SemanticUpdateOrDelete(
+    AccessibilityBridge::SemanticUpdateOrDelete&& m)
+    : type(m.type),
+      update_node(std::move(m.update_node)),
+      id_to_delete(m.id_to_delete) {}
+
+AccessibilityBridge::SemanticUpdateOrDelete::SemanticUpdateOrDelete(
+    Type type,
+    fuchsia::accessibility::semantics::Node node,
+    uint32_t id_to_delete)
+    : type(type), update_node(std::move(node)), id_to_delete(id_to_delete) {}
+
 void AccessibilityBridge::OnCommitComplete() {
   commit_inflight_ = false;
   TryCommit();
@@ -163,7 +168,6 @@ void AccessibilityBridge::OnSemanticsModeChanged(
     // The SemanticsManager will clear all state in this case, which is mirrored
     // here.
     to_send_.clear();
-    to_delete_.clear();
     commit_inflight_ = false;
   }
 
@@ -171,9 +175,31 @@ void AccessibilityBridge::OnSemanticsModeChanged(
   callback();
 }
 
+void AccessibilityBridge::DeleteSubtree(ui::AXTree* tree, ui::AXNode* node) {
+  DCHECK(tree);
+  DCHECK(node);
+
+  // When navigating, page 1, including the root, is deleted after page 2 has
+  // loaded. Since the root id is the same for page 1 and 2, page 2's root id
+  // ends up getting deleted. To handle this, the root will only be updated.
+  if (node->id() != root_id_) {
+    to_send_.push_back(
+        SemanticUpdateOrDelete(SemanticUpdateOrDelete::Type::DELETE, {},
+                               ConvertToFuchsiaNodeId(node->id())));
+  }
+  for (ui::AXNode* child : node->children())
+    DeleteSubtree(tree, child);
+}
+
 void AccessibilityBridge::OnNodeWillBeDeleted(ui::AXTree* tree,
                                               ui::AXNode* node) {
-  to_delete_.push_back(ConvertToFuchsiaNodeId(node->id()));
+  DeleteSubtree(tree, node);
+  TryCommit();
+}
+
+void AccessibilityBridge::OnSubtreeWillBeDeleted(ui::AXTree* tree,
+                                                 ui::AXNode* node) {
+  DeleteSubtree(tree, node);
   TryCommit();
 }
 
@@ -183,16 +209,23 @@ void AccessibilityBridge::OnAtomicUpdateFinished(
     const std::vector<ui::AXTreeObserver::Change>& changes) {
   root_id_ = tree_.root()->id();
   for (const ui::AXTreeObserver::Change& change : changes) {
-    // Reparent changes aren't included here because they consist of a delete
-    // and create change, which are already being handled.
-    if (change.type == ui::AXTreeObserver::NODE_CREATED ||
-        change.type == ui::AXTreeObserver::SUBTREE_CREATED ||
-        change.type == ui::AXTreeObserver::NODE_CHANGED) {
-      ui::AXNodeData ax_data = change.node->data();
+    ui::AXNodeData ax_data;
+    switch (change.type) {
+      case ui::AXTreeObserver::NODE_CREATED:
+      case ui::AXTreeObserver::SUBTREE_CREATED:
+      case ui::AXTreeObserver::NODE_CHANGED:
+        ax_data = change.node->data();
         if (change.node->id() == root_id_) {
           ax_data.id = kSemanticNodeRootId;
         }
-      to_send_.push_back(AXNodeDataToSemanticNode(ax_data));
+        to_send_.push_back(
+            SemanticUpdateOrDelete(SemanticUpdateOrDelete::Type::UPDATE,
+                                   AXNodeDataToSemanticNode(ax_data), 0));
+        break;
+      case ui::AXTreeObserver::NODE_REPARENTED:
+      case ui::AXTreeObserver::SUBTREE_REPARENTED:
+        DeleteSubtree(tree, change.node);
+        break;
     }
   }
   TryCommit();

fuchsia/engine/browser/accessibility_bridge.h

@@ -50,9 +50,30 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
  private:
   FRIEND_TEST_ALL_PREFIXES(AccessibilityBridgeTest, OnSemanticsModeChanged);
 
-  // Handles batching of semantic nodes and committing them to the SemanticTree.
+  // A struct used for caching semantic information. This allows for updates and
+  // deletes to be stored in the same vector to preserve all ordering
+  // information.
+  struct SemanticUpdateOrDelete {
+    enum Type { UPDATE, DELETE };
+
+    SemanticUpdateOrDelete(SemanticUpdateOrDelete&& m);
+    SemanticUpdateOrDelete(Type type,
+                           fuchsia::accessibility::semantics::Node node,
+                           uint32_t id_to_delete);
+    ~SemanticUpdateOrDelete() = default;
+
+    Type type;
+    fuchsia::accessibility::semantics::Node update_node;
+    uint32_t id_to_delete;
+  };
+
+  // Processes pending data and commits it to the Semantic Tree.
   void TryCommit();
 
+  // Helper function for TryCommit() that sends the contents of |to_send_| to
+  // the Semantic Tree, starting at |start|.
+  void DispatchSemanticsMessages(size_t start, size_t size);
+
   // Callback for SemanticTree::CommitUpdates.
   void OnCommitComplete();
 
@@ -60,6 +81,11 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   // root.
   uint32_t ConvertToFuchsiaNodeId(int32_t ax_node_id);
 
+  // Deletes all nodes in subtree rooted at and including |node|, unless |node|
+  // is the root of the tree.
+  // |tree| and |node| are owned by the accessibility bridge.
+  void DeleteSubtree(ui::AXTree* tree, ui::AXNode* node);
+
   // content::WebContentsObserver implementation.
   void AccessibilityEventReceived(
       const content::AXEventNotificationDetails& details) override;
@@ -76,6 +102,7 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
 
   // ui::AXTreeObserver implementation.
   void OnNodeWillBeDeleted(ui::AXTree* tree, ui::AXNode* node) override;
+  void OnSubtreeWillBeDeleted(ui::AXTree* tree, ui::AXNode* node) override;
   void OnAtomicUpdateFinished(
       ui::AXTree* tree,
       bool root_changed,
@@ -86,8 +113,8 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   content::WebContents* web_contents_;
   ui::AXSerializableTree tree_;
 
-  std::vector<fuchsia::accessibility::semantics::Node> to_send_;
-  std::vector<uint32_t> to_delete_;
+  // Cache for pending data to be sent to the Semantic Tree between commits.
+  std::vector<SemanticUpdateOrDelete> to_send_;
   bool commit_inflight_ = false;
 
   // Maintain a map of callbacks as multiple hit test events can happen at once.

fuchsia/engine/browser/accessibility_bridge_browsertest.cc

@@ -266,5 +266,13 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, TestNavigation) {
       controller.get(), fuchsia::web::LoadUrlParams(), title2.spec()));
 
   semantics_manager_.semantic_tree()->RunUntilNodeCountAtLeast(kPage2NodeCount);
+  EXPECT_TRUE(
+      semantics_manager_.semantic_tree()->HasNodeWithLabel(kPage2Title));
   EXPECT_TRUE(semantics_manager_.semantic_tree()->HasNodeWithLabel(kNodeName));
+
+  // Check that data from the first page has been deleted successfully.
+  EXPECT_FALSE(
+      semantics_manager_.semantic_tree()->HasNodeWithLabel(kButtonName));
+  EXPECT_FALSE(
+      semantics_manager_.semantic_tree()->HasNodeWithLabel(kParagraphName));
 }