Bug 214882 - Improve range idioms and other changes to prepare the way for more reduction in live range use
Summary: Improve range idioms and other changes to prepare the way for more reduction ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
: 215521 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-07-28 10:00 PDT by Darin Adler
Modified: 2020-08-15 10:16 PDT (History)
17 users (show)

See Also:

Attachments
Patch (108.16 KB, patch)
2020-07-28 15:58 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (109.18 KB, patch)
2020-07-28 16:48 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2020-07-28 10:00:54 PDT 
Improve range idioms and other changes to prepare the way for more reduction in live range use
Comment 1 Darin Adler 2020-07-28 15:58:36 PDT Comment hidden (obsolete)
Comment 2 Darin Adler 2020-07-28 16:48:07 PDT 
Created attachment 405427 [details]
Patch
Comment 3 Sam Weinig 2020-07-28 19:17:04 PDT 
Comment on attachment 405427 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:1181
>  VisiblePositionRange AccessibilityObject::lineRangeForPosition(const VisiblePosition& visiblePosition) const
>  {
> -    VisiblePosition startPosition = startOfLine(visiblePosition);
> -    VisiblePosition endPosition = endOfLine(visiblePosition);
> -    return VisiblePositionRange(startPosition, endPosition);
> +    return { startOfLine(visiblePosition), endOfLine(visiblePosition) };

Some of these functions don't seem like they belong as member functions on AccessibilityObject, given how general they are, and that they don't seem to be using any state or functions of AccessibilityObject. In another change (not this one), it might be a good idea to move some of these to VisibileUnit.h or similar.
Comment 4 Darin Adler 2020-07-29 09:27:38 PDT 
Comment on attachment 405427 [details]
Patch

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

>> Source/WebCore/accessibility/AccessibilityObject.cpp:1181
>> +    return { startOfLine(visiblePosition), endOfLine(visiblePosition) };
> 
> Some of these functions don't seem like they belong as member functions on AccessibilityObject, given how general they are, and that they don't seem to be using any state or functions of AccessibilityObject. In another change (not this one), it might be a good idea to move some of these to VisibileUnit.h or similar.

Agreed. I’ll try to come back to that later. In some cases the functions look universal but have some sort of subtle dependency on the accessibility object, so it’s a non-trivial project to disentangle.
Comment 5 EWS 2020-07-29 09:36:46 PDT 
Committed r265044: <https://trac.webkit.org/changeset/265044>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405427 [details].
Comment 6 Radar WebKit Bug Importer 2020-07-29 09:37:18 PDT 
<rdar://problem/66277887>
Comment 7 Andres Gonzalez 2020-08-15 10:16:25 PDT 
*** Bug 215521 has been marked as a duplicate of this bug. ***