Commit 0d25be9b authored by Aaron Leventhal's avatar Aaron Leventhal Committed by Chromium LUCI CQ

Speculative fix: iframe hypertext returns null hyperlink at index 0

An iframe can only have 1 hyperlink. If the iframe is not loaded yet,
then it should have 0.

When hyperlink 0 is retrieved, it should never be null. However, this
is occurring where the url is "empty". It may be occurring during
document destruction.

There exists code to update the parent hypertext when a
BrowserAccessibilityManager's contents are attached, but not on
destruction. It's unclear whether a single BrowserAccessibilityManager
can be destroyed without the entire tree of them being destroyed.
To be safe, update the parent when a BrowserAccessibilityManager
is destroyed.

Bug: 1157731
Change-Id: I96a6654d3aa2d6934c07a0145e76edad9ee93243
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611665Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840662}
parent feee9990
...@@ -487,6 +487,11 @@ IFACEMETHODIMP BrowserAccessibilityComWin::get_nHyperlinks( ...@@ -487,6 +487,11 @@ IFACEMETHODIMP BrowserAccessibilityComWin::get_nHyperlinks(
return E_INVALIDARG; return E_INVALIDARG;
*hyperlink_count = hypertext_.hyperlink_offset_to_index.size(); *hyperlink_count = hypertext_.hyperlink_offset_to_index.size();
DCHECK(!ui::IsIframe(owner()->GetRole()) || *hyperlink_count <= 1)
<< "iframes should have 1 hyperlink, unless the child document is "
"destroyed/unloaded, in which case it should have 0";
return S_OK; return S_OK;
} }
...@@ -503,6 +508,9 @@ IFACEMETHODIMP BrowserAccessibilityComWin::get_hyperlink( ...@@ -503,6 +508,9 @@ IFACEMETHODIMP BrowserAccessibilityComWin::get_hyperlink(
return E_INVALIDARG; return E_INVALIDARG;
} }
DCHECK(!ui::IsIframe(owner()->GetRole()) || index == 0)
<< "An iframe cannot have more than 1 hyperlink";
int32_t id = hypertext_.hyperlinks[index]; int32_t id = hypertext_.hyperlinks[index];
AXPlatformNode* node = AXPlatformNodeWin::GetFromUniqueId(id); AXPlatformNode* node = AXPlatformNodeWin::GetFromUniqueId(id);
if (!node) { if (!node) {
......
...@@ -188,6 +188,12 @@ BrowserAccessibilityManager::BrowserAccessibilityManager( ...@@ -188,6 +188,12 @@ BrowserAccessibilityManager::BrowserAccessibilityManager(
} }
BrowserAccessibilityManager::~BrowserAccessibilityManager() { BrowserAccessibilityManager::~BrowserAccessibilityManager() {
// If the root's parent is in another accessibility tree but it wasn't
// previously connected, post the proper notifications on the parent.
BrowserAccessibility* parent = nullptr;
if (connected_to_parent_tree_node_)
parent = GetParentNodeFromParentTree();
// Fire any events that need to be fired when tree nodes get deleted. For // Fire any events that need to be fired when tree nodes get deleted. For
// example, events that fire every time "OnSubtreeWillBeDeleted" is called. // example, events that fire every time "OnSubtreeWillBeDeleted" is called.
ax_tree()->Destroy(); ax_tree()->Destroy();
...@@ -198,6 +204,8 @@ BrowserAccessibilityManager::~BrowserAccessibilityManager() { ...@@ -198,6 +204,8 @@ BrowserAccessibilityManager::~BrowserAccessibilityManager() {
} }
ui::AXTreeManagerMap::GetInstance().RemoveTreeManager(ax_tree_id_); ui::AXTreeManagerMap::GetInstance().RemoveTreeManager(ax_tree_id_);
ParentConnectionChanged(parent);
} }
bool BrowserAccessibilityManager::Unserialize( bool BrowserAccessibilityManager::Unserialize(
...@@ -343,6 +351,17 @@ BrowserAccessibility* BrowserAccessibilityManager::GetParentNodeFromParentTree() ...@@ -343,6 +351,17 @@ BrowserAccessibility* BrowserAccessibilityManager::GetParentNodeFromParentTree()
: nullptr; : nullptr;
} }
void BrowserAccessibilityManager::ParentConnectionChanged(
BrowserAccessibility* parent) {
if (!parent)
return;
parent->OnDataChanged();
parent->UpdatePlatformAttributes();
parent =
RetargetForEvents(parent, RetargetEventType::RetargetEventTypeGenerated);
FireGeneratedEvent(ui::AXEventGenerator::Event::CHILDREN_CHANGED, parent);
}
BrowserAccessibility* BrowserAccessibilityManager::GetPopupRoot() const { BrowserAccessibility* BrowserAccessibilityManager::GetPopupRoot() const {
DCHECK(popup_root_ids_.size() <= 1); DCHECK(popup_root_ids_.size() <= 1);
if (popup_root_ids_.size() == 1) { if (popup_root_ids_.size() == 1) {
...@@ -454,11 +473,7 @@ bool BrowserAccessibilityManager::OnAccessibilityEvents( ...@@ -454,11 +473,7 @@ bool BrowserAccessibilityManager::OnAccessibilityEvents(
BrowserAccessibility* parent = GetParentNodeFromParentTree(); BrowserAccessibility* parent = GetParentNodeFromParentTree();
if (parent) { if (parent) {
if (!connected_to_parent_tree_node_) { if (!connected_to_parent_tree_node_) {
parent->OnDataChanged(); ParentConnectionChanged(parent);
parent->UpdatePlatformAttributes();
parent = RetargetForEvents(parent,
RetargetEventType::RetargetEventTypeGenerated);
FireGeneratedEvent(ui::AXEventGenerator::Event::CHILDREN_CHANGED, parent);
connected_to_parent_tree_node_ = true; connected_to_parent_tree_node_ = true;
} }
} else { } else {
......
...@@ -196,6 +196,8 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver, ...@@ -196,6 +196,8 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver,
// If this tree has a parent tree, return the parent node in that tree. // If this tree has a parent tree, return the parent node in that tree.
BrowserAccessibility* GetParentNodeFromParentTree() const; BrowserAccessibility* GetParentNodeFromParentTree() const;
void ParentConnectionChanged(BrowserAccessibility* parent);
// In general, there is only a single node with the role of kRootWebArea, // In general, there is only a single node with the role of kRootWebArea,
// but if a popup is opened, a second nested "root" is created in the same // but if a popup is opened, a second nested "root" is created in the same
// tree as the "true" root. This will keep track of the nested root node. // tree as the "true" root. This will keep track of the nested root node.
......
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