Skip to content

Commit

Permalink
Fix AXTreeManager type confusion in AXPlatformNodeTextRangeProviderWin
Browse files Browse the repository at this point in the history
In `AXPlatformNodeTextRangeProviderWin::GetOwner`, there is an
assumption that the position's tree is an `AXPlatformTreeManager`, but
there are some cases (Views) where this is not currently the case.
In those cases, we can't safely cast to `AXPlatformTreeManager`.

This change adds a virtual method to determine whether an AXTreeManager
is an AXPlatformTreeManager to allow us to safely cast.

(cherry picked from commit 028d1e2)

Fixed: 1451763
Change-Id: I65ebae42d200aaddbfe914e54a2c5fcb69974205
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599972
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Original-Commit-Position: refs/heads/main@{#1155268}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4605023
Auto-Submit: Kurt Catti-Schmidt <kschmi@microsoft.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#553}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
KurtCattiSchmidt authored and Chromium LUCI CQ committed Jun 10, 2023
1 parent 1f869c2 commit e6bd2af
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 13 deletions.
4 changes: 4 additions & 0 deletions ui/accessibility/ax_tree_manager.cc
Expand Up @@ -143,6 +143,10 @@ AXTreeID AXTreeManager::GetParentTreeID() const {
return ax_tree_ ? ax_tree_->data().parent_tree_id : AXTreeIDUnknown();
}

bool AXTreeManager::IsPlatformTreeManager() const {
return false;
}

AXNode* AXTreeManager::GetRoot() const {
return ax_tree_ ? ax_tree_->root() : nullptr;
}
Expand Down
11 changes: 8 additions & 3 deletions ui/accessibility/ax_tree_manager.h
Expand Up @@ -16,9 +16,10 @@ namespace ui {
class AXNode;
class AXTreeManagerMap;

// Abstract interface for a class that owns an AXTree and manages its
// connections to other AXTrees in the same page or desktop (parent and child
// trees).
// Interface for a class that owns an AXTree and manages its connections
// to other AXTrees in the same page or desktop (parent and child trees)
// as well as a mapping of AXNode's by ID for supporting `GetNodeFromTree`
// and related methods.
//
// Note, the tree manager may be created for a tree which has unknown (not
// valid) tree id. A such tree is not registered with the tree map and thus
Expand Down Expand Up @@ -86,6 +87,10 @@ class AX_EXPORT AXTreeManager : public AXTreeObserver {
// Returns AXTreeIDUnknown if this tree doesn't have a parent tree.
virtual AXTreeID GetParentTreeID() const;

// Whether this manager can access platform nodes. Defaults to false
// and is overridden in `AXPlatformTreeManager` to return true.
virtual bool IsPlatformTreeManager() const;

// Returns the AXNode that is at the root of the current tree.
AXNode* GetRoot() const;

Expand Down
1 change: 1 addition & 0 deletions ui/accessibility/platform/BUILD.gn
Expand Up @@ -47,6 +47,7 @@ component("platform") {
"child_iterator_base.h",

# Used by browser_accessibility_manager.cc.
"ax_platform_tree_manager.cc",
"ax_platform_tree_manager.h",

# Used by browser_accessibility.cc.
Expand Down
Expand Up @@ -1213,7 +1213,7 @@ std::u16string AXPlatformNodeTextRangeProviderWin::GetString(
}

AXPlatformNodeWin* AXPlatformNodeTextRangeProviderWin::GetOwner() const {
// Unit tests can't call |GetPlatformNodeFromTree|, so they must provide an
// Unit tests can't call `GetPlatformNodeFromTree`, so they must provide an
// owner node.
if (owner_for_test_.Get())
return owner_for_test_.Get();
Expand All @@ -1227,14 +1227,16 @@ AXPlatformNodeWin* AXPlatformNodeTextRangeProviderWin::GetOwner() const {
const AXNode* anchor = position->GetAnchor();
DCHECK(anchor);
const AXTreeManager* ax_tree_manager = position->GetManager();
DCHECK(ax_tree_manager);
if (ax_tree_manager && ax_tree_manager->IsPlatformTreeManager()) {
const AXPlatformTreeManager* platform_tree_manager =
static_cast<const AXPlatformTreeManager*>(ax_tree_manager);
DCHECK(platform_tree_manager);

const AXPlatformTreeManager* platform_tree_manager =
static_cast<const AXPlatformTreeManager*>(ax_tree_manager);
DCHECK(platform_tree_manager);
return static_cast<AXPlatformNodeWin*>(
platform_tree_manager->GetPlatformNodeFromTree(*anchor));
}

return static_cast<AXPlatformNodeWin*>(
platform_tree_manager->GetPlatformNodeFromTree(*anchor));
return nullptr;
}

AXPlatformNodeDelegate* AXPlatformNodeTextRangeProviderWin::GetDelegate(
Expand Down
13 changes: 13 additions & 0 deletions ui/accessibility/platform/ax_platform_tree_manager.cc
@@ -0,0 +1,13 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ui/accessibility/platform/ax_platform_tree_manager.h"

namespace ui {

bool AXPlatformTreeManager::IsPlatformTreeManager() const {
return true;
}

} // namespace ui
8 changes: 5 additions & 3 deletions ui/accessibility/platform/ax_platform_tree_manager.h
Expand Up @@ -15,9 +15,9 @@ namespace ui {
class AXPlatformNode;
class AXPlatformNodeDelegate;

// Abstract interface for a class that owns an AXTree and manages its
// connections to other AXTrees in the same page or desktop (parent and child
// trees).
// Abstract interface for a class that manages AXPlatformNodes and is
// able to query for them via `GetPlatformNodeFromTree`. Extends
// AXTreeManager, so AXNodes are also managed.
class COMPONENT_EXPORT(AX_PLATFORM) AXPlatformTreeManager
: public AXTreeManager {
public:
Expand All @@ -32,6 +32,8 @@ class COMPONENT_EXPORT(AX_PLATFORM) AXPlatformTreeManager
// of the accessibility tree.
virtual AXPlatformNodeDelegate* RootDelegate() const = 0;

bool IsPlatformTreeManager() const override;

protected:
explicit AXPlatformTreeManager(std::unique_ptr<AXTree> tree)
: AXTreeManager(std::move(tree)) {}
Expand Down

0 comments on commit e6bd2af

Please sign in to comment.