Commit 129040a6 authored by Benjamin Beaudry's avatar Benjamin Beaudry Committed by Commit Bot

Replace use of ScopedObserver by a simple observer in TextRangeEndpoints

In CL:2432805, we added a ScopedObserver on
AXPlatformNodeTextRangeProviderWin::TextRangeProvider. However, that
appears to cause a crash and we decided to go back to a simple observer
instead.

The crash was caused by the difference in lifetime of the AXTreeManager
and the AXPlatformNodeTextRangeProviderWin. In some cases, the
AXTreeManager can get deleted before the TextRangeEndpoints does, so
when the destructor of the ScopedObserver calls
ScopedObserver::RemoveAll on an already deleted AXTreeManager, it
crashes.

Bug: N/A
Change-Id: I24291b6609d9f1102f94e177d1751b14e005425b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505774Reviewed-by: default avatarDaniel Libby <dlibby@microsoft.com>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#821942}
parent 83e275fd
......@@ -1447,7 +1447,10 @@ AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::TextRangeEndpoints() {
end_ = AXNodePosition::CreateNullPosition();
}
AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::~TextRangeEndpoints() {}
AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::~TextRangeEndpoints() {
SetStart(AXNodePosition::CreateNullPosition());
SetEnd(AXNodePosition::CreateNullPosition());
}
void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::SetStart(
AXPositionInstance new_start) {
......@@ -1486,15 +1489,15 @@ void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::AddObserver(
AXTreeManager* ax_tree_manager =
AXTreeManagerMap::GetInstance().GetManager(tree_id);
DCHECK(ax_tree_manager);
observer_.Add(ax_tree_manager);
ax_tree_manager->AddObserver(this);
}
void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::RemoveObserver(
const AXTreeID tree_id) {
AXTreeManager* ax_tree_manager =
AXTreeManagerMap::GetInstance().GetManager(tree_id);
DCHECK(ax_tree_manager);
observer_.Remove(ax_tree_manager);
if (ax_tree_manager)
ax_tree_manager->RemoveObserver(this);
}
void AXPlatformNodeTextRangeProviderWin::TextRangeEndpoints::
......
......@@ -11,7 +11,6 @@
#include <tuple>
#include <vector>
#include "base/scoped_observer.h"
#include "ui/accessibility/ax_node_position.h"
#include "ui/accessibility/ax_position.h"
#include "ui/accessibility/ax_range.h"
......@@ -193,6 +192,13 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1"))
Microsoft::WRL::ComPtr<AXPlatformNodeWin> owner_;
// Why we can't use a ScopedObserver here:
// We tried using a ScopedObserver instead of a simple observer in this case,
// but there appears to be a problem with the lifetime of the referenced
// AXTreeManager in the ScopedObserver. The AXTreeManager can get deleted
// before the TextRangeEndpoints does, so when the destructor of the
// ScopedObserver calls ScopedObserver::RemoveAll on an already deleted
// AXTreeManager, it crashes.
class TextRangeEndpoints : public AXTreeObserver {
public:
TextRangeEndpoints();
......@@ -209,8 +215,6 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1"))
private:
AXPositionInstance start_;
AXPositionInstance end_;
ScopedObserver<AXTreeManager, AXTreeObserver> observer_{this};
};
TextRangeEndpoints endpoints_;
};
......
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