[fuchsia] Use node id conversion functions instead of checked_cast

In AXTreeConverter, use ConvertToFuchsiaNodeId to handle node id edge
cases.

Test: AXTreeConverterTest
Bug: fuchsia:60692
Change-Id: I2fe50a5f522d2a58d8cb88bf3f086173a6c6ea62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466617
Commit-Queue: Sharon Yang <yangsharon@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816650}

fuchsia/engine/browser/accessibility_bridge.cc

@@ -220,9 +220,8 @@ void AccessibilityBridge::OnAtomicUpdateFinished(
   // deleted are already gone, which means that all updates collected here in
   // |to_update_| are going to be executed after |to_delete_|.
   for (const ui::AXTreeObserver::Change& change : changes) {
-    ui::AXNodeData ax_data = change.node->data();
-    ax_data.id = ConvertToFuchsiaNodeId(change.node->id(), root_id_);
-    to_update_.push_back(AXNodeDataToSemanticNode(ax_data));
+    to_update_.push_back(
+        AXNodeDataToSemanticNode(change.node->data(), root_id_));
   }
   // TODO(https://crbug.com/1134737): Separate updates of atomic updates and
   // don't allow all of them to be in the same commit.

fuchsia/engine/browser/ax_tree_converter.cc

@@ -175,11 +175,12 @@ std::vector<fuchsia::accessibility::semantics::Action> ConvertActions(
   return fuchsia_actions;
 }
 
-std::vector<uint32_t> ConvertChildIds(std::vector<int32_t> ids) {
+std::vector<uint32_t> ConvertChildIds(std::vector<int32_t> ids,
+                                      int32_t ax_root_id) {
   std::vector<uint32_t> child_ids;
   child_ids.reserve(ids.size());
   for (auto i : ids) {
-    child_ids.push_back(base::checked_cast<uint32_t>(i));
+    child_ids.push_back(ConvertToFuchsiaNodeId(i, ax_root_id));
   }
   return child_ids;
 }
@@ -207,14 +208,15 @@ fuchsia::ui::gfx::mat4 ConvertTransform(gfx::Transform* transform) {
 }  // namespace
 
 fuchsia::accessibility::semantics::Node AXNodeDataToSemanticNode(
-    const ui::AXNodeData& node) {
+    const ui::AXNodeData& node,
+    int32_t ax_root_id) {
   fuchsia::accessibility::semantics::Node fuchsia_node;
-  fuchsia_node.set_node_id(base::checked_cast<uint32_t>(node.id));
+  fuchsia_node.set_node_id(ConvertToFuchsiaNodeId(node.id, ax_root_id));
   fuchsia_node.set_role(AxRoleToFuchsiaSemanticRole(node.role));
   fuchsia_node.set_states(ConvertStates(node));
   fuchsia_node.set_attributes(ConvertAttributes(node));
   fuchsia_node.set_actions(ConvertActions(node));
-  fuchsia_node.set_child_ids(ConvertChildIds(node.child_ids));
+  fuchsia_node.set_child_ids(ConvertChildIds(node.child_ids, ax_root_id));
   fuchsia_node.set_location(ConvertBoundingBox(node.relative_bounds.bounds));
   if (node.relative_bounds.transform) {
     fuchsia_node.set_transform(

fuchsia/engine/browser/ax_tree_converter.h

@@ -16,7 +16,7 @@
 // present. Those that are will be converted. The Fuchsia SemanticsManager
 // accepts partial updates, so |node| does not require all fields to be set.
 WEB_ENGINE_EXPORT fuchsia::accessibility::semantics::Node
-AXNodeDataToSemanticNode(const ui::AXNodeData& node);
+AXNodeDataToSemanticNode(const ui::AXNodeData& node, int32_t ax_root_id);
 
 // Converts Fuchsia action of type |fuchsia_action| to an ax::mojom::Action of
 // type |mojom_action|. Function will return true if |fuchsia_action| is

fuchsia/engine/browser/ax_tree_converter_unittest.cc

@@ -42,9 +42,7 @@ ui::AXNodeData CreateAXNodeData(ax::mojom::Role role,
                                 base::StringPiece description,
                                 ax::mojom::CheckedState checked_state) {
   ui::AXNodeData node;
-  // Important! ID must be set to zero here because its default value (-1), will
-  // fail when getting converted to an unsigned int (Fuchsia's ID format).
-  node.id = 0;
+  node.id = 2;
   node.role = role;
   node.AddAction(action);
   node.AddIntAttribute(ax::mojom::IntAttribute::kCheckedState,
@@ -129,10 +127,7 @@ TEST_F(AXTreeConverterTest, AllFieldsSetAndEqual) {
   auto& source_node_data = nodes.first;
   auto& expected_node = nodes.second;
 
-  auto converted_node = AXNodeDataToSemanticNode(source_node_data);
-  EXPECT_EQ(ConvertToFuchsiaNodeId(source_node_data.id, kRootId),
-            converted_node.node_id());
-
+  auto converted_node = AXNodeDataToSemanticNode(source_node_data, kRootId);
   EXPECT_TRUE(fidl::Equals(converted_node, expected_node));
 }
 
@@ -145,16 +140,15 @@ TEST_F(AXTreeConverterTest, SomeFieldsSetAndEqual) {
   source_node_data.role = ax::mojom::Role::kImage;
   source_node_data.AddStringAttribute(ax::mojom::StringAttribute::kValue,
                                       kValue1);
-  auto converted_node = AXNodeDataToSemanticNode(source_node_data);
-  EXPECT_EQ(static_cast<uint32_t>(source_node_data.id),
-            converted_node.node_id());
+  auto converted_node = AXNodeDataToSemanticNode(source_node_data, kRootId);
 
   Node expected_node;
-  expected_node.set_node_id(static_cast<uint32_t>(source_node_data.id));
+  expected_node.set_node_id(
+      ConvertToFuchsiaNodeId(source_node_data.id, kRootId));
   expected_node.set_actions(
       std::vector<Action>{Action::SET_FOCUS, Action::SET_VALUE});
   expected_node.set_child_ids(
-      std::vector<uint32_t>{static_cast<uint32_t>(kChildId1)});
+      std::vector<uint32_t>{ConvertToFuchsiaNodeId(kChildId1, kRootId)});
   expected_node.set_role(Role::IMAGE);
   States states;
   states.set_hidden(false);
@@ -178,9 +172,7 @@ TEST_F(AXTreeConverterTest, FieldMismatch) {
       ax::mojom::Role::kHeader, ax::mojom::Action::kSetValue,
       std::vector<int32_t>{kChildId1, kChildId2, kChildId3}, relative_bounds,
       kLabel1, kDescription1, ax::mojom::CheckedState::kFalse);
-  auto converted_node = AXNodeDataToSemanticNode(source_node_data);
-  EXPECT_EQ(static_cast<uint32_t>(source_node_data.id),
-            converted_node.node_id());
+  auto converted_node = AXNodeDataToSemanticNode(source_node_data, kRootId);
 
   Attributes attributes;
   attributes.set_label(kLabel1);
@@ -206,13 +198,13 @@ TEST_F(AXTreeConverterTest, FieldMismatch) {
   auto modified_node_data = source_node_data;
   modified_node_data.AddStringAttribute(ax::mojom::StringAttribute::kName,
                                         kLabel2);
-  converted_node = AXNodeDataToSemanticNode(modified_node_data);
+  converted_node = AXNodeDataToSemanticNode(modified_node_data, kRootId);
   EXPECT_FALSE(fidl::Equals(converted_node, expected_node));
 
   // The same as above, this time changing |child_ids|.
   modified_node_data = source_node_data;
   modified_node_data.child_ids = std::vector<int32_t>{};
-  converted_node = AXNodeDataToSemanticNode(modified_node_data);
+  converted_node = AXNodeDataToSemanticNode(modified_node_data, kRootId);
   EXPECT_FALSE(fidl::Equals(converted_node, expected_node));
 }
 
@@ -228,7 +220,7 @@ TEST_F(AXTreeConverterTest, DefaultAction) {
       expected_node.mutable_actions()->begin(),
       fuchsia::accessibility::semantics::Action::DEFAULT);
 
-  auto converted_node = AXNodeDataToSemanticNode(source_node_data);
+  auto converted_node = AXNodeDataToSemanticNode(source_node_data, kRootId);
   EXPECT_EQ(ConvertToFuchsiaNodeId(source_node_data.id, kRootId),
             converted_node.node_id());
 
@@ -255,39 +247,39 @@ TEST_F(AXTreeConverterTest, ConvertRoles) {
   node.id = 0;
   node.role = ax::mojom::Role::kButton;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::BUTTON,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kCheckBox;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::CHECK_BOX,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kHeader;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::HEADER,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kImage;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::IMAGE,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kLink;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::LINK,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kRadioButton;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::RADIO_BUTTON,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kSlider;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::SLIDER,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kTextField;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::TEXT_FIELD,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 
   node.role = ax::mojom::Role::kStaticText;
   EXPECT_EQ(fuchsia::accessibility::semantics::Role::STATIC_TEXT,
-            AXNodeDataToSemanticNode(node).role());
+            AXNodeDataToSemanticNode(node, kRootId).role());
 }
 
 }  // namespace