Commit 748888c0 authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

Remove assumption that the parent AX tree is the desktop.

Currently we have a single root accessibility tree (the "desktop"
tree) containing all aura Windows and views in the browser process,
and all WebContents accessibility trees are direct children of that
tree.

Under multi-process mash, we're going to want it to be possible for a
remote app to host a WebContents, so we need to remove the assumption
that the parent accessibility tree of a WebContents must be the root
accessibility tree.

To do this, modify AutomationInternalCustomBindings::GetParent
so that it checks for any tree that lists the current tree as
its child tree, rather than only trying the desktop tree.
To make this efficient, have AutomationAXTreeWrapper keep track
of a map from child tree ID to its host.

This is reasonably well-covered by existing tests - if GetParent
doesn't work correctly, nearly all automation tests break.

Bug: 888147
Change-Id: Ib7350d51dffed345dd03ec11e6cc0b7591bb243c
Reviewed-on: https://chromium-review.googlesource.com/c/1286903
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604729}
parent 09c286ca
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/no_destructor.h"
#include "chrome/common/extensions/chrome_extension_messages.h" #include "chrome/common/extensions/chrome_extension_messages.h"
#include "chrome/renderer/extensions/automation_internal_custom_bindings.h" #include "chrome/renderer/extensions/automation_internal_custom_bindings.h"
#include "ui/accessibility/ax_node.h" #include "ui/accessibility/ax_node.h"
...@@ -10,6 +11,12 @@ namespace extensions { ...@@ -10,6 +11,12 @@ namespace extensions {
namespace { namespace {
std::map<ui::AXTreeID, AutomationAXTreeWrapper*>& GetChildTreeIDReverseMap() {
static base::NoDestructor<std::map<ui::AXTreeID, AutomationAXTreeWrapper*>>
child_tree_id_reverse_map;
return *child_tree_id_reverse_map;
}
// Convert from ax::mojom::Event to api::automation::EventType. // Convert from ax::mojom::Event to api::automation::EventType.
api::automation::EventType ToAutomationEvent(ax::mojom::Event event_type) { api::automation::EventType ToAutomationEvent(ax::mojom::Event event_type) {
switch (event_type) { switch (event_type) {
...@@ -204,9 +211,28 @@ AutomationAXTreeWrapper::~AutomationAXTreeWrapper() { ...@@ -204,9 +211,28 @@ AutomationAXTreeWrapper::~AutomationAXTreeWrapper() {
tree_.SetDelegate(nullptr); tree_.SetDelegate(nullptr);
} }
// static
AutomationAXTreeWrapper* AutomationAXTreeWrapper::GetParentOfTreeId(
ui::AXTreeID tree_id) {
std::map<ui::AXTreeID, AutomationAXTreeWrapper*>& child_tree_id_reverse_map =
GetChildTreeIDReverseMap();
const auto& iter = child_tree_id_reverse_map.find(tree_id);
if (iter != child_tree_id_reverse_map.end())
return iter->second;
return nullptr;
}
bool AutomationAXTreeWrapper::OnAccessibilityEvents( bool AutomationAXTreeWrapper::OnAccessibilityEvents(
const ExtensionMsg_AccessibilityEventBundleParams& event_bundle, const ExtensionMsg_AccessibilityEventBundleParams& event_bundle,
bool is_active_profile) { bool is_active_profile) {
std::map<ui::AXTreeID, AutomationAXTreeWrapper*>& child_tree_id_reverse_map =
GetChildTreeIDReverseMap();
for (const ui::AXTreeID& tree_id : tree_.GetAllChildTreeIds()) {
DCHECK_EQ(child_tree_id_reverse_map[tree_id], this);
child_tree_id_reverse_map.erase(tree_id);
}
for (const auto& update : event_bundle.updates) { for (const auto& update : event_bundle.updates) {
set_event_from(update.event_from); set_event_from(update.event_from);
deleted_node_ids_.clear(); deleted_node_ids_.clear();
...@@ -229,6 +255,11 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents( ...@@ -229,6 +255,11 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents(
} }
} }
for (const ui::AXTreeID& tree_id : tree_.GetAllChildTreeIds()) {
DCHECK(!base::ContainsKey(child_tree_id_reverse_map, tree_id));
child_tree_id_reverse_map.insert(std::make_pair(tree_id, this));
}
// Exit early if this isn't the active profile. // Exit early if this isn't the active profile.
if (!is_active_profile) if (!is_active_profile)
return true; return true;
......
...@@ -22,6 +22,10 @@ class AutomationAXTreeWrapper : public ui::AXEventGenerator { ...@@ -22,6 +22,10 @@ class AutomationAXTreeWrapper : public ui::AXEventGenerator {
AutomationInternalCustomBindings* owner); AutomationInternalCustomBindings* owner);
~AutomationAXTreeWrapper() override; ~AutomationAXTreeWrapper() override;
// Returns the AutomationAXTreeWrapper that lists |tree_id| as one of its
// child trees, if any.
static AutomationAXTreeWrapper* GetParentOfTreeId(ui::AXTreeID tree_id);
ui::AXTreeID tree_id() const { return tree_id_; } ui::AXTreeID tree_id() const { return tree_id_; }
ui::AXTree* tree() { return &tree_; } ui::AXTree* tree() { return &tree_; }
AutomationInternalCustomBindings* owner() { return owner_; } AutomationInternalCustomBindings* owner() { return owner_; }
......
...@@ -1374,17 +1374,22 @@ ui::AXNode* AutomationInternalCustomBindings::GetParent( ...@@ -1374,17 +1374,22 @@ ui::AXNode* AutomationInternalCustomBindings::GetParent(
if (node->parent()) if (node->parent())
return node->parent(); return node->parent();
AutomationAXTreeWrapper* parent_tree_wrapper = nullptr;
ui::AXTreeID parent_tree_id = ui::AXTreeID parent_tree_id =
(*in_out_tree_wrapper)->tree()->data().parent_tree_id; (*in_out_tree_wrapper)->tree()->data().parent_tree_id;
if (parent_tree_id != ui::AXTreeIDUnknown()) {
// If the tree specifies its parent tree ID, use that. That provides
// some additional security guarantees, so a tree can't be "claimed"
// by something else.
parent_tree_wrapper = GetAutomationAXTreeWrapperFromTreeID(parent_tree_id);
} else {
// Otherwise if it was unspecified, check to see if another tree listed
// this one as its child, and then we know the parent.
parent_tree_wrapper = AutomationAXTreeWrapper::GetParentOfTreeId(
(*in_out_tree_wrapper)->tree_id());
}
// Try the desktop tree if the parent is unknown. If this tree really is
// a child of the desktop tree, we'll find its parent, and if not, the
// search, below, will fail until the real parent tree loads.
if (parent_tree_id == ui::AXTreeIDUnknown())
parent_tree_id = ui::DesktopAXTreeID();
AutomationAXTreeWrapper* parent_tree_wrapper =
GetAutomationAXTreeWrapperFromTreeID(parent_tree_id);
if (!parent_tree_wrapper) if (!parent_tree_wrapper)
return nullptr; return nullptr;
......
...@@ -333,6 +333,13 @@ std::set<int32_t> AXTree::GetNodeIdsForChildTreeId( ...@@ -333,6 +333,13 @@ std::set<int32_t> AXTree::GetNodeIdsForChildTreeId(
return std::set<int32_t>(); return std::set<int32_t>();
} }
const std::set<AXTreeID> AXTree::GetAllChildTreeIds() const {
std::set<AXTreeID> result;
for (auto entry : child_tree_id_reverse_map_)
result.insert(entry.first);
return result;
}
bool AXTree::Unserialize(const AXTreeUpdate& update) { bool AXTree::Unserialize(const AXTreeUpdate& update) {
AXTreeUpdateState update_state; AXTreeUpdateState update_state;
int32_t old_root_id = root_ ? root_->id() : 0; int32_t old_root_id = root_ ? root_->id() : 0;
......
...@@ -227,6 +227,9 @@ class AX_EXPORT AXTree : public AXNode::OwnerTree { ...@@ -227,6 +227,9 @@ class AX_EXPORT AXTree : public AXNode::OwnerTree {
// have a kChildTreeId int attribute with that value. // have a kChildTreeId int attribute with that value.
std::set<int32_t> GetNodeIdsForChildTreeId(AXTreeID child_tree_id) const; std::set<int32_t> GetNodeIdsForChildTreeId(AXTreeID child_tree_id) const;
// Get all of the child tree IDs referenced by any node in this tree.
const std::set<AXTreeID> GetAllChildTreeIds() const;
// Map from a relation attribute to a map from a target id to source ids. // Map from a relation attribute to a map from a target id to source ids.
const IntReverseRelationMap& int_reverse_relations() { const IntReverseRelationMap& int_reverse_relations() {
return int_reverse_relations_; return int_reverse_relations_;
......
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