Invalidate index in parent cache upon child reordering

This CL detects when the children's order has changed
in a tree update in order to invalidate the unignored
index in parent cache. Before this change, the added
test case would fail and could cause an infinite
loop in the tree walker in the extension.

Bug: 1005799
Test: Added unit test and manual testing of display assistant
Change-Id: I902e74b0b5e25b949486bc9d7edc208cdd2aa846
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1813521Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Randy Rossi <rmrossi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698515}

ui/accessibility/ax_node.cc

@@ -66,6 +66,11 @@ size_t AXNode::GetUnignoredIndexInParent() const {
   return unignored_index_in_parent_;
 }
 
+size_t AXNode::GetIndexInParent() const {
+  DCHECK(!tree_->GetTreeUpdateInProgressState());
+  return index_in_parent_;
+}
+
 AXNode* AXNode::GetFirstUnignoredChild() const {
   DCHECK(!tree_->GetTreeUpdateInProgressState());
   return ComputeFirstUnignoredChildRecursive();

ui/accessibility/ax_node.h

@@ -114,6 +114,7 @@ class AX_EXPORT AXNode final {
   AXNode* GetUnignoredChildAtIndex(size_t index) const;
   AXNode* GetUnignoredParent() const;
   size_t GetUnignoredIndexInParent() const;
+  size_t GetIndexInParent() const;
   AXNode* GetFirstUnignoredChild() const;
   AXNode* GetLastUnignoredChild() const;
   AXNode* GetDeepestFirstUnignoredChild() const;

ui/accessibility/ax_tree.cc

@@ -1141,6 +1141,15 @@ bool AXTree::ComputePendingChanges(const AXTreeUpdate& update,
 bool AXTree::ComputePendingChangesToNode(const AXNodeData& new_data,
                                          bool is_new_root,
                                          AXTreeUpdateState* update_state) {
+  // Compare every child's index in parent in the update with the existing
+  // index in parent.  If the order has changed, invalidate the cached
+  // unignored index in parent.
+  for (size_t j = 0; j < new_data.child_ids.size(); j++) {
+    AXNode* node = GetFromId(new_data.child_ids[j]);
+    if (node && node->GetIndexInParent() != j)
+      update_state->InvalidateParentNodeUnignoredCacheValues(node->id());
+  }
+
   // If the node does not exist in the tree throw an error unless this
   // is the new root and it can be created.
   if (!update_state->ShouldPendingNodeExistInTree(new_data.id)) {

ui/accessibility/ax_tree_unittest.cc

@@ -651,6 +651,54 @@ TEST(AXTreeTest, ImplicitChildrenDelete) {
   EXPECT_EQ(tree.GetFromId(3), nullptr);
 }
 
+TEST(AXTreeTest, IndexInParentAfterReorder) {
+  // This test covers the case where an AXTreeUpdate includes
+  // reordered children.  The unignored index in parent
+  // values should be updated.
+
+  // Setup initial tree state.
+  // Tree:
+  //        1
+  //    2   3  4
+  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.resize(3);
+  initial_state.nodes[0].child_ids[0] = 2;
+  initial_state.nodes[0].child_ids[1] = 3;
+  initial_state.nodes[0].child_ids[2] = 4;
+  initial_state.nodes[1].id = 2;
+  initial_state.nodes[2].id = 3;
+  initial_state.nodes[3].id = 4;
+  AXTree tree(initial_state);
+
+  // Index in parent correct.
+  EXPECT_EQ(tree.GetFromId(2)->GetUnignoredIndexInParent(), (size_t)0);
+  EXPECT_EQ(tree.GetFromId(3)->GetUnignoredIndexInParent(), (size_t)1);
+  EXPECT_EQ(tree.GetFromId(4)->GetUnignoredIndexInParent(), (size_t)2);
+
+  // Perform an update where we reorder children to [ 4 3 2 ]
+  AXTreeUpdate update;
+  update.nodes.resize(4);
+  update.root_id = 1;
+  update.nodes[0].id = 1;
+  update.nodes[0].child_ids.resize(3);
+  update.nodes[0].child_ids[0] = 4;
+  update.nodes[0].child_ids[1] = 3;
+  update.nodes[0].child_ids[2] = 2;
+  update.nodes[1].id = 2;
+  update.nodes[2].id = 3;
+  update.nodes[3].id = 4;
+
+  ASSERT_TRUE(tree.Unserialize(update));
+
+  // Index in parent should have changed as well.
+  EXPECT_EQ((size_t)0, tree.GetFromId(4)->GetUnignoredIndexInParent());
+  EXPECT_EQ((size_t)1, tree.GetFromId(3)->GetUnignoredIndexInParent());
+  EXPECT_EQ((size_t)2, tree.GetFromId(2)->GetUnignoredIndexInParent());
+}
+
 TEST(AXTreeTest, ImplicitAttributeDelete) {
   // This test covers the case where an AXTreeUpdate includes a node without
   // mentioning one of that node's attributes, this should cause a delete of any