Bug 272279 – AX: Use more reference types in accessibility code when values are known to be non-null

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Tyler Wilcock]

This reduces unnecessary null checks and allows us to remove ASSERT(value) and unchecked dereferences.

[Radar WebKit Bug Importer]

<rdar://problem/126020262>

[Tyler Wilcock]

Created attachment 470787 [details]
Patch

[Tyler Wilcock]

Created attachment 470788 [details]
Patch

[Tyler Wilcock]

Created attachment 470789 [details]
Patch

[Tyler Wilcock]

Created attachment 470790 [details]
Patch

[chris fleizach]

Comment on attachment 470790 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=470790&action=review

> Source/WebCore/accessibility/AXObjectCache.cpp:154
> +static bool rendererIsValid(Node& node)

Feels like name should be

NodeRendererIsValid

> Source/WebCore/accessibility/AXObjectCache.cpp:3136
>      Node* domNode = &node;

Do we still
Need a ptr here or can we use a reference in the method

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1743
>      for (RefPtr e = position.anchorElementAncestor(); e && e != rootEditableElement; e = e->parentElement()) {

Do we have to use e for a variable name?

[Tyler Wilcock]

Created attachment 470919 [details]
Patch

[Tyler Wilcock]

> > Source/WebCore/accessibility/AXObjectCache.cpp:3136
> >      Node* domNode = &node;
> 
> Do we still
> Need a ptr here or can we use a reference in the method
We do still need a pointer, as `domNode` gets re-assigned in the loop below, and references aren't allowed to be re-assigned.

Fixed the other two comments, thanks!

[Tyler Wilcock]

Created attachment 470966 [details]
Patch

[Tyler Wilcock]

Created attachment 470982 [details]
Patch

[EWS]

Committed 277672@main (9487ec0b9ebd): <https://commits.webkit.org/277672@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 470982 [details].