Complete initial role and state mappings for ARC++ accessibility

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

Index: chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc
diff --git a/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc b/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc
index c65d9a4a8dcbf4fe204e568a86d0317177f708ce..943569aa015a52b41fc5f487c8736f53d9e972ce 100644
--- a/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc
+++ b/chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/aura/accessibility/automation_manager_aura.h"
 #include "chrome/common/extensions/chrome_extension_messages.h"
 #include "components/exo/wm_helper.h"
+#include "ui/accessibility/platform/ax_android_constants.h"
 #include "ui/aura/window.h"
 
 namespace {
@@ -70,6 +71,18 @@ const gfx::Rect GetBounds(arc::mojom::AccessibilityNodeInfoData* node) {
   return bounds_in_screen;
 }
 
+bool GetBooleanProperty(arc::mojom::AccessibilityNodeInfoData* node,
+                        arc::mojom::AccessibilityBooleanProperty prop) {
+  if (!node->booleanProperties)
+    return false;
+
+  auto it = node->booleanProperties->find(prop);
+  if (it == node->booleanProperties->end())
+    return false;
+
+  return it->second;
+}
+
 bool GetStringProperty(arc::mojom::AccessibilityNodeInfoData* node,
                        arc::mojom::AccessibilityStringProperty prop,
                        std::string* out_value) {
@@ -84,6 +97,77 @@ bool GetStringProperty(arc::mojom::AccessibilityNodeInfoData* node,
   return true;
 }
 
+ui::AXRole GetAXRole(arc::mojom::AccessibilityNodeInfoData* node) {
+  std::string class_name, text;
+  GetStringProperty(node, arc::mojom::AccessibilityStringProperty::CLASS_NAME,
+                    &class_name);
+  GetStringProperty(node, arc::mojom::AccessibilityStringProperty::TEXT, &text);

[yawano, 2017/02/28 05:36:07]

nit: We can get this later. We are not using the text until no role matches in
the list. We can get text after we matched with all roles.

+
+#define MAP_ROLE(android_class_name, chrome_role) \

[dmazzoni, 2017/02/28 04:31:25]

Perhaps these should be a hash_map from string to ui::AXRole, at least as much
as possible?

[David Tseng, 2017/02/28 18:52:30]

It's not one to one and will probably be less so as time goes on, so let's keep
it this way. Happy to make this cleaner in any other ways you can think of.

+  if (class_name == android_class_name)           \
+    return chrome_role;
+
+  MAP_ROLE(ui::kAXAbsListViewClassname, ui::AX_ROLE_LIST);
+  MAP_ROLE(ui::kAXButtonClassname, ui::AX_ROLE_BUTTON);
+  MAP_ROLE(ui::kAXCheckBoxClassname, ui::AX_ROLE_CHECK_BOX);
+  MAP_ROLE(ui::kAXCompoundButtonClassname, ui::AX_ROLE_CHECK_BOX);
+  MAP_ROLE(ui::kAXCheckedTextViewClassname, ui::AX_ROLE_STATIC_TEXT);
+  MAP_ROLE(ui::kAXDialogClassname, ui::AX_ROLE_DIALOG);
+  MAP_ROLE(ui::kAXEditTextClassname, ui::AX_ROLE_TEXT_FIELD);
+  MAP_ROLE(ui::kAXGridViewClassname, ui::AX_ROLE_TABLE);
+  MAP_ROLE(ui::kAXImageClassname, ui::AX_ROLE_IMAGE);
+  if (GetBooleanProperty(node,
+                         arc::mojom::AccessibilityBooleanProperty::CLICKABLE)) {
+    MAP_ROLE(ui::kAXImageViewClassname, ui::AX_ROLE_BUTTON);
+  } else {
+    MAP_ROLE(ui::kAXImageViewClassname, ui::AX_ROLE_IMAGE);
+  }
+  MAP_ROLE(ui::kAXListViewClassname, ui::AX_ROLE_LIST);
+  MAP_ROLE(ui::kAXMenuItemClassname, ui::AX_ROLE_MENU_ITEM);
+  MAP_ROLE(ui::kAXPagerClassname, ui::AX_ROLE_BUTTON);
+  MAP_ROLE(ui::kAXProgressBarClassname, ui::AX_ROLE_PROGRESS_INDICATOR);
+  MAP_ROLE(ui::kAXRadioButtonClassname, ui::AX_ROLE_RADIO_BUTTON);
+  MAP_ROLE(ui::kAXSeekBarClassname, ui::AX_ROLE_SLIDER);
+  MAP_ROLE(ui::kAXSwitchClassname, ui::AX_ROLE_SWITCH);
+  MAP_ROLE(ui::kAXSpinnerClassname, ui::AX_ROLE_SPIN_BUTTON);

[yawano, 2017/02/28 05:36:07]

question: We have different mapping for AX_ROLE_SPIN_BUTTON in
browser_accessibility_android.cc. Is this intentional? In
browser_accessibility_android.cc, ui::AX_ROLE_SPIN_BUTTON is mapped to
ui::kAxEditTextClassname.

[David Tseng, 2017/02/28 18:50:37]

I think the more appropriate mapping isto a pop_up_button.

BAA maps a few things to spinner (one which is popup button).
Role.java maps spinner to a drop down list.

[yawano, 2017/03/01 03:49:51]

sgtm.

+  MAP_ROLE(ui::kAXTabWidgetClassname, ui::AX_ROLE_TAB_LIST);
+  MAP_ROLE(ui::kAXToggleButtonClassname, ui::AX_ROLE_TOGGLE_BUTTON);
+  MAP_ROLE(ui::kAXViewClassname, ui::AX_ROLE_DIV);
+  MAP_ROLE(ui::kAXViewGroupClassname, ui::AX_ROLE_GROUP);
+  MAP_ROLE(ui::kAXWebViewClassname, ui::AX_ROLE_WEB_VIEW);
+
+#undef MAP_ROLE
+
+  if (!text.empty())

[dmazzoni, 2017/02/28 04:31:25]

How about only if it's a leaf node. The rest of the code will be
very confused if a static text node has children (other than
inline text boxes)

[David Tseng, 2017/02/28 18:50:37]

This is the fallback case (in case we can't assign any role to the node).

From what I've seen and what BrowserAccessibilityAndroid does, this seems to be
reasonable behavior (i.e. the chlidren are text nodes themselves) so ChromeVox
would stop descending here. Let me know if you have any specific cases where you
think this would break.

+    return ui::AX_ROLE_STATIC_TEXT;
+  else
+    return ui::AX_ROLE_DIV;
+}
+
+int32_t GetAXState(arc::mojom::AccessibilityNodeInfoData* node) {
+  int32_t state = 0;
+
+#define MAP_STATE(android_boolean_property, chrome_state) \
+  if (GetBooleanProperty(node, android_boolean_property)) \
+    state |= 1 << chrome_state;

[yawano, 2017/02/28 05:36:07]

Can we change GetAxState to be passed AxNodeData and use
AxNodeData::AddStateFlag here? It would be better if we could capsule the
implementation details (i.e. 1 << chrome_state) into AxNodeData.

[David Tseng, 2017/02/28 18:50:37]

Done

+
+  using AXBooleanProperty = arc::mojom::AccessibilityBooleanProperty;
+
+  MAP_STATE(AXBooleanProperty::CHECKED, ui::AX_STATE_CHECKED);
+  MAP_STATE(AXBooleanProperty::EDITABLE, ui::AX_STATE_EDITABLE);
+  MAP_STATE(AXBooleanProperty::FOCUSABLE, ui::AX_STATE_FOCUSABLE);
+  MAP_STATE(AXBooleanProperty::MULTI_LINE, ui::AX_STATE_MULTILINE);
+  MAP_STATE(AXBooleanProperty::PASSWORD, ui::AX_STATE_PROTECTED);
+  MAP_STATE(AXBooleanProperty::SELECTED, ui::AX_STATE_SELECTED);
+
+#undef MAP_STATE
+
+  if (!GetBooleanProperty(node, AXBooleanProperty::ENABLED))
+    state |= 1 << ui::AX_STATE_DISABLED;
+
+  return state;
+}
+
 }  // namespace
 
 namespace arc {
@@ -103,7 +187,6 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(
   tree_map_.clear();
   parent_map_.clear();
   root_id_ = -1;
-  focused_node_id_ = -1;
   for (size_t i = 0; i < event_data->nodeData.size(); ++i) {
     if (!event_data->nodeData[i]->intListProperties)
       continue;
@@ -128,6 +211,8 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(
   params.event_type = ToAXEvent(event_data->eventType);
   if (params.event_type == ui::AX_EVENT_FOCUS)
     focused_node_id_ = params.id;
+  else if (params.event_type == ui::AX_EVENT_BLUR)
+    focused_node_id_ = -1;
   params.tree_id = tree_id_;
   params.id = event_data->sourceId;
 
@@ -210,7 +295,6 @@ void AXTreeSourceArc::SerializeNode(mojom::AccessibilityNodeInfoData* node,
   if (!node)
     return;
   out_data->id = node->id;
-  out_data->state = 0;
 
   using AXStringProperty = arc::mojom::AccessibilityStringProperty;
   std::string text;
@@ -223,15 +307,18 @@ void AXTreeSourceArc::SerializeNode(mojom::AccessibilityNodeInfoData* node,
   int32_t id = node->id;
   if (id == root_id_)
     out_data->role = ui::AX_ROLE_ROOT_WEB_AREA;
-  else if (!text.empty())
-    out_data->role = ui::AX_ROLE_STATIC_TEXT;
   else
-    out_data->role = ui::AX_ROLE_DIV;
+    out_data->role = GetAXRole(node);
+
+  out_data->state = GetAXState(node);
 
   const gfx::Rect bounds_in_screen = GetBounds(node);
   out_data->location.SetRect(bounds_in_screen.x(), bounds_in_screen.y(),
                              bounds_in_screen.width(),
                              bounds_in_screen.height());
+
+  if (out_data->role == ui::AX_ROLE_TEXT_FIELD && !text.empty())
+    out_data->AddStringAttribute(ui::AX_ATTR_VALUE, text);
 }
 
 void AXTreeSourceArc::Reset() {