[fuchsia][a11y] Implements support for multiple iframes.

This change implements support in the AccessibilityBridge to deal with
multiple semantic trees, each one representing an iframe.

The trees are combined before they are sent to Fuchsia into a single
semantic tree.

Test: AccessibilityBridgeTest.*
Bug: fuchsia:63612,fuchsia:63610,fuchsia64329
Change-Id: I174ec9db254647edb33ada5b31eb24595debe031
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2556462Reviewed-by: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Lucas Radaelli <lucasradaelli@google.com>
Cr-Commit-Position: refs/heads/master@{#831997}

fuchsia/engine/browser/accessibility_bridge.cc

@@ -36,7 +36,6 @@ AccessibilityBridge::AccessibilityBridge(
       on_error_callback_(std::move(on_error_callback)) {
   DCHECK(web_contents_);
   Observe(web_contents_);
-  ax_tree_.AddObserver(this);
 
   semantics_manager->RegisterViewForSemantics(
       std::move(view_ref), binding_.NewBinding(), semantic_tree_.NewRequest());
@@ -48,10 +47,12 @@ AccessibilityBridge::AccessibilityBridge(
 
 AccessibilityBridge::~AccessibilityBridge() {
   InterruptPendingActions();
+  ax_trees_.clear();
 }
 
 void AccessibilityBridge::TryCommit() {
-  if (commit_inflight_ || (to_delete_.empty() && to_update_.empty()))
+  if (commit_inflight_ || (to_delete_.empty() && to_update_.empty()) ||
+      ShouldHoldCommit())
     return;
 
   // Deletions come before updates because first the nodes are deleted, and
@@ -91,14 +92,24 @@ void AccessibilityBridge::AccessibilityEventReceived(
   if (!enable_semantic_updates_)
     return;
 
-  // Updates to AXTree must be applied first.
-  for (const ui::AXTreeUpdate& update : details.updates) {
-    if (web_contents_->GetMainFrame()->GetAXTreeID() != details.ax_tree_id) {
-      // TODO(https://crbug.com/1128954): Add support for combining AXTrees.
-      continue;
+  const auto& id = details.ax_tree_id;
+  if (!UpdateAXTreeID(id))
+    return;
+
+  auto ax_tree_it = ax_trees_.find(id);
+  ui::AXSerializableTree* ax_tree;
+  if (ax_tree_it == ax_trees_.end()) {
+    auto new_tree = std::make_unique<ui::AXSerializableTree>();
+    ax_tree = new_tree.get();
+    ax_tree->AddObserver(this);
+    ax_trees_[id] = std::move(new_tree);
+  } else {
+    ax_tree = ax_tree_it->second.get();
   }
 
-    if (!ax_tree_.Unserialize(update)) {
+  // Updates to AXTree must be applied first.
+  for (const ui::AXTreeUpdate& update : details.updates) {
+    if (!ax_tree->Unserialize(update)) {
       // If this fails, it is a fatal error that will cause an early exit.
       std::move(on_error_callback_).Run(ZX_ERR_INTERNAL);
       return;
@@ -111,8 +122,8 @@ void AccessibilityBridge::AccessibilityEventReceived(
         pending_hit_test_callbacks_.find(event.action_request_id) !=
             pending_hit_test_callbacks_.end()) {
       fuchsia::accessibility::semantics::Hit hit;
-      hit.set_node_id(id_mapper_->ToFuchsiaNodeID(ax_tree_.data().tree_id,
-                                                  event.id, false));
+      hit.set_node_id(
+          id_mapper_->ToFuchsiaNodeID(ax_tree->GetAXTreeID(), event.id, false));
 
       // Run the pending callback with the hit.
       pending_hit_test_callbacks_[event.action_request_id](std::move(hit));
@@ -146,7 +157,11 @@ void AccessibilityBridge::OnAccessibilityActionRequested(
   action_data.target_node_id = ax_id->second;
 
   if (action == fuchsia::accessibility::semantics::Action::SHOW_ON_SCREEN) {
-    ui::AXNode* node = ax_tree_.GetFromId(action_data.target_node_id);
+    ui::AXNode* node = nullptr;
+    auto ax_tree_it = ax_trees_.find(ax_id->first);
+    if (ax_tree_it != ax_trees_.end())
+      node = ax_tree_it->second->GetFromId(action_data.target_node_id);
+
     if (!node) {
       callback(false);
       return;
@@ -161,7 +176,9 @@ void AccessibilityBridge::OnAccessibilityActionRequested(
     action_data.scroll_behavior = ax::mojom::ScrollBehavior::kScrollIfVisible;
   }
 
-  web_contents_->GetMainFrame()->AccessibilityPerformAction(action_data);
+  web_contents_->GetMainFrame()
+      ->FromAXTreeID(ax_id->first)
+      ->AccessibilityPerformAction(action_data);
   callback(true);
 
   if (event_received_callback_for_test_) {
@@ -210,7 +227,9 @@ void AccessibilityBridge::OnSemanticsModeChanged(
     to_delete_.clear();
     to_update_.clear();
     commit_inflight_ = false;
-    ax_tree_.Destroy();
+    ax_trees_.clear();
+    tree_connections_.clear();
+    frame_id_to_tree_id_.clear();
     InterruptPendingActions();
   }
 
@@ -218,18 +237,23 @@ void AccessibilityBridge::OnSemanticsModeChanged(
   callback();
 }
 
-void AccessibilityBridge::OnNodeWillBeDeleted(ui::AXTree* tree,
-                                              ui::AXNode* node) {
+void AccessibilityBridge::OnNodeDeleted(ui::AXTree* tree, int32_t node_id) {
   to_delete_.push_back(
-      id_mapper_->ToFuchsiaNodeID(ax_tree_.data().tree_id, node->id(), false));
+      id_mapper_->ToFuchsiaNodeID(tree->GetAXTreeID(), node_id, false));
 }
 
 void AccessibilityBridge::OnAtomicUpdateFinished(
     ui::AXTree* tree,
     bool root_changed,
     const std::vector<ui::AXTreeObserver::Change>& changes) {
-  DCHECK_EQ(tree, &ax_tree_);
-  root_id_ = ax_tree_.root()->id();
+  if (root_changed)
+    MaybeDisconnectTreeFromParentTree(tree);
+
+  const bool is_main_frame_tree =
+      tree->GetAXTreeID() == web_contents_->GetMainFrame()->GetAXTreeID();
+  if (is_main_frame_tree)
+    root_id_ = tree->root()->id();
+
   // Changes included here are only nodes that are still on the tree. Since this
   // indicates the end of an atomic update, it is safe to assume that these
   // nodes will not change until the next change arrives. Nodes that would be
@@ -237,9 +261,16 @@ void AccessibilityBridge::OnAtomicUpdateFinished(
   // |to_update_| are going to be executed after |to_delete_|.
   for (const ui::AXTreeObserver::Change& change : changes) {
     const auto& node = change.node->data();
-    to_update_.push_back(AXNodeDataToSemanticNode(
-        node, ax_tree_.data().tree_id, node.id == root_id_, id_mapper_.get()));
+    const bool is_root = is_main_frame_tree ? node.id == root_id_ : false;
+    to_update_.push_back(AXNodeDataToSemanticNode(node, tree->GetAXTreeID(),
+                                                  is_root, id_mapper_.get()));
+    if (node.HasStringAttribute(ax::mojom::StringAttribute::kChildTreeId)) {
+      const auto child_tree_id = ui::AXTreeID::FromString(
+          node.GetStringAttribute(ax::mojom::StringAttribute::kChildTreeId));
+      tree_connections_[child_tree_id] = {node.id, tree->GetAXTreeID(), false};
+    }
   }
+  UpdateTreeConnections();
   // TODO(https://crbug.com/1134737): Separate updates of atomic updates and
   // don't allow all of them to be in the same commit.
   TryCommit();
@@ -261,3 +292,161 @@ float AccessibilityBridge::GetDeviceScaleFactor() {
   }
   return web_contents_->GetRenderWidgetHostView()->GetDeviceScaleFactor();
 }
+
+const ui::AXSerializableTree* AccessibilityBridge::ax_tree_for_test() {
+  if (ax_trees_.empty())
+    return nullptr;
+
+  return ax_trees_.cbegin()->second.get();
+}
+
+void AccessibilityBridge::UpdateTreeConnections() {
+  std::vector<ui::AXTreeID> connections_to_remove;
+  for (auto& kv : tree_connections_) {
+    ui::AXSerializableTree* child_tree = nullptr;
+    auto it = ax_trees_.find(kv.first);
+    if (it != ax_trees_.end())
+      child_tree = it->second.get();
+
+    ui::AXSerializableTree* parent_tree = nullptr;
+    ui::AXNode* ax_node = nullptr;
+
+    const auto& parent_ax_tree_id = kv.second.parent_tree_id;
+    auto parent_it = ax_trees_.find(parent_ax_tree_id);
+    if (parent_it != ax_trees_.end())
+      parent_tree = parent_it->second.get();
+
+    if (parent_tree) {
+      ax_node = parent_tree->GetFromId(kv.second.parent_node_id);
+      if (ax_node) {
+        ax_node = ax_node->HasStringAttribute(
+                      ax::mojom::StringAttribute::kChildTreeId)
+                      ? ax_node
+                      : nullptr;
+      }
+    }
+
+    if (!child_tree && (!parent_tree || !ax_node)) {
+      // Both the child tree and the parent tree are gone, so this connection is
+      // no longer relevant.
+      connections_to_remove.push_back(kv.first);
+      continue;
+    }
+
+    if (!child_tree || (!parent_tree || !ax_node)) {
+      // Only one side of the connection is still valid, so mark the trees as
+      // disconnected and wait for either the connection to be done again or the
+      // other side to drop.
+      kv.second.is_connected = false;
+      continue;
+    }
+
+    if (kv.second.is_connected)
+      continue;  // No work to do, trees connected and present.
+
+    auto fuchsia_node = AXNodeDataToSemanticNode(
+        ax_node->data(), parent_ax_tree_id, false, id_mapper_.get());
+
+    // Now, the connection really happens:
+    // This node, from the parent tree, will have a child that points to the
+    // root of the child tree.
+    auto child_tree_root_id = id_mapper_->ToFuchsiaNodeID(
+        child_tree->GetAXTreeID(), child_tree->root()->id(), false);
+    fuchsia_node.mutable_child_ids()->push_back(child_tree_root_id);
+    to_update_.push_back(std::move(fuchsia_node));
+    kv.second.is_connected = true;  // Trees are connected!
+  }
+
+  for (const auto& to_delete : connections_to_remove) {
+    tree_connections_.erase(to_delete);
+  }
+}
+
+bool AccessibilityBridge::ShouldHoldCommit() {
+  const auto& main_frame_tree_id = web_contents_->GetMainFrame()->GetAXTreeID();
+  auto main_tree_it = ax_trees_.find(main_frame_tree_id);
+  if (main_tree_it == ax_trees_.end()) {
+    // The main tree is not present yet, commit should be held.
+    return true;
+  }
+
+  for (const auto& kv : tree_connections_) {
+    if (!kv.second.is_connected) {
+      // Trees are not connected, which means that a node is pointing to
+      // something that is not present yet. Since this causes an invalid tree,
+      // the commit should be held.
+      return true;
+    }
+  }
+  return false;
+}
+
+void AccessibilityBridge::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+  DCHECK(render_frame_host);
+
+  frame_id_to_tree_id_.erase(render_frame_host->GetGlobalFrameRoutingId());
+
+  const auto& id = render_frame_host->GetAXTreeID();
+  auto it = ax_trees_.find(id);
+  if (it != ax_trees_.end()) {
+    it->second.get()->Destroy();
+    MaybeDisconnectTreeFromParentTree(it->second.get());
+    ax_trees_.erase(it);
+    UpdateTreeConnections();
+    TryCommit();
+  }
+}
+
+bool AccessibilityBridge::UpdateAXTreeID(const ui::AXTreeID& tree_id) {
+  auto* frame = content::RenderFrameHost::FromAXTreeID(tree_id);
+  if (!frame)
+    return false;
+
+  auto frame_id = frame->GetGlobalFrameRoutingId();
+  DCHECK(frame_id);
+  auto frame_iter = frame_id_to_tree_id_.find(frame_id);
+  if (frame_iter == frame_id_to_tree_id_.end()) {
+    // This is the first time this frame was seen. Save its AXTreeID.
+    frame_id_to_tree_id_[frame_id] = tree_id;
+    return true;
+  }
+
+  // This frame already exists. Check if the AXTreeID of this frame has changed.
+  if (frame_iter->second == tree_id)
+    return true;  // No updates needed.
+
+  const auto& old_tree_id = frame_iter->second;
+  id_mapper_->UpdateAXTreeIDForCachedNodeIDs(old_tree_id, tree_id);
+  auto old_tree_iter = ax_trees_.find(old_tree_id);
+  if (old_tree_iter != ax_trees_.end()) {
+    // This AXTree has changed its AXTreeID. Update the map with the new key.
+    auto data = std::move(old_tree_iter->second);
+    ax_trees_.erase(old_tree_iter);
+    ax_trees_[tree_id] = std::move(data);
+
+    // If this tree is connected to a parent tree or is the parent tree of
+    // another tree, also update its ID in the tree connections map.
+    auto connected_tree_iter = tree_connections_.find(old_tree_id);
+    if (connected_tree_iter != tree_connections_.end()) {
+      auto data = std::move(connected_tree_iter->second);
+      tree_connections_.erase(connected_tree_iter);
+      tree_connections_[tree_id] = std::move(data);
+      MaybeDisconnectTreeFromParentTree(ax_trees_[tree_id].get());
+    }
+    for (auto& kv : tree_connections_) {
+      if (kv.second.parent_tree_id == old_tree_id)
+        kv.second.parent_tree_id = tree_id;
+    }
+  }
+
+  frame_iter->second = tree_id;
+  return true;
+}
+
+void AccessibilityBridge::MaybeDisconnectTreeFromParentTree(ui::AXTree* tree) {
+  const auto& key = tree->GetAXTreeID();
+  auto it = tree_connections_.find(key);
+  if (it != tree_connections_.end())
+    it->second.is_connected = false;
+}

fuchsia/engine/browser/accessibility_bridge.h

@@ -10,6 +10,7 @@
 #include <fuchsia/ui/views/cpp/fidl.h>
 #include <lib/fidl/cpp/binding.h>
 
+#include <base/containers/flat_map.h>
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/optional.h"
@@ -52,7 +53,7 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   AccessibilityBridge(const AccessibilityBridge&) = delete;
   AccessibilityBridge& operator=(const AccessibilityBridge&) = delete;
 
-  const ui::AXSerializableTree* ax_tree_for_test() { return &ax_tree_; }
+  const ui::AXSerializableTree* ax_tree_for_test();
 
   void set_event_received_callback_for_test(base::OnceClosure callback) {
     event_received_callback_for_test_ = std::move(callback);
@@ -67,9 +68,37 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   FRIEND_TEST_ALL_PREFIXES(AccessibilityBridgeTest,
                            TreeModificationsAreForwarded);
 
+  // Represents a connection between two AXTrees that are in different frames.
+  struct TreeConnection {
+    // ID of the node in the parent tree that points to this tree.
+    int32_t parent_node_id = 0;
+    // ID of the parent tree.
+    ui::AXTreeID parent_tree_id = ui::AXTreeIDUnknown();
+    // Whether the trees are connected.
+    bool is_connected = false;
+  };
+
   // Processes pending data and commits it to the Semantic Tree.
   void TryCommit();
 
+  // Connects trees if they are present or deletes the connection if both are
+  // gone.
+  void UpdateTreeConnections();
+
+  // Returns true if the main frame AXTree is not present or if trees are not
+  // connected.
+  bool ShouldHoldCommit();
+
+  // The AXTreeID of a tree can change. Updates all internal references of an
+  // AXTreeID by fetching the RenderFrameHost associated with |tree_id| and
+  // updates the value if it is different from the previously used AXTreeID.
+  // Returns false if the frame does not exist anymore, true otherwise.
+  bool UpdateAXTreeID(const ui::AXTreeID& tree_id);
+
+  // If |tree| is connected to another tree as its child, mark them as
+  // disconnected.
+  void MaybeDisconnectTreeFromParentTree(ui::AXTree* tree);
+
   // Callback for SemanticTree::CommitUpdates.
   void OnCommitComplete();
 
@@ -84,6 +113,7 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   // content::WebContentsObserver implementation.
   void AccessibilityEventReceived(
       const content::AXEventNotificationDetails& details) override;
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
 
   // fuchsia::accessibility::semantics::SemanticListener implementation.
   void OnAccessibilityActionRequested(
@@ -96,7 +126,7 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
                               OnSemanticsModeChangedCallback callback) final;
 
   // ui::AXTreeObserver implementation.
-  void OnNodeWillBeDeleted(ui::AXTree* tree, ui::AXNode* node) override;
+  void OnNodeDeleted(ui::AXTree* tree, int32_t node_id) override;
   void OnAtomicUpdateFinished(
       ui::AXTree* tree,
       bool root_changed,
@@ -105,7 +135,19 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   fuchsia::accessibility::semantics::SemanticTreePtr semantic_tree_;
   fidl::Binding<fuchsia::accessibility::semantics::SemanticListener> binding_;
   content::WebContents* web_contents_;
-  ui::AXSerializableTree ax_tree_;
+
+  // Holds one semantic tree per iframe.
+  base::flat_map<ui::AXTreeID, std::unique_ptr<ui::AXSerializableTree>>
+      ax_trees_;
+
+  // Maps frames to AXTrees.
+  base::flat_map<content::GlobalFrameRoutingId, ui::AXTreeID>
+      frame_id_to_tree_id_;
+
+  // Keeps track of semantic trees connections.
+  // The key is the AXTreeID of the semantic tree that is connected to another
+  // tree.
+  base::flat_map<ui::AXTreeID, TreeConnection> tree_connections_;
 
   // Whether semantic updates are enabled.
   bool enable_semantic_updates_ = false;
@@ -123,7 +165,7 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
   // will cause the frame |this| is owned by to be torn down.
   base::OnceCallback<void(zx_status_t)> on_error_callback_;
 
-  // The root id of |ax_tree_|.
+  // The root id of the AXTree of the main frame.
   int32_t root_id_ = 0;
 
   // Maps node IDs from one platform to another.

fuchsia/engine/browser/accessibility_bridge_browsertest.cc

@@ -659,9 +659,39 @@ IN_PROC_BROWSER_TEST_F(AccessibilityBridgeTest, OutOfProcessIframe) {
   EXPECT_TRUE(
       semantics_manager_.semantic_tree()->GetNodeFromLabel(kPageIframeTitle));
 
-  // TODO(https://crbug.com/1128954): Add support for combining AXTrees.
-  // Expect that the contents of the iframe are not available in the semantic
-  // tree.
+  // Data that is part of the iframe should be in the semantic tree.
+  EXPECT_TRUE(
+      semantics_manager_.semantic_tree()->GetNodeFromLabel(kButtonName1));
+
+  // Makes the iframe navigate to a different page.
+  GURL out_of_process_url_2 = second_test_server.GetURL(kPage2Path);
+  const auto script =
+      base::StringPrintf("document.getElementById(\"iframeId\").src = '%s'",
+                         out_of_process_url_2.spec().c_str());
+
+  frame_ptr_->ExecuteJavaScript(
+      {"*"}, cr_fuchsia::MemBufferFromString(script, "test2"),
+      [](fuchsia::web::Frame_ExecuteJavaScript_Result result) {
+        CHECK(result.is_response());
+      });
+
+  semantics_manager_.semantic_tree()->RunUntilCommitCountIs(4);
+
+  // check that the iframe navigated to a different page.
+  EXPECT_TRUE(semantics_manager_.semantic_tree()->GetNodeFromLabel(kNodeName));
+
+  // Old iframe data should be gone.
   EXPECT_FALSE(
       semantics_manager_.semantic_tree()->GetNodeFromLabel(kButtonName1));
+
+  // Makes the main page navigate to a different page, causing the iframe to go
+  // away.
+  LoadPage(kPage2Path, kPage2Title);
+
+  semantics_manager_.semantic_tree()->RunUntilCommitCountIs(5);
+
+  // We've navigated to a different page that has no iframes. Only one frame
+  // should be present.
+  num_frames = frame_impl_->web_contents_for_test()->GetAllFrames().size();
+  EXPECT_EQ(num_frames, 1);
 }