Commit c14fae86 authored by Lucas Radaelli's avatar Lucas Radaelli Committed by Commit Bot

[fuchsia][a11y] Refactors convert node ID logic.

This change refactors the logic to convert an ax node ID to a fuchsia semantic node ID.

It also fixes the bug where if a regular ax node had ID == 0, it would conflict with the root ID.

R=yangsharon

TEST: web_engine_unittests
Change-Id: Ie4ee52c65fbd5ad623986e015ad521e66c5e67fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430264Reviewed-by: default avatarKevin Marshall <kmarshall@chromium.org>
Reviewed-by: default avatarSharon Yang <yangsharon@chromium.org>
Commit-Queue: Lucas Radaelli <lucasradaelli@google.com>
Cr-Commit-Position: refs/heads/master@{#811926}
parent d45155e8
......@@ -115,17 +115,6 @@ void AccessibilityBridge::OnCommitComplete() {
TryCommit();
}
uint32_t AccessibilityBridge::ConvertToFuchsiaNodeId(int32_t ax_node_id) {
if (ax_node_id == root_id_) {
// On the Fuchsia side, the root node is indicated by id
// |kSemanticNodeRootId|, which is 0.
return kSemanticNodeRootId;
} else {
// AXNode ids are signed, Semantic Node ids are unsigned.
return bit_cast<uint32_t>(ax_node_id);
}
}
void AccessibilityBridge::AccessibilityEventReceived(
const content::AXEventNotificationDetails& details) {
// Updates to AXTree must be applied first.
......@@ -143,7 +132,7 @@ 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(ConvertToFuchsiaNodeId(event.id));
hit.set_node_id(ConvertToFuchsiaNodeId(event.id, root_id_));
// Run the pending callback with the hit.
pending_hit_test_callbacks_[event.action_request_id](std::move(hit));
......@@ -228,7 +217,7 @@ void AccessibilityBridge::DeleteSubtree(ui::AXNode* node) {
if (node->id() != root_id_) {
to_send_.push_back(
SemanticUpdateOrDelete(SemanticUpdateOrDelete::Type::DELETE, {},
ConvertToFuchsiaNodeId(node->id())));
ConvertToFuchsiaNodeId(node->id(), root_id_)));
}
for (ui::AXNode* child : node->children())
DeleteSubtree(child);
......
......@@ -86,10 +86,6 @@ class WEB_ENGINE_EXPORT AccessibilityBridge
// Callback for SemanticTree::CommitUpdates.
void OnCommitComplete();
// Converts AXNode ids to Semantic Node ids, and handles special casing of
// the 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.
......
......@@ -8,9 +8,11 @@
#include <fuchsia/ui/gfx/cpp/fidl.h>
#include <fuchsia/ui/views/cpp/fidl.h>
#include <lib/ui/scenic/cpp/commands.h>
#include <stdint.h>
#include <vector>
#include "base/bit_cast.h"
#include "base/numerics/safe_conversions.h"
#include "ui/accessibility/ax_enums.mojom.h"
#include "ui/accessibility/ax_tree_id.h"
#include "ui/gfx/geometry/rect_f.h"
......@@ -19,6 +21,14 @@ using fuchsia::accessibility::semantics::MAX_LABEL_SIZE;
namespace {
// Fuchsia's default root node ID.
constexpr uint32_t kFuchsiaRootNodeId = 0;
// Remmaped value for an AxNode ID that is 0 and is not a root.
// Value is chosen specifically to be outside the range of a 32-bit signed int,
// so as to not conflict with other values used by Chromium.
constexpr uint32_t kZeroIdRemappedForFuchsia = 1u + INT32_MAX;
fuchsia::accessibility::semantics::Role ConvertRole(ax::mojom::Role role) {
if (role == ax::mojom::Role::kButton)
return fuchsia::accessibility::semantics::Role::BUTTON;
......@@ -127,7 +137,7 @@ std::vector<uint32_t> ConvertChildIds(std::vector<int32_t> ids) {
std::vector<uint32_t> child_ids;
child_ids.reserve(ids.size());
for (auto i : ids) {
child_ids.push_back(bit_cast<uint32_t>(i));
child_ids.push_back(base::checked_cast<uint32_t>(i));
}
return child_ids;
}
......@@ -157,7 +167,7 @@ fuchsia::ui::gfx::mat4 ConvertTransform(gfx::Transform* transform) {
fuchsia::accessibility::semantics::Node AXNodeDataToSemanticNode(
const ui::AXNodeData& node) {
fuchsia::accessibility::semantics::Node fuchsia_node;
fuchsia_node.set_node_id(bit_cast<uint32_t>(node.id));
fuchsia_node.set_node_id(base::checked_cast<uint32_t>(node.id));
fuchsia_node.set_role(ConvertRole(node.role));
fuchsia_node.set_states(ConvertStates(node));
fuchsia_node.set_attributes(ConvertAttributes(node));
......@@ -192,3 +202,16 @@ bool ConvertAction(fuchsia::accessibility::semantics::Action fuchsia_action,
return false;
}
}
uint32_t ConvertToFuchsiaNodeId(int32_t ax_node_id, int32_t ax_root_node_id) {
if (ax_node_id == ax_root_node_id)
return kFuchsiaRootNodeId;
if (ax_node_id == kFuchsiaRootNodeId) {
// This AxNode is not the root, but its ID is the same as Fuchsia's root ID.
// We remap it to something different to not create a confflict.
return kZeroIdRemappedForFuchsia;
}
return base::checked_cast<uint32_t>(ax_node_id);
}
......@@ -24,4 +24,12 @@ AXNodeDataToSemanticNode(const ui::AXNodeData& node);
bool ConvertAction(fuchsia::accessibility::semantics::Action fuchsia_action,
ax::mojom::Action* mojom_action);
// Converts |ax_node_id|, which is signed, to a Fuchsia node ID, which is
// unsigned.
// Fuchsia requires root node IDs to be zero. This function ensures
// that the conversion takes that into account.
// If |ax_node_id| is 0 and is not the root, we return the
// MAX(int32_t) + 1, which is a number that will never conflict with other IDs.
uint32_t ConvertToFuchsiaNodeId(int32_t ax_node_id, int32_t ax_root_node_id);
#endif // FUCHSIA_ENGINE_BROWSER_AX_TREE_CONVERTER_H_
......@@ -41,6 +41,9 @@ 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.role = role;
node.AddAction(action);
node.AddIntAttribute(ax::mojom::IntAttribute::kCheckedState,
......@@ -113,8 +116,9 @@ TEST_F(AXTreeConverterTest, AllFieldsSetAndEqual) {
states.set_hidden(false);
states.set_selected(false);
auto expected_node = CreateSemanticNode(
source_node_data.id, Role::BUTTON, std::move(attributes),
std::move(states), std::vector<Action>{Action::SET_FOCUS},
static_cast<uint32_t>(source_node_data.id), Role::BUTTON,
std::move(attributes), std::move(states),
std::vector<Action>{Action::SET_FOCUS},
std::vector<uint32_t>{kChildId1, kChildId2, kChildId3}, box, mat.value);
EXPECT_TRUE(fidl::Equals(converted_node, expected_node));
......@@ -122,6 +126,7 @@ TEST_F(AXTreeConverterTest, AllFieldsSetAndEqual) {
TEST_F(AXTreeConverterTest, SomeFieldsSetAndEqual) {
ui::AXNodeData source_node_data;
source_node_data.id = 0;
source_node_data.AddAction(ax::mojom::Action::kFocus);
source_node_data.AddAction(ax::mojom::Action::kSetValue);
source_node_data.child_ids = std::vector<int32_t>{kChildId1};
......@@ -133,10 +138,11 @@ TEST_F(AXTreeConverterTest, SomeFieldsSetAndEqual) {
converted_node.node_id());
Node expected_node;
expected_node.set_node_id(source_node_data.id);
expected_node.set_node_id(static_cast<uint32_t>(source_node_data.id));
expected_node.set_actions(
std::vector<Action>{Action::SET_FOCUS, Action::SET_VALUE});
expected_node.set_child_ids(std::vector<uint32_t>{kChildId1});
expected_node.set_child_ids(
std::vector<uint32_t>{static_cast<uint32_t>(kChildId1)});
expected_node.set_role(Role::IMAGE);
States states;
states.set_hidden(false);
......@@ -198,4 +204,19 @@ TEST_F(AXTreeConverterTest, FieldMismatch) {
EXPECT_FALSE(fidl::Equals(converted_node, expected_node));
}
TEST_F(AXTreeConverterTest, ConvertToFuchsiaNodeId) {
// Root AxNode is 0, Fuchsia is also 0.
EXPECT_EQ(0u, ConvertToFuchsiaNodeId(0, 0));
// Root AxNode is not 0, Fuchsia is still 0.
EXPECT_EQ(0u, ConvertToFuchsiaNodeId(2, 2));
// Regular AxNode is 0, Fuchsia can't be 0.
EXPECT_EQ(static_cast<uint32_t>(std::numeric_limits<int32_t>::max()) + 1,
ConvertToFuchsiaNodeId(0, 2));
// Regular AxNode is not 0, Fuchsia is same value.
EXPECT_EQ(10u, ConvertToFuchsiaNodeId(10, 0));
}
} // namespace
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment