Skip to content

Commit

Permalink
When an ancestor suddenly becomes a leaf, prune descendants
Browse files Browse the repository at this point in the history
Otherwise, reparenting errors occur when the descendants are
accessed, as it is impossible to have a leaf object with children.

Bug: 1205275
Change-Id: Iff94622994840bbe77d28ac67ef234250945602e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2876448
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#880122}
  • Loading branch information
aleventhal authored and Chromium LUCI CQ committed May 6, 2021
1 parent 35fd1f3 commit db50d61
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2947,6 +2947,10 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, IgnoredCrash) {
RunRegressionTest(FILE_PATH_LITERAL("ignored-crash.html"));
}

IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, MissingParent) {
RunRegressionTest(FILE_PATH_LITERAL("missing-parent.html"));
}

IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, RemovePseudoContent) {
RunRegressionTest(FILE_PATH_LITERAL("remove-pseudo-content.html"));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rootWebArea name='done'
++genericContainer ignored
++++genericContainer
++++++image
32 changes: 32 additions & 0 deletions content/test/data/accessibility/regression/missing-parent.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<!--
@WAIT-FOR:done
-->
<!-- Applying the class "strange" turns <col> into a LayoutImage, which
cannot have children, yet a preexisting descendants exists that tries to
find its parent, triggering a DCHECK -->
<style>
.strange { text-align: center; content: url(data:image/png,aaa); }
</style>

<foo></foo>

<script>
const col = document.createElement('col');
const span = document.createElement('span');
const bar = document.createElement('bar');
const baz = document.createElement('baz');

// Create structure manually as parser will refuse:
// <foo><col><span><bar>
document.querySelector('foo').appendChild(col);
col.appendChild(span);
span.appendChild(bar);
bar.appendChild(baz);

document.addEventListener('DOMContentLoaded', () => {
setTimeout(() => {
document.querySelector('foo').className = 'strange';
document.title = 'done';
}, 50);
});
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -1148,11 +1148,16 @@ AXObject* AXObjectCacheImpl::CreateAndInit(LayoutObject* layout_object,
// Example: parent calls Init() => ComputeAccessibilityIsIgnored() =>
// CanSetFocusAttribute() => CanBeActiveDescendant() =>
// IsARIAControlledByTextboxWithActiveDescendant() => GetOrCreate().
if (layout_object_mapping_.at(layout_object))
return Get(layout_object);
if (layout_object_mapping_.at(layout_object)) {
AXObject* result = Get(layout_object);
DCHECK(result) << "Missing cached AXObject for " << layout_object;
return result;
}

AXObject* new_obj = CreateFromRenderer(layout_object);

DCHECK(new_obj) << "Could not create AXObject for " << layout_object;

// Will crash later if we have two objects for the same layoutObject.
DCHECK(!layout_object_mapping_.at(layout_object))
<< "Already have an AXObject for " << layout_object;
Expand Down Expand Up @@ -1738,6 +1743,41 @@ void AXObjectCacheImpl::UpdateCacheAfterNodeIsAttached(Node* node) {
&AXObjectCacheImpl::UpdateCacheAfterNodeIsAttachedWithCleanLayout, node);
}

bool AXObjectCacheImpl::IsStillInTree(AXObject* obj) {
// Return an AXObject for the node if the AXObject is still in the tree.
// If there is a viable included parent, that means it's still in the tree.
// Otherwise, repair missing parent, or prune the object if no viable parent
// can be found. For example, through CSS changes, an ancestor became an
// image, which is always a leaf; therefore, no descendants are "in the tree".

if (!obj)
return false;

if (obj->IsMissingParent()) {
// Parent is missing. Attempt to repair it with a viable recomputed parent.
AXObject* ax_parent = obj->ComputeParent();
if (!IsStillInTree(ax_parent)) {
// Parent is unrepairable, meaning that this AXObject can no longer be
// attached to the tree and is no longer viable. Prune it now.
Remove(obj);
return false;
}
obj->SetParent(ax_parent);
return true;
}

if (!obj->LastKnownIsIncludedInTreeValue()) {
// Current object was not included in the tree, therefore, recursively
// keep checking up a level until a viable included parent is found.
if (!IsStillInTree(obj->CachedParentObject())) {
Remove(obj);
return false;
}
}

return true;
}

void AXObjectCacheImpl::UpdateCacheAfterNodeIsAttachedWithCleanLayout(
Node* node) {
if (!node || !node->isConnected())
Expand Down Expand Up @@ -1915,8 +1955,11 @@ void AXObjectCacheImpl::ChildrenChangedWithCleanLayout(Node* optional_node,
<< "Unclean document at lifecycle " << document->Lifecycle().ToString();
#endif // DCHECK_IS_ON()

if (obj)
if (obj) {
if (!IsStillInTree(obj))
return; // Object is no longer in tree, and therefore not viable.
obj->ChildrenChanged();
}

if (optional_node)
relation_cache_->UpdateRelatedTree(optional_node, obj);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ class MODULES_EXPORT AXObjectCacheImpl
AXObject* Get(const Node*);
AXObject* Get(const LayoutObject*);

// Return true if the object is still part of the tree, meaning that ancestors
// exist or can be repaired all the way to the root.
bool IsStillInTree(AXObject*);

AXObject* FirstAccessibleObjectFromNode(const Node*);

void ChildrenChangedWithCleanLayout(Node* optional_node_for_relation_update,
Expand Down

0 comments on commit db50d61

Please sign in to comment.