Commit 98acdea7 authored by Dominic Mazzoni's avatar Dominic Mazzoni Committed by Commit Bot

Avoid recalculating native role in AXNodeObject::CanHaveChildren

CanHaveChildren() is called frequently, including from delicate
situations like when a LayoutObject is being deleted. Currently it
calls NativeAccessibilityRoleIgnoringAria(), which is actually
somewhat expensive and calls UpdateDistribution(), and clusterfuzz
found a way for that to result in a UAF.

The easy solution is just to save the result of
NativeAccessibilityRoleIgnoringAria when we initially
compute the node's role. Then CanHaveChildren() can just
check it directly rather than recomputing it each time.

Bug: 852735, 852251
Change-Id: Id745d3b42c1f89434e519195e8511159621734d0
Reviewed-on: https://chromium-review.googlesource.com/1175491
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarAlice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583726}
parent 537f3a38
...@@ -312,15 +312,16 @@ AccessibilityRole AXLayoutObject::DetermineAccessibilityRole() { ...@@ -312,15 +312,16 @@ AccessibilityRole AXLayoutObject::DetermineAccessibilityRole() {
if (!layout_object_) if (!layout_object_)
return kUnknownRole; return kUnknownRole;
native_role_ = NativeAccessibilityRoleIgnoringAria();
if ((aria_role_ = DetermineAriaRoleAttribute()) != kUnknownRole) if ((aria_role_ = DetermineAriaRoleAttribute()) != kUnknownRole)
return aria_role_; return aria_role_;
AccessibilityRole role = NativeAccessibilityRoleIgnoringAria();
// Anything that needs to still be exposed but doesn't have a more specific // Anything that needs to still be exposed but doesn't have a more specific
// role should be considered a generic container. Examples are // role should be considered a generic container. Examples are
// layout blocks with no node, in-page link targets, and plain elements // layout blocks with no node, in-page link targets, and plain elements
// such as a <span> with ARIA markup. // such as a <span> with ARIA markup.
return role == kUnknownRole ? kGenericContainerRole : role; return native_role_ == kUnknownRole ? kGenericContainerRole : native_role_;
} }
Node* AXLayoutObject::GetNodeOrContainingBlockNode() const { Node* AXLayoutObject::GetNodeOrContainingBlockNode() const {
......
...@@ -87,6 +87,7 @@ const int kDefaultHeadingLevel = 2; ...@@ -87,6 +87,7 @@ const int kDefaultHeadingLevel = 2;
AXNodeObject::AXNodeObject(Node* node, AXObjectCacheImpl& ax_object_cache) AXNodeObject::AXNodeObject(Node* node, AXObjectCacheImpl& ax_object_cache)
: AXObject(ax_object_cache), : AXObject(ax_object_cache),
children_dirty_(false), children_dirty_(false),
native_role_(kUnknownRole),
node_(node) {} node_(node) {}
AXNodeObject* AXNodeObject::Create(Node* node, AXNodeObject* AXNodeObject::Create(Node* node,
...@@ -519,13 +520,14 @@ AccessibilityRole AXNodeObject::DetermineAccessibilityRole() { ...@@ -519,13 +520,14 @@ AccessibilityRole AXNodeObject::DetermineAccessibilityRole() {
if (!GetNode()) if (!GetNode())
return kUnknownRole; return kUnknownRole;
native_role_ = NativeAccessibilityRoleIgnoringAria();
if ((aria_role_ = DetermineAriaRoleAttribute()) != kUnknownRole) if ((aria_role_ = DetermineAriaRoleAttribute()) != kUnknownRole)
return aria_role_; return aria_role_;
if (GetNode()->IsTextNode()) if (GetNode()->IsTextNode())
return kStaticTextRole; return kStaticTextRole;
AccessibilityRole role = NativeAccessibilityRoleIgnoringAria(); return native_role_ == kUnknownRole ? kGenericContainerRole : native_role_;
return role == kUnknownRole ? kGenericContainerRole : role;
} }
void AXNodeObject::AccessibilityChildrenFromAOMProperty( void AXNodeObject::AccessibilityChildrenFromAOMProperty(
...@@ -2156,7 +2158,7 @@ bool AXNodeObject::CanHaveChildren() const { ...@@ -2156,7 +2158,7 @@ bool AXNodeObject::CanHaveChildren() const {
return false; return false;
} }
switch (NativeAccessibilityRoleIgnoringAria()) { switch (native_role_) {
case kButtonRole: case kButtonRole:
case kCheckBoxRole: case kCheckBoxRole:
case kImageRole: case kImageRole:
......
...@@ -54,6 +54,8 @@ class MODULES_EXPORT AXNodeObject : public AXObject { ...@@ -54,6 +54,8 @@ class MODULES_EXPORT AXNodeObject : public AXObject {
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
bool initialized_ = false; bool initialized_ = false;
#endif #endif
// The accessibility role, not taking ARIA into account.
AccessibilityRole native_role_;
bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const override; bool ComputeAccessibilityIsIgnored(IgnoredReasons* = nullptr) const override;
const AXObject* InheritsPresentationalRoleFrom() const override; const AXObject* InheritsPresentationalRoleFrom() const override;
......
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