Ensure that tree updates from ARC++ always trigger re-computation of unignored state

In ARC++, we send the entire tree from inside the Android container out to
Chrome.

As a result, each time the AXTreeSerializer is asked to serialize, we rely upon
it to compute the incremental tree update starting at the event target. This can
exercise the serializer in various ways not seen on other platforms.

In order to ensure we always serve a fresh tree to Chrome OS extensions, that
listen on the other end of AXTreeSourceArc, set |node_id_to_clear| on each
AXTreeUpdate sent to the extension. This has the advantage of forcing
re-computation of unignored child count and index in parent which have been
buggy given corner cases of tree update permutations.

This change also makes a few cleanups/improvements:
- PopulateAXRole gets moved into ArcAccessibilityDataInfoWrapper::Serialize. This was done to ensure tests that only call Serialize get the *entire* serialized AXNodeData. Previously, we were omitting role population due to the special casing of the root node.
- removed logic surrounding assigning root node role from AXTreeSourceArc. This was done previously because the tree may or may not contain ancestor windows from ARC++ (i.e. AccessibilityWindowDataInfoWrappers were not being used). We can now safely assume we have windows in the tree. This change also relaxes the role assignment on the root *Node* (not window). We now compute its role as normal as any other node, but still assign it as modal. This is to cover strange cases in in tests where we have just one node in the node tree (one window with a child node). That node might be a staticText, but we were assigning it arbitrarily as a genericContainer. (see test change).

Bug: b:142544528
Test: unit_tests --gtest_filter=AXTreeSourceArc*.*
Change-Id: I9c14d9c7fdc10cfffd9ce734921276fd503eac33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856830
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706060}

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

@@ -88,7 +88,8 @@ void AccessibilityNodeInfoDataWrapper::PopulateAXRole(
 
   if (!GetProperty(AXBooleanProperty::IMPORTANCE) &&
       !HasProperty(AXStringProperty::TEXT) &&
-      !HasProperty(AXStringProperty::CONTENT_DESCRIPTION)) {
+      !HasProperty(AXStringProperty::CONTENT_DESCRIPTION) &&
+      !tree_source_->IsRootOfNodeTree(GetId())) {
     out_data->role = ax::mojom::Role::kIgnored;
     return;
   }
@@ -271,6 +272,8 @@ void AccessibilityNodeInfoDataWrapper::PopulateAXState(
 
 void AccessibilityNodeInfoDataWrapper::Serialize(
     ui::AXNodeData* out_data) const {
+  PopulateAXRole(out_data);
+
   // String properties.
   int labelled_by = -1;
 
@@ -381,11 +384,13 @@ void AccessibilityNodeInfoDataWrapper::Serialize(
   if (GetProperty(AXBooleanProperty::SELECTED)) {
     out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kSelected, true);
   }
-
   if (GetProperty(AXBooleanProperty::SUPPORTS_TEXT_LOCATION)) {
     out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kSupportsTextLocation,
                                true);
   }
+  if (tree_source_->IsRootOfNodeTree(GetId())) {
+    out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kModal, true);
+  }
 
   // Range info.
   if (node_ptr_->range_info) {

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

@@ -93,6 +93,8 @@ void AccessibilityWindowInfoDataWrapper::Serialize(
   if (!tree_source_->GetRoot())
     return;
 
+  PopulateAXRole(out_data);
+
   // String properties.
   std::string title;
   if (GetProperty(mojom::AccessibilityWindowStringProperty::TITLE, &title)) {

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

@@ -197,10 +197,12 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) {
   event.id = event_data->source_id;
 
   event_bundle.updates.emplace_back();
-  if (event_data->event_type == AXEventType::WINDOW_CONTENT_CHANGED) {
-    current_tree_serializer_->InvalidateSubtree(
-        GetFromId(event_data->source_id));
-  }
+
+  // Force the tree, starting at the target of the event, to update, so
+  // unignored fields get updated.
+  event_bundle.updates[0].node_id_to_clear = event_data->source_id;
+  current_tree_serializer_->InvalidateSubtree(GetFromId(event_data->source_id));
+
   current_tree_serializer_->SerializeChanges(GetFromId(event_data->source_id),
                                              &event_bundle.updates.back());
 
@@ -335,16 +337,6 @@ void AXTreeSourceArc::SerializeNode(ArcAccessibilityInfoData* info_data,
 
   int32_t id = info_data->GetId();
   out_data->id = id;
-  // If the node is the root, or if the node's parent is the root window,
-  // set a role of generic container.
-  if (info_data->IsNode() && id == root_id_) {
-    out_data->role = ax::mojom::Role::kRootWebArea;
-  } else if (info_data->IsNode() && parent_map_.at(id) == root_id_) {
-    out_data->role = ax::mojom::Role::kGenericContainer;
-    out_data->AddBoolAttribute(ax::mojom::BoolAttribute::kModal, true);
-  } else {
-    info_data->PopulateAXRole(out_data);
-  }
   info_data->Serialize(out_data);
 }
 
@@ -396,6 +388,23 @@ void AXTreeSourceArc::InvalidateTree() {
   current_tree_serializer_->Reset();
 }
 
+bool AXTreeSourceArc::IsRootOfNodeTree(int32_t id) const {
+  const auto& node_it = tree_map_.find(id);
+  if (node_it == tree_map_.end())
+    return false;
+
+  if (!node_it->second->IsNode())
+    return false;
+
+  const auto& parent_it = parent_map_.find(id);
+  if (parent_it == parent_map_.end())
+    return true;
+
+  const auto& parent_tree_it = tree_map_.find(parent_it->second);
+  CHECK(parent_tree_it != tree_map_.end());
+  return !parent_tree_it->second->IsNode();
+}
+
 gfx::Rect AXTreeSourceArc::ComputeEnclosingBounds(
     ArcAccessibilityInfoData* info_data) const {
   gfx::Rect computed_bounds;

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

@@ -85,6 +85,10 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   // Invalidates the tree serializer.
   void InvalidateTree();
 
+  // Returns true if the node id is the root of the node tree (which can have a
+  // parent window).
+  bool IsRootOfNodeTree(int32_t id) const;
+
   bool is_notification() { return is_notification_; }
 
   bool is_input_method_window() { return is_input_method_window_; }

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

@@ -12,6 +12,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/accessibility/ax_action_data.h"
 #include "ui/accessibility/ax_enums.mojom.h"
+#include "ui/accessibility/ax_tree.h"
 #include "ui/accessibility/platform/ax_android_constants.h"
 
 namespace arc {
@@ -102,11 +103,15 @@ class MockAutomationEventRouter
   MockAutomationEventRouter() {}
   ~MockAutomationEventRouter() override = default;
 
+  ui::AXTree* tree() { return &tree_; }
+
   void DispatchAccessibilityEvents(
       const ExtensionMsg_AccessibilityEventBundleParams& events) override {
-    for (auto&& event : events.events) {
+    for (auto&& event : events.events)
       event_count_[event.event_type]++;
-    }
+
+    for (const auto& update : events.updates)
+      tree_.Unserialize(update);
   }
 
   void DispatchAccessibilityLocationChange(
@@ -126,6 +131,7 @@ class MockAutomationEventRouter
       const base::Optional<gfx::Rect>& rect) override {}
 
   std::map<ax::mojom::Event, int> event_count_;
+  ui::AXTree tree_;
 };
 
 class AXTreeSourceArcTest : public testing::Test,
@@ -148,42 +154,42 @@ class AXTreeSourceArcTest : public testing::Test,
 
   AXTreeSourceArcTest()
       : router_(new MockAutomationEventRouter()),
-        tree_(new TestAXTreeSourceArc(this, router_.get())) {}
+        tree_source_(new TestAXTreeSourceArc(this, router_.get())) {}
 
  protected:
   void CallNotifyAccessibilityEvent(AXEventData* event_data) {
-    tree_->NotifyAccessibilityEvent(event_data);
+    tree_source_->NotifyAccessibilityEvent(event_data);
   }
 
   void CallGetChildren(
       AXNodeInfoData* node,
       std::vector<ArcAccessibilityInfoData*>* out_children) const {
-    AccessibilityNodeInfoDataWrapper node_data(tree_.get(), node);
-    tree_->GetChildren(&node_data, out_children);
+    AccessibilityNodeInfoDataWrapper node_data(tree_source_.get(), node);
+    tree_source_->GetChildren(&node_data, out_children);
   }
 
   void CallSerializeNode(AXNodeInfoData* node,
                          std::unique_ptr<ui::AXNodeData>* out_data) const {
     ASSERT_TRUE(out_data);
-    AccessibilityNodeInfoDataWrapper node_data(tree_.get(), node);
+    AccessibilityNodeInfoDataWrapper node_data(tree_source_.get(), node);
     *out_data = std::make_unique<ui::AXNodeData>();
-    tree_->SerializeNode(&node_data, out_data->get());
+    tree_source_->SerializeNode(&node_data, out_data->get());
   }
 
   void CallSerializeWindow(AXWindowInfoData* window,
                            std::unique_ptr<ui::AXNodeData>* out_data) const {
     ASSERT_TRUE(out_data);
-    AccessibilityWindowInfoDataWrapper window_data(tree_.get(), window);
+    AccessibilityWindowInfoDataWrapper window_data(tree_source_.get(), window);
     *out_data = std::make_unique<ui::AXNodeData>();
-    tree_->SerializeNode(&window_data, out_data->get());
+    tree_source_->SerializeNode(&window_data, out_data->get());
   }
 
   ArcAccessibilityInfoData* CallGetFromId(int32_t id) const {
-    return tree_->GetFromId(id);
+    return tree_source_->GetFromId(id);
   }
 
   bool CallGetTreeData(ui::AXTreeData* data) {
-    return tree_->GetTreeData(data);
+    return tree_source_->GetTreeData(data);
   }
 
   MockAutomationEventRouter* GetRouter() const { return router_.get(); }
@@ -192,11 +198,23 @@ class AXTreeSourceArcTest : public testing::Test,
     return router_->event_count_[type];
   }
 
+  ui::AXTree* tree() { return router_->tree(); }
+
+  void ExpectTree(const std::string& expected) {
+    const std::string& tree_text = tree()->ToString();
+    size_t first_new_line = tree_text.find("\n");
+    ASSERT_NE(std::string::npos, first_new_line);
+    ASSERT_GT(tree_text.size(), ++first_new_line);
+
+    // Omit the first line, which contains an unguessable ax tree id.
+    EXPECT_EQ(expected, tree_text.substr(first_new_line));
+  }
+
  private:
   void OnAction(const ui::AXActionData& data) const override {}
 
   const std::unique_ptr<MockAutomationEventRouter> router_;
-  const std::unique_ptr<AXTreeSourceArc> tree_;
+  const std::unique_ptr<AXTreeSourceArc> tree_source_;
 
   DISALLOW_COPY_AND_ASSIGN(AXTreeSourceArcTest);
 };
@@ -608,7 +626,7 @@ TEST_F(AXTreeSourceArcTest, AccessibleNameComputationWindowWithChildren) {
   ASSERT_TRUE(
       data->GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
   EXPECT_EQ("node text", name);
-  EXPECT_EQ(ax::mojom::Role::kGenericContainer, data->role);
+  EXPECT_EQ(ax::mojom::Role::kStaticText, data->role);
   EXPECT_TRUE(data->GetBoolAttribute(ax::mojom::BoolAttribute::kModal));
 
   CallSerializeNode(child_node, &data);
@@ -838,4 +856,69 @@ TEST_F(AXTreeSourceArcTest, EventTypeForViewSelected) {
   EXPECT_EQ(1, GetDispatchedEventCount(ax::mojom::Event::kValueChanged));
 }
 
+TEST_F(AXTreeSourceArcTest, SerializeAndUnserialize) {
+  auto event = AXEventData::New();
+  event->source_id = 10;
+  event->task_id = 1;
+  event->event_type = AXEventType::VIEW_FOCUSED;
+
+  event->window_data = std::vector<mojom::AccessibilityWindowInfoDataPtr>();
+  event->window_data->emplace_back(AXWindowInfoData::New());
+  AXWindowInfoData* root_window = event->window_data->back().get();
+  root_window->window_id = 100;
+  root_window->root_node_id = 10;
+
+  event->node_data.emplace_back(AXNodeInfoData::New());
+  AXNodeInfoData* root = event->node_data.back().get();
+  root->id = 10;
+  SetProperty(root, AXIntListProperty::CHILD_NODE_IDS, std::vector<int>({1}));
+
+  event->node_data.emplace_back(AXNodeInfoData::New());
+  AXNodeInfoData* node1 = event->node_data.back().get();
+  node1->id = 1;
+  SetProperty(node1, AXIntListProperty::CHILD_NODE_IDS, std::vector<int>({2}));
+
+  // An ignored node.
+  event->node_data.emplace_back(AXNodeInfoData::New());
+  AXNodeInfoData* node2 = event->node_data.back().get();
+  node2->id = 2;
+
+  // |node2| is ignored by default because
+  // AXBooleanProperty::IMPORTANCE has a default false value.
+
+  CallNotifyAccessibilityEvent(event.get());
+  EXPECT_EQ(1, GetDispatchedEventCount(ax::mojom::Event::kFocus));
+  ExpectTree(
+      "id=100 window (0, 0)-(0, 0) child_ids=10\n"
+      "  id=10 genericContainer INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "modal=true child_ids=1\n"
+      "    id=1 ignored INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "child_ids=2\n"
+      "      id=2 ignored INVISIBLE (0, 0)-(0, 0) restriction=disabled\n");
+
+  EXPECT_EQ(0U, tree()->GetFromId(10)->GetUnignoredChildCount());
+
+  // An unignored node.
+  event->node_data.emplace_back(AXNodeInfoData::New());
+  AXNodeInfoData* node3 = event->node_data.back().get();
+  node3->id = 3;
+  SetProperty(node3, AXStringProperty::CONTENT_DESCRIPTION, "some text");
+  SetProperty(node2, AXIntListProperty::CHILD_NODE_IDS, std::vector<int>({3}));
+
+  // |node3| is unignored since it has some text.
+
+  CallNotifyAccessibilityEvent(event.get());
+  ExpectTree(
+      "id=100 window (0, 0)-(0, 0) child_ids=10\n"
+      "  id=10 genericContainer INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "modal=true child_ids=1\n"
+      "    id=1 ignored INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "child_ids=2\n"
+      "      id=2 ignored INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "child_ids=3\n"
+      "        id=3 genericContainer INVISIBLE (0, 0)-(0, 0) "
+      "restriction=disabled name=some text \n");
+  EXPECT_EQ(1U, tree()->GetFromId(10)->GetUnignoredChildCount());
+}
+
 }  // namespace arc