Commit 4470d41d authored by Alice Boxhall's avatar Alice Boxhall Committed by Commit Bot

Fix crash caused by walking deleted children of node.

Example crash: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8906269450018283968/+/steps/content_browsertests__with_patch_/0/logs/Deterministic_failure:_DumpAccessibilityEventsTest.DeleteSubtree__x2f_linux__status_CRASH_/0

Fixed by avoiding computing bounds from child nodes during tree updates.

Change-Id: Ia2a293ee4e2c655fe3939ca76264efd54b3cf354
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731634Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683785}
parent 20f4fa8d
...@@ -554,6 +554,7 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, ...@@ -554,6 +554,7 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("live-region-elem-reparent.html")); RunEventTest(FILE_PATH_LITERAL("live-region-elem-reparent.html"));
} }
// TODO(aboxhall): Fix flakiness.
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsLiveRegionIgnoresClick) { AccessibilityEventsLiveRegionIgnoresClick) {
RunEventTest(FILE_PATH_LITERAL("live-region-ignores-click.html")); RunEventTest(FILE_PATH_LITERAL("live-region-ignores-click.html"));
...@@ -796,4 +797,8 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, ...@@ -796,4 +797,8 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("select-selected-add-remove.html")); RunEventTest(FILE_PATH_LITERAL("select-selected-add-remove.html"));
} }
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, DeleteSubtree) {
RunEventTest(FILE_PATH_LITERAL("delete-subtree.html"));
}
} // namespace content } // namespace content
CHILDREN-CHANGED index:0 CHILD:(role=ROLE_CHECK_BOX) role=ROLE_ARTICLE ENABLED,SENSITIVE,VISIBLE
CHILDREN-CHANGED index:0 CHILD:(role=ROLE_PUSH_BUTTON) role=ROLE_ARTICLE ENABLED,SENSITIVE,SHOWING,VISIBLE
CHILDREN-CHANGED index:1 CHILD:(role=ROLE_RADIO_BUTTON) role=ROLE_ARTICLE ENABLED,SENSITIVE,VISIBLE
STATE-CHANGE:DEFUNCT:TRUE role=ROLE_INVALID name='(null)' DEFUNCT
STATE-CHANGE:DEFUNCT:TRUE role=ROLE_INVALID name='(null)' DEFUNCT
<!--
@BLINK-ALLOW:offscreen
@BLINK-ALLOW:size=(0, 0)
@BLINK-ALLOW:clipsChildren*
@BLINK-ALLOW:unclippedSize=(50, 20)
@BLINK-ALLOW:pageSize=(50, 20)
@BLINK-ALLOW:pageSize=(1, 1)
@BLINK-ALLOW:pageSize=(50, 1)
-->
<!DOCTYPE html>
<div id="parent" aria-label="parent" role="article"
style="position: absolute; width: 0; height: 0; overflow: visible">
<div style="width:50px; height: 20px" aria-label="first child" role="checkbox">first child</div>
<div style="width:50px; height: 20px" aria-label="second child" role="radio">second child</div>
</div>
<script>
function go() {
document.getElementById("parent").innerHTML = "<div role=button>new child</div>";
}
</script>
...@@ -233,7 +233,9 @@ gfx::RectF AXTree::RelativeToTreeBounds(const AXNode* node, ...@@ -233,7 +233,9 @@ gfx::RectF AXTree::RelativeToTreeBounds(const AXNode* node,
// If the node bounds is empty (either width or height is zero), // If the node bounds is empty (either width or height is zero),
// try to compute good bounds from the children. // try to compute good bounds from the children.
if (bounds.IsEmpty()) { // If a tree update is in progress, skip this step as children may be in a
// bad state.
if (bounds.IsEmpty() && !GetTreeUpdateInProgressState()) {
for (size_t i = 0; i < node->children().size(); i++) { for (size_t i = 0; i < node->children().size(); i++) {
ui::AXNode* child = node->children()[i]; ui::AXNode* child = node->children()[i];
bounds.Union(GetTreeBounds(child)); bounds.Union(GetTreeBounds(child));
......
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