arc-a11y: Clean up AccessibilityInfoDataWrapper::CanBeAccessibilityFocused

CL:2222068 added IsAccessibilityFocusableContainer() in
AccessibilityNodeInfoDataWrapper. The functionality of the method is
very similar to CanBeAccessibilityFocused().
This change cleans up AccessibilityNodeInfoDataWrapper.

AX-Relnotes: n/a.
Bug: b:152930082
Test: unit_tests --gtest_filter="AXTreeSourceArcTest.*"
Test: tast tests still passes.
Change-Id: Ie461cdeae43278aabc21a6537dd530bec3415554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2246120
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779643}

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

@@ -84,45 +84,20 @@ bool AccessibilityNodeInfoDataWrapper::IsIgnored() const {
   if (IsAccessibilityFocusableContainer())
     return false;
 
-  // On screen reader mode, text might be used by focusable ancestor.
-  // Make such nodes ignored. See ComputeNameFromContents().
-  bool has_text = HasNonEmptyStringProperty(
-                      node_ptr_, AXStringProperty::CONTENT_DESCRIPTION) ||
-                  HasNonEmptyStringProperty(node_ptr_, AXStringProperty::TEXT);
-  if (!has_text)
-    return false;
+  if (!HasText())
+    return false;  // A layout container with a11y importance.
 
-  AccessibilityInfoDataWrapper* parent = tree_source_->GetParent(
-      const_cast<AccessibilityNodeInfoDataWrapper*>(this));
-  while (parent && parent->IsNode()) {
-    if (parent->IsAccessibilityFocusableContainer())
-      return true;
-    parent = tree_source_->GetParent(parent);
-  }
-  return false;
+  return !HasAccessibilityFocusableText();
 }
 
 bool AccessibilityNodeInfoDataWrapper::CanBeAccessibilityFocused() const {
-  // TODO(hirokisato): Remove this method and only use
-  // IsAccessibilityFocusableContainer().
-
-  // An important node with a non-generic role and:
-  // - actionable nodes
-  // - top level scrollables with a name
-  // - interesting leaf nodes
-  ui::AXNodeData data;
-  PopulateAXRole(&data);
-  bool non_generic_role = data.role != ax::mojom::Role::kGenericContainer &&
-                          data.role != ax::mojom::Role::kGroup &&
-                          data.role != ax::mojom::Role::kList &&
-                          data.role != ax::mojom::Role::kGrid;
-  bool actionable = GetProperty(AXBooleanProperty::CLICKABLE) ||
-                    GetProperty(AXBooleanProperty::FOCUSABLE) ||
-                    GetProperty(AXBooleanProperty::CHECKABLE);
-  bool top_level_scrollable = HasProperty(AXStringProperty::TEXT) &&
-                              GetProperty(AXBooleanProperty::SCROLLABLE);
-  return !IsIgnored() && non_generic_role &&
-         (actionable || top_level_scrollable || IsInterestingLeaf());
+  // Using HasText() here is incomplete because it doesn't match the
+  // populated ax name. However, this method is used only from AXTreeSourceArc
+  // and not used for actual focusability computation of ChromeVox, and it's
+  // enough to check only hasText().
+  return (IsAccessibilityFocusableContainer() ||
+          HasAccessibilityFocusableText()) &&
+         HasText();
 }
 
 bool AccessibilityNodeInfoDataWrapper::IsAccessibilityFocusableContainer()
@@ -639,6 +614,29 @@ bool AccessibilityNodeInfoDataWrapper::HasCoveringSpan(
   return false;
 }
 
+bool AccessibilityNodeInfoDataWrapper::HasText() const {
+  // The same properties are checked as ComputeNameFromContentsInternal.
+  return HasNonEmptyStringProperty(node_ptr_,
+                                   AXStringProperty::CONTENT_DESCRIPTION) ||
+         HasNonEmptyStringProperty(node_ptr_, AXStringProperty::TEXT);
+}
+
+bool AccessibilityNodeInfoDataWrapper::HasAccessibilityFocusableText() const {
+  if (!is_important_ || !HasText())
+    return false;
+
+  // If any ancestor has a focusable property, the text is used by that node.
+  AccessibilityInfoDataWrapper* parent =
+      tree_source_->GetFirstImportantAncestor(
+          const_cast<AccessibilityNodeInfoDataWrapper*>(this));
+  while (parent && parent->IsNode()) {
+    if (parent->IsAccessibilityFocusableContainer())
+      return false;
+    parent = tree_source_->GetFirstImportantAncestor(parent);
+  }
+  return true;
+}
+
 void AccessibilityNodeInfoDataWrapper::ComputeNameFromContents(
     std::vector<std::string>* names) const {
   std::vector<AccessibilityInfoDataWrapper*> children;
@@ -702,12 +700,4 @@ bool AccessibilityNodeInfoDataWrapper::IsToplevelScrollItem() const {
       ->IsScrollableContainer();
 }
 
-bool AccessibilityNodeInfoDataWrapper::IsInterestingLeaf() const {
-  std::vector<AccessibilityInfoDataWrapper*> children;
-  GetChildren(&children);
-  // TODO(hirokisato) Even if the node has children, they might be empty. In
-  // this case we should return true.
-  return HasImportantProperty(node_ptr_) && children.empty();
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/accessibility/accessibility_node_info_data_wrapper.h

@@ -67,13 +67,14 @@ class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper {
   bool HasCoveringSpan(mojom::AccessibilityStringProperty prop,
                        mojom::SpanType span_type) const;
 
-  void ComputeNameFromContents(std::vector<std::string>* names) const;
+  bool HasText() const;
+  bool HasAccessibilityFocusableText() const;
 
+  void ComputeNameFromContents(std::vector<std::string>* names) const;
   void ComputeNameFromContentsInternal(std::vector<std::string>* names) const;
 
   bool IsScrollableContainer() const;
   bool IsToplevelScrollItem() const;
-  bool IsInterestingLeaf() const;
 
   mojom::AccessibilityNodeInfoData* node_ptr_ = nullptr;