Commit 00f72818 authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

Fix bug in LayoutTreeBuilder accessibility patch

This change changed the accessibility tree to be built
using LayoutTreeBuilder: crrev.com/c/1547617

This caused crbug.com/951503 - a crash in
blink::AXNodeObject::AddChildren, due to a node
being deleted while it was in the process of
iterating over its children.

I can reliably reproduce this crash by loading
https://www.komputerswiat.pl/gamezilla when
accessibility is enabled.

I discovered that the root cause was due to the
change in AXObjectCacheImpl::GetOrCreate(LayoutObject*) -
specifically code that identifies an old entry in the
node mapping that needs to be updated.

The problem with the code is that it assumes there's a
1:1 mapping between nodes and layout objects - but this
isn't always true. When there's a continuation, you
could have two layout objects that correspond to the
same Node.

The fix is easy - just check node->GetLayoutObject
and skip checking the node mapping if it's not the
same.

Bug: 951503, 835455
Tbr: nektar@chromium.org, aboxhall@chromium.org
Change-Id: Ie5bc4fa5766f00bc8fe882454a5d15a1467f198c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1580140Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653498}
parent f7775ebb
......@@ -490,7 +490,7 @@ AXObject* AXObjectCacheImpl::GetOrCreate(LayoutObject* layout_object) {
layout_object_mapping_.Set(layout_object, axid);
new_obj->Init();
new_obj->SetLastKnownIsIgnoredValue(new_obj->AccessibilityIsIgnored());
if (node) {
if (node && node->GetLayoutObject() == layout_object) {
AXID prev_axid = node_object_mapping_.at(node);
if (prev_axid != 0 && prev_axid != axid) {
Remove(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