Bug 272279 - AX: Use more reference types in accessibility code when values are known to be non-null
Summary: AX: Use more reference types in accessibility code when values are known to b...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2024-04-06 09:54 PDT by Tyler Wilcock
Modified: 2024-04-18 07:55 PDT (History)
19 users (show)

See Also:


Attachments
Patch (71.43 KB, patch)
2024-04-06 09:57 PDT, Tyler Wilcock
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (71.83 KB, patch)
2024-04-06 10:15 PDT, Tyler Wilcock
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (74.24 KB, patch)
2024-04-06 10:20 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (74.28 KB, patch)
2024-04-06 10:29 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (74.55 KB, patch)
2024-04-14 07:25 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (74.55 KB, patch)
2024-04-17 11:44 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (74.55 KB, patch)
2024-04-17 21:43 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2024-04-06 09:54:01 PDT
This reduces unnecessary null checks and allows us to remove ASSERT(value) and unchecked dereferences.
Comment 1 Radar WebKit Bug Importer 2024-04-06 09:54:09 PDT
<rdar://problem/126020262>
Comment 2 Tyler Wilcock 2024-04-06 09:57:38 PDT
Created attachment 470787 [details]
Patch
Comment 3 Tyler Wilcock 2024-04-06 10:15:52 PDT
Created attachment 470788 [details]
Patch
Comment 4 Tyler Wilcock 2024-04-06 10:20:14 PDT
Created attachment 470789 [details]
Patch
Comment 5 Tyler Wilcock 2024-04-06 10:29:55 PDT
Created attachment 470790 [details]
Patch
Comment 6 chris fleizach 2024-04-14 06:20:09 PDT
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?
Comment 7 Tyler Wilcock 2024-04-14 07:25:36 PDT
Created attachment 470919 [details]
Patch
Comment 8 Tyler Wilcock 2024-04-14 07:27:51 PDT
> > 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!
Comment 9 Tyler Wilcock 2024-04-17 11:44:30 PDT
Created attachment 470966 [details]
Patch
Comment 10 Tyler Wilcock 2024-04-17 21:43:12 PDT
Created attachment 470982 [details]
Patch
Comment 11 EWS 2024-04-18 07:55:42 PDT
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].