Reorder ARC++ accessibility nodes using layout

This change is largely motivated by TalkBack utils.

The problem
It seems as though Android's accessibility tree is not in-order (rendered from left to right, top to bottom) when walking the tree in pre order.

As a result, app drawers appear at the end of the navigation linearization.

Bug: 813721
Test: navigate linearly through Play Store. Verify navigation button/app text/search text field get seen first, then the app contents
Change-Id: I489b18708b8fb3a03a9d6e0cf0d42c830c9ac21f
Reviewed-on: https://chromium-review.googlesource.com/928047Reviewed-by: Yuki Awano <yawano@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539065}

chrome/browser/chromeos/BUILD.gn

@@ -1750,6 +1750,7 @@ source_set("unit_tests") {
     "accessibility/spoken_feedback_event_rewriter_unittest.cc",
     "app_mode/startup_app_launcher_unittest.cc",
     "arc/accessibility/arc_accessibility_helper_bridge_unittest.cc",
+    "arc/accessibility/ax_tree_source_arc_unittest.cc",
     "arc/arc_play_store_enabled_preference_handler_unittest.cc",
     "arc/arc_session_manager_unittest.cc",
     "arc/arc_support_host_unittest.cc",

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

@@ -408,6 +408,12 @@ bool AXTreeSourceArc::GetTreeData(ui::AXTreeData* data) const {
   return true;
 }
 
+void AXTreeSourceArc::GetChildrenForTest(
+    mojom::AccessibilityNodeInfoData* node,
+    std::vector<mojom::AccessibilityNodeInfoData*>* out_children) const {
+  GetChildren(node, out_children);
+}
+
 mojom::AccessibilityNodeInfoData* AXTreeSourceArc::GetRoot() const {
   mojom::AccessibilityNodeInfoData* root = GetFromId(root_id_);
   return root;
@@ -437,9 +443,43 @@ void AXTreeSourceArc::GetChildren(
   if (it == node->int_list_properties->end())
     return;
 
-  for (size_t i = 0; i < it->second.size(); ++i) {
+  for (size_t i = 0; i < it->second.size(); ++i)
     out_children->push_back(GetFromId(it->second[i]));
-  }
+
+  // Sort children based on their enclosing bounding rectangles, based on their
+  // descendants.
+  std::sort(out_children->begin(), out_children->end(),
+            [this](auto left, auto right) {
+              auto left_bounds = ComputeEnclosingBounds(left);
+              auto right_bounds = ComputeEnclosingBounds(right);
+
+              // Top to bottom sort (non-overlapping).
+              if (!left_bounds.Intersects(right_bounds))
+                return left_bounds.y() < right_bounds.y();
+
+              // Overlapping
+              // Left to right.
+              int left_difference = left_bounds.x() - right_bounds.x();
+              if (left_difference != 0)
+                return left_difference < 0;
+
+              // Top to bottom.
+              int top_difference = left_bounds.y() - right_bounds.y();
+              if (top_difference != 0)
+                return top_difference < 0;
+
+              // Larger to smaller.
+              int height_difference =
+                  left_bounds.height() - right_bounds.height();
+              if (height_difference != 0)
+                return height_difference > 0;
+
+              int width_difference = left_bounds.width() - right_bounds.width();
+              if (width_difference != 0)
+                return width_difference > 0;
+
+              return true;
+            });
 }
 
 mojom::AccessibilityNodeInfoData* AXTreeSourceArc::GetParent(
@@ -529,7 +569,7 @@ void AXTreeSourceArc::SerializeNode(mojom::AccessibilityNodeInfoData* node,
   if (root_id_ != -1 && wm_helper) {
     aura::Window* focused_window =
         is_notification_ ? nullptr : wm_helper->GetFocusedWindow();
-    const gfx::Rect local_bounds = GetBounds(node, focused_window);
+    const gfx::Rect& local_bounds = GetBounds(node, focused_window);
     out_data->location.SetRect(local_bounds.x(), local_bounds.y(),
                                local_bounds.width(), local_bounds.height());
   }
@@ -595,6 +635,44 @@ const gfx::Rect AXTreeSourceArc::GetBounds(
   return node_bounds;
 }
 
+gfx::Rect AXTreeSourceArc::ComputeEnclosingBounds(
+    mojom::AccessibilityNodeInfoData* node) const {
+  gfx::Rect computed_bounds;
+  ComputeEnclosingBoundsInternal(node, computed_bounds);
+  return computed_bounds.IsEmpty() ? node->bounds_in_screen : computed_bounds;
+}
+
+void AXTreeSourceArc::ComputeEnclosingBoundsInternal(
+    mojom::AccessibilityNodeInfoData* node,
+    gfx::Rect& computed_bounds) const {
+  // Only consider nodes that can possibly be accessibility focused. In Chrome,
+  // this amounts to nodes with a non-generic container role.
+  ui::AXNodeData data;
+  PopulateAXRole(node, &data);
+  const gfx::Rect& bounds = node->bounds_in_screen;
+  if (data.role != ax::mojom::Role::kGenericContainer &&
+      data.role != ax::mojom::Role::kGroup && !bounds.IsEmpty() &&
+      GetBooleanProperty(
+          node, arc::mojom::AccessibilityBooleanProperty::VISIBLE_TO_USER)) {
+    computed_bounds.Union(bounds);
+    return;
+  }
+
+  if (!node->int_list_properties)
+    return;
+
+  auto it = node->int_list_properties->find(
+      arc::mojom::AccessibilityIntListProperty::CHILD_NODE_IDS);
+  if (it == node->int_list_properties->end())
+    return;
+
+  for (size_t i = 0; i < it->second.size(); ++i) {
+    ComputeEnclosingBoundsInternal(GetFromId(it->second[i]), computed_bounds);
+  }
+
+  return;
+}
+
 void AXTreeSourceArc::PerformAction(const ui::AXActionData& data) {
   delegate_->OnAction(data);
 }

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

@@ -59,6 +59,11 @@ class AXTreeSourceArc
   // Gets the window id of this tree.
   int32_t window_id() const { return window_id_; }
 
+  // Gets children for testing.
+  void GetChildrenForTest(
+      mojom::AccessibilityNodeInfoData* node,
+      std::vector<mojom::AccessibilityNodeInfoData*>* out_children) const;
+
  private:
   class FocusStealer;
 
@@ -88,6 +93,15 @@ class AXTreeSourceArc
   const gfx::Rect GetBounds(mojom::AccessibilityNodeInfoData* node,
                             aura::Window* focused_window) const;
 
+  // Computes the smallest rect that encloses all of the descendants of |node|.
+  gfx::Rect ComputeEnclosingBounds(
+      mojom::AccessibilityNodeInfoData* node) const;
+
+  // Helper to recursively compute bounds for |node|. Returns true if non-empty
+  // bounds were encountered.
+  void ComputeEnclosingBoundsInternal(mojom::AccessibilityNodeInfoData* node,
+                                      gfx::Rect& computed_bounds) const;
+
   // AXHostDelegate overrides.
   void PerformAction(const ui::AXActionData& data) override;
 

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

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h"
+
+#include "components/arc/common/accessibility_helper.mojom.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/accessibility/platform/ax_android_constants.h"
+
+namespace arc {
+
+class AXTreeSourceArcTest : public testing::Test,
+                            public AXTreeSourceArc::Delegate {
+ public:
+  AXTreeSourceArcTest() = default;
+
+ private:
+  void OnAction(const ui::AXActionData& data) const override {}
+
+  DISALLOW_COPY_AND_ASSIGN(AXTreeSourceArcTest);
+};
+
+TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
+  AXTreeSourceArc tree(this);
+
+  auto event1 = arc::mojom::AccessibilityEventData::New();
+  event1->source_id = 0;
+  event1->task_id = 1;
+  event1->event_type = arc::mojom::AccessibilityEventType::VIEW_FOCUSED;
+  event1->node_data.push_back(arc::mojom::AccessibilityNodeInfoData::New());
+  event1->node_data[0]->id = 0;
+  event1->node_data[0]->int_list_properties =
+      std::unordered_map<arc::mojom::AccessibilityIntListProperty,
+                         std::vector<int>>();
+  event1->node_data[0]->int_list_properties.value().insert(
+      std::make_pair(arc::mojom::AccessibilityIntListProperty::CHILD_NODE_IDS,
+                     std::vector<int>({1, 2})));
+
+  // Child button.
+  event1->node_data.push_back(arc::mojom::AccessibilityNodeInfoData::New());
+  event1->node_data[1]->id = 1;
+  event1->node_data[1]->string_properties =
+      std::unordered_map<arc::mojom::AccessibilityStringProperty,
+                         std::string>();
+  event1->node_data[1]->string_properties.value().insert(
+      std::make_pair(arc::mojom::AccessibilityStringProperty::CLASS_NAME,
+                     ui::kAXButtonClassname));
+  event1->node_data[1]->boolean_properties =
+      std::unordered_map<arc::mojom::AccessibilityBooleanProperty, bool>();
+  event1->node_data[1]->boolean_properties.value().insert(std::make_pair(
+      arc::mojom::AccessibilityBooleanProperty::VISIBLE_TO_USER, true));
+
+  // Another child button.
+  event1->node_data.push_back(arc::mojom::AccessibilityNodeInfoData::New());
+  event1->node_data[2]->id = 2;
+  event1->node_data[2]->string_properties =
+      std::unordered_map<arc::mojom::AccessibilityStringProperty,
+                         std::string>();
+  event1->node_data[2]->string_properties.value().insert(
+      std::make_pair(arc::mojom::AccessibilityStringProperty::CLASS_NAME,
+                     ui::kAXButtonClassname));
+  event1->node_data[2]->boolean_properties =
+      std::unordered_map<arc::mojom::AccessibilityBooleanProperty, bool>();
+  event1->node_data[2]->boolean_properties.value().insert(std::make_pair(
+      arc::mojom::AccessibilityBooleanProperty::VISIBLE_TO_USER, true));
+
+  // Populate the tree source with the data.
+  tree.NotifyAccessibilityEvent(event1.get());
+
+  // Live edit the data sources to exercise each layout.
+
+  // Non-overlapping, bottom to top.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(0, 0, 50, 50);
+  std::vector<mojom::AccessibilityNodeInfoData*> top_to_bottom;
+  tree.GetChildrenForTest(event1->node_data[0].get(), &top_to_bottom);
+  ASSERT_EQ(2U, top_to_bottom.size());
+  ASSERT_EQ(2, top_to_bottom[0]->id);
+  ASSERT_EQ(1, top_to_bottom[1]->id);
+
+  // Non-overlapping, top to bottom.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(0, 0, 50, 50);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  top_to_bottom.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &top_to_bottom);
+  ASSERT_EQ(2U, top_to_bottom.size());
+  ASSERT_EQ(1, top_to_bottom[0]->id);
+  ASSERT_EQ(2, top_to_bottom[1]->id);
+
+  // Overlapping; right to left.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(101, 100, 99, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  std::vector<mojom::AccessibilityNodeInfoData*> left_to_right;
+  tree.GetChildrenForTest(event1->node_data[0].get(), &left_to_right);
+  ASSERT_EQ(2U, left_to_right.size());
+  ASSERT_EQ(2, left_to_right[0]->id);
+  ASSERT_EQ(1, left_to_right[1]->id);
+
+  // Overlapping; left to right.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(101, 100, 99, 100);
+  left_to_right.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &left_to_right);
+  ASSERT_EQ(2U, left_to_right.size());
+  ASSERT_EQ(1, left_to_right[0]->id);
+  ASSERT_EQ(2, left_to_right[1]->id);
+
+  // Overlapping, bottom to top.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 99, 100, 100);
+  top_to_bottom.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &top_to_bottom);
+  ASSERT_EQ(2U, top_to_bottom.size());
+  ASSERT_EQ(2, top_to_bottom[0]->id);
+  ASSERT_EQ(1, top_to_bottom[1]->id);
+
+  // Overlapping, top to bottom.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 99, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  top_to_bottom.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &top_to_bottom);
+  ASSERT_EQ(2U, top_to_bottom.size());
+  ASSERT_EQ(1, top_to_bottom[0]->id);
+  ASSERT_EQ(2, top_to_bottom[1]->id);
+
+  // Identical. smaller to larger.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 10);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  std::vector<mojom::AccessibilityNodeInfoData*> dimension;
+  tree.GetChildrenForTest(event1->node_data[0].get(), &dimension);
+  ASSERT_EQ(2U, dimension.size());
+  ASSERT_EQ(2, dimension[0]->id);
+  ASSERT_EQ(1, dimension[1]->id);
+
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 10, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  dimension.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &dimension);
+  ASSERT_EQ(2U, dimension.size());
+  ASSERT_EQ(2, dimension[0]->id);
+  ASSERT_EQ(1, dimension[1]->id);
+
+  // Identical. Larger to smaller.
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 100, 10);
+  dimension.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &dimension);
+  ASSERT_EQ(2U, dimension.size());
+  ASSERT_EQ(1, dimension[0]->id);
+  ASSERT_EQ(2, dimension[1]->id);
+
+  event1->node_data[1]->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
+  event1->node_data[2]->bounds_in_screen = gfx::Rect(100, 100, 10, 100);
+  dimension.clear();
+  tree.GetChildrenForTest(event1->node_data[0].get(), &dimension);
+  ASSERT_EQ(2U, dimension.size());
+  ASSERT_EQ(1, dimension[0]->id);
+  ASSERT_EQ(2, dimension[1]->id);
+}
+
+}  // namespace arc