Commit 4a337b92 authored by Benjamin Beaudry's avatar Benjamin Beaudry Committed by Chromium LUCI CQ

Fix crash caused by invalid position in windows TextRangeProvider

Users of Narrator experience a consistent crash caused by an endpoint
in AXPlatformNodeTextRangeProvider being on a position that isn't null
but had no associated anchor. This position is likely a product of the
implementation of TextRangeEndpoints::OnNodeWillBeDeleted, where we
move the position to its parent that might have been deleted previously.

The crash was caused by an observer not being removed. It was not
removed because of the use of !IsNullPosition(), in which we return true
if GetAnchor() returns a nullptr.

Eventually, when bug:1152939 is fixed at the source, it will be safe to
go back with IsNullPosition().

Bug: 1152939
Change-Id: I633d403d8182adca807fde61563a343ce7ee78e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561648
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: default avatarDaniel Libby <dlibby@microsoft.com>
Reviewed-by: default avatarIan Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#831993}
parent 3b8d2259
......@@ -1462,7 +1462,10 @@ AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::~TextRangeEndpoints() {
void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::SetStart(
AXPositionInstance new_start) {
bool did_tree_change = start_->tree_id() != new_start->tree_id();
if (did_tree_change && !start_->IsNullPosition() &&
// TODO(bebeaudr): We can't use IsNullPosition() here because of
// https://crbug.com/1152939. Once this is fixed, we can go back to
// IsNullPosition().
if (did_tree_change && start_->kind() != AXPositionKind::NULL_POSITION &&
start_->tree_id() != end_->tree_id()) {
RemoveObserver(start_->tree_id());
}
......@@ -1478,7 +1481,10 @@ void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::SetStart(
void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::SetEnd(
AXPositionInstance new_end) {
bool did_tree_change = end_->tree_id() != new_end->tree_id();
if (did_tree_change && !end_->IsNullPosition() &&
// TODO(bebeaudr): We can't use IsNullPosition() here because of
// https://crbug.com/1152939. Once this is fixed, we can go back to
// IsNullPosition().
if (did_tree_change && end_->kind() != AXPositionKind::NULL_POSITION &&
end_->tree_id() != start_->tree_id()) {
RemoveObserver(end_->tree_id());
}
......
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