Commit e5ab7cc7 authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Commit Bot

Make AXTreeSerializer safer

The tree serializer should not crash even if the given tree has invalid
nodes. These changes don't alter behavior but removed possibilities for
accessing dangling pointers.

Bug: 981311
Change-Id: I8f2e55771c12a03955f7632335bea347b6a5af30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1707949
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678670}
parent 4419fda3
...@@ -462,12 +462,14 @@ void AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>:: ...@@ -462,12 +462,14 @@ void AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
template <typename AXSourceNode, typename AXNodeData, typename AXTreeData> template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
void AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>:: void AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
DeleteClientSubtree(ClientTreeNode* client_node) { DeleteClientSubtree(ClientTreeNode* client_node) {
for (size_t i = 0; i < client_node->children.size(); ++i) { if (client_node == client_root_) {
client_id_map_.erase(client_node->children[i]->id); Reset(); // Do not try to reuse a bad root later.
DeleteClientSubtree(client_node->children[i]); return;
delete client_node->children[i];
} }
client_node->children.clear(); for (size_t i = 0; i < client_node->children.size(); ++i)
DeleteClientSubtree(client_node->children[i]);
client_id_map_.erase(client_node->id);
delete client_node;
} }
template <typename AXSourceNode, typename AXNodeData, typename AXTreeData> template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
...@@ -531,7 +533,8 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>:: ...@@ -531,7 +533,8 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
// There shouldn't be any reparenting because we've already handled it // There shouldn't be any reparenting because we've already handled it
// above. If this happens, reset and return an error. // above. If this happens, reset and return an error.
ClientTreeNode* client_child = client_id_map_[new_child_id];
ClientTreeNode* client_child = ClientTreeNodeById(new_child_id);
if (client_child && client_child->parent != client_node) { if (client_child && client_child->parent != client_node) {
DVLOG(1) << "Reparenting detected"; DVLOG(1) << "Reparenting detected";
Reset(); Reset();
...@@ -552,9 +555,7 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>:: ...@@ -552,9 +555,7 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
ClientTreeNode* old_child = old_children[i]; ClientTreeNode* old_child = old_children[i];
int old_child_id = old_child->id; int old_child_id = old_child->id;
if (new_child_ids.find(old_child_id) == new_child_ids.end()) { if (new_child_ids.find(old_child_id) == new_child_ids.end()) {
client_id_map_.erase(old_child_id);
DeleteClientSubtree(old_child); DeleteClientSubtree(old_child);
delete old_child;
} else { } else {
client_child_id_map[old_child_id] = old_child; client_child_id_map[old_child_id] = old_child;
} }
...@@ -593,8 +594,10 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>:: ...@@ -593,8 +594,10 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
new_child_ids.erase(child_id); new_child_ids.erase(child_id);
actual_serialized_node_child_ids.push_back(child_id); actual_serialized_node_child_ids.push_back(child_id);
if (client_child_id_map.find(child_id) != client_child_id_map.end()) { ClientTreeNode* reused_child = nullptr;
ClientTreeNode* reused_child = client_child_id_map[child_id]; if (client_child_id_map.find(child_id) != client_child_id_map.end())
reused_child = ClientTreeNodeById(child_id);
if (reused_child) {
client_node->children.push_back(reused_child); client_node->children.push_back(reused_child);
const bool ignored_state_changed = const bool ignored_state_changed =
reused_child->ignored != reused_child->ignored !=
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment