Fix crash in AXPlatformNodeBase::GetData

The crash for this bug is in ui::AXPlatformNodeBase::GetData, however
the actual issue is that the node is nullptr due to it being out of
range from the parent nodes children array. I traced this back to the
parent's cached unignored child count being incorrect.

This happens when node_id_to_clear is set - the node is cleared, but
the parent's unignored cached values are not cleared. This change simply
adds logic to add the parent node to the list of nodes to update
their unignored cache. This logic is identical to what happens when
the ignored state changes a few lines below.

Bug: 974435
Change-Id: I5392eff3cd31900739bff1f40a31edf7d84e9882
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1769109
Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690860}

ui/accessibility/ax_tree.cc

@@ -441,6 +441,18 @@ struct AXTreeUpdateState {
     return base::Contains(invalidate_unignored_cached_values_ids, node_id);
   }
 
+  // Adds the parent of |node_id| to the list of nodes to invalidate unignored
+  // cached values.
+  void InvalidateParentNodeUnignoredCacheValues(AXNode::AXID node_id) {
+    DCHECK(computing_pending_changes) << "This method should be called before "
+                                         "any updates are made to the tree.";
+    base::Optional<AXNode::AXID> parent_node_id =
+        GetParentIdForPendingNode(node_id);
+    if (parent_node_id) {
+      invalidate_unignored_cached_values_ids.insert(*parent_node_id);
+    }
+  }
+
   // Indicates if the tree is calculating what changes will occur during
   // an update before the update applies changes.
   bool computing_pending_changes;
@@ -1167,6 +1179,10 @@ bool AXTree::ComputePendingChangesToNode(const AXNodeData& new_data,
   if (update_state->DoesPendingNodeRequireInit(new_data.id)) {
     update_state->invalidate_unignored_cached_values_ids.insert(new_data.id);
 
+    // If this node has been cleared via |node_id_to_clear| or is a new node,
+    // the last-known parent's unignored cache needs to be updated.
+    update_state->InvalidateParentNodeUnignoredCacheValues(new_data.id);
+
     for (AXNode::AXID child_id : new_child_id_set) {
       // If a |child_id| is already pending for creation, then it must be a
       // duplicate entry in the tree.
@@ -1206,12 +1222,7 @@ bool AXTree::ComputePendingChangesToNode(const AXNodeData& new_data,
     update_state->invalidate_unignored_cached_values_ids.insert(new_data.id);
 
     // If this ignored state had changed also invalidate the parent.
-    base::Optional<AXNode::AXID> parent_node_id =
-        update_state->GetParentIdForPendingNode(new_data.id);
-    if (parent_node_id && ignored_changed) {
-      update_state->invalidate_unignored_cached_values_ids.insert(
-          *parent_node_id);
-    }
+    update_state->InvalidateParentNodeUnignoredCacheValues(new_data.id);
   }
 
   for (AXNode::AXID child_id : create_or_destroy_ids) {

ui/accessibility/ax_tree_unittest.cc

@@ -817,6 +817,35 @@ TEST(AXTreeTest, MultipleIgnoredChangesDoesNotBreakCache) {
   EXPECT_TRUE(tree.GetFromId(3)->data().HasState(ax::mojom::State::kIgnored));
 }
 
+TEST(AXTreeTest, NodeToClearUpdatesParentUnignoredCount) {
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(4);
+  initial_state.nodes[0].id = 1;
+  initial_state.nodes[0].child_ids.push_back(2);
+  initial_state.nodes[1].id = 2;
+  initial_state.nodes[1].AddState(ax::mojom::State::kIgnored);
+  initial_state.nodes[1].child_ids.push_back(3);
+  initial_state.nodes[1].child_ids.push_back(4);
+  initial_state.nodes[2].id = 3;
+  initial_state.nodes[3].id = 4;
+
+  AXTree tree(initial_state);
+  EXPECT_EQ(2u, tree.GetFromId(1)->GetUnignoredChildCount());
+  EXPECT_EQ(2u, tree.GetFromId(2)->GetUnignoredChildCount());
+
+  AXTreeUpdate update;
+  update.nodes.resize(1);
+  update.node_id_to_clear = 2;
+  update.root_id = 1;
+  update.nodes[0] = initial_state.nodes[1];
+  update.nodes[0].state = 0;
+  update.nodes[0].child_ids.resize(0);
+  EXPECT_TRUE(tree.Unserialize(update)) << tree.error();
+
+  EXPECT_EQ(1u, tree.GetFromId(1)->GetUnignoredChildCount());
+}
+
 TEST(AXTreeTest, TreeObserverIsNotCalledForReparenting) {
   AXTreeUpdate initial_state;
   initial_state.root_id = 1;
@@ -1634,6 +1663,7 @@ TEST(AXTreeTest, CachedUnignoredValues) {
   root = tree.root();
   EXPECT_EQ(2u, root->GetUnignoredChildCount());
   EXPECT_EQ(2, root->GetUnignoredChildAtIndex(0)->id());
+  EXPECT_EQ(2u, tree.GetFromId(2)->GetUnignoredChildCount());
   EXPECT_EQ(0u, tree.GetFromId(4)->GetUnignoredIndexInParent());
   EXPECT_EQ(1u, tree.GetFromId(5)->GetUnignoredIndexInParent());
 
@@ -1644,6 +1674,7 @@ TEST(AXTreeTest, CachedUnignoredValues) {
 
   EXPECT_TRUE(tree.Unserialize(update2));
 
+  EXPECT_EQ(1u, tree.GetFromId(2)->GetUnignoredChildCount());
   EXPECT_EQ(0u, tree.GetFromId(5)->GetUnignoredIndexInParent());
 
   // Ensure siblings of a deleted node are updated.
@@ -1654,6 +1685,7 @@ TEST(AXTreeTest, CachedUnignoredValues) {
 
   EXPECT_TRUE(tree.Unserialize(update3));
 
+  EXPECT_EQ(1u, tree.GetFromId(1)->GetUnignoredChildCount());
   EXPECT_EQ(0u, tree.GetFromId(3)->GetUnignoredIndexInParent());
 
   // Ensure new nodes are correctly updated.
@@ -1667,6 +1699,7 @@ TEST(AXTreeTest, CachedUnignoredValues) {
 
   EXPECT_TRUE(tree.Unserialize(update4));
 
+  EXPECT_EQ(2u, tree.GetFromId(1)->GetUnignoredChildCount());
   EXPECT_EQ(0u, tree.GetFromId(3)->GetUnignoredIndexInParent());
   EXPECT_EQ(1u, tree.GetFromId(6)->GetUnignoredIndexInParent());
   EXPECT_EQ(0u, tree.GetFromId(7)->GetUnignoredIndexInParent());