Track all changed nodes during an update

ui/accessibility/ax_tree.cc

 // Copyright 2013 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 "ui/accessibility/ax_tree.h"
 
 #include <stddef.h>
 
 #include <set>
 
@@ skipping 18 matching lines @@
 // Intermediate state to keep track of during a tree update.
 struct AXTreeUpdateState {
   AXTreeUpdateState() : new_root(nullptr) {}
 
   // During an update, this keeps track of all nodes that have been
   // implicitly referenced as part of this update, but haven't been
   // updated yet. It's an error if there are any pending nodes at the
   // end of Unserialize.
   std::set<AXNode*> pending_nodes;
 
+  // This is similar to above, but we store node ids here because this list gets
+  // generated before any nodes get created or re-used. Its purpose is to allow
+  // us to know what nodes will be updated so we can make more intelligent
+  // decisions about when to notify delegates of removals or reparenting.
+  std::set<int> changed_node_ids;
+
   // Keeps track of new nodes created during this update.
   std::set<AXNode*> new_nodes;
 
   // The new root in this update, if any.
   AXNode* new_root;
 
   // Keeps track of any nodes removed. Used to identify re-parented nodes.
   std::set<int> removed_node_ids;
 };
 
@@ skipping 36 matching lines @@
 void AXTree::UpdateData(const AXTreeData& data) {
   data_ = data;
   if (delegate_)
     delegate_->OnTreeDataChanged(this);
 }
 
 bool AXTree::Unserialize(const AXTreeUpdate& update) {
   AXTreeUpdateState update_state;
   int32_t old_root_id = root_ ? root_->id() : 0;
 
+  // First, make a note of any nodes we will touch as part of this update
+  // including implicitly via parent/child relationships.
+  for (size_t i = 0; i < update.nodes.size(); ++i) {
+    update_state.changed_node_ids.insert(update.nodes[i].id);
+    for (size_t j = 0; j < update.nodes[i].child_ids.size(); ++j)
+      update_state.changed_node_ids.insert(update.nodes[i].child_ids[j]);
+  }
+
   if (update.has_tree_data)
     UpdateData(update.tree_data);
 
   if (update.node_id_to_clear != 0) {
     AXNode* node = GetFromId(update.node_id_to_clear);
     if (!node) {
       error_ = base::StringPrintf("Bad node_id_to_clear: %d",
                                   update.node_id_to_clear);
       return false;
     }
@@ skipping 135 matching lines @@
     root_ = node;
     if (old_root)
       DestroySubtree(old_root, update_state);
   }
 
   return success;
 }
 
 void AXTree::DestroySubtree(AXNode* node,
                             AXTreeUpdateState* update_state) {
-  if (delegate_)
+  if (delegate_ && (!update_state ||

[dmazzoni, 2016/09/06 22:49:44]

I don't think we support |update_state| being nullptr anywhere.
It's only a pointer because it's modifiable and because its
lifetime is less than the lifetime of the AXTree object.
I'd just make it a DCHECK instead.

[David Tseng, 2016/09/07 16:37:01]

Alright, done, DestroySubtree only gets called from Unserialize, so this appears
to be right.


Btw, are you ok with not notifying the delegate of a subtree deletion when the
AXTree is destroyed? I think that might be the last (hopefully) remaining corner
case.

+                    update_state->changed_node_ids.find(node->id()) ==
+                        update_state->changed_node_ids.end()))
     delegate_->OnSubtreeWillBeDeleted(this, node);
   DestroyNodeAndSubtree(node, update_state);
 }
 
 void AXTree::DestroyNodeAndSubtree(AXNode* node,
                                    AXTreeUpdateState* update_state) {
-  if (delegate_)
+  if (delegate_ && (!update_state ||
+                    update_state->changed_node_ids.find(node->id()) ==
+                        update_state->changed_node_ids.end()))
     delegate_->OnNodeWillBeDeleted(this, node);
   id_map_.erase(node->id());
   for (int i = 0; i < node->child_count(); ++i)
     DestroyNodeAndSubtree(node->ChildAtIndex(i), update_state);
   if (update_state) {
     update_state->pending_nodes.erase(node);
     update_state->removed_node_ids.insert(node->id());
   }
   node->Destroy();
 }
@@ skipping 52 matching lines @@
       update_state->pending_nodes.insert(child);
       update_state->new_nodes.insert(child);
     }
     new_children->push_back(child);
   }
 
   return success;
 }
 
 }  // namespace ui