Bug 215004 - Remove Range::create and many more uses of live ranges
Summary: Remove Range::create and many more uses of live ranges
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
Depends on:
Blocks:
 
Reported: 2020-07-30 20:09 PDT by Darin Adler
Modified: 2020-08-01 09:52 PDT (History)
19 users (show)

See Also:

Attachments
Patch (123.61 KB, patch)
2020-07-30 20:10 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (126.34 KB, patch)
2020-07-30 21:06 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (128.23 KB, patch)
2020-07-30 21:57 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (143.78 KB, patch)
2020-07-31 11:00 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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-30 20:09:57 PDT 
Remove Range::create and almost all uses of live ranges: after this just burn down about 150 uses of createLiveRange
Comment 1 Darin Adler 2020-07-30 20:10:55 PDT Comment hidden (obsolete)
Comment 2 Darin Adler 2020-07-30 20:24:47 PDT 
The follow up to this is search and destroy of calls to createLiveRange. Should be done in chunks in case we break something.
Comment 3 Darin Adler 2020-07-30 21:06:52 PDT Comment hidden (obsolete)
Comment 4 Darin Adler 2020-07-30 21:57:51 PDT Comment hidden (obsolete)
Comment 5 Darin Adler 2020-07-31 11:00:29 PDT 
Created attachment 405709 [details]
Patch
Comment 6 Darin Adler 2020-07-31 11:02:55 PDT 
Last version passed EWS all green, but didn’t have change log yet. Now up with change log and ready to review.
Comment 7 Sam Weinig 2020-08-01 08:38:35 PDT 
Comment on attachment 405709 [details]
Patch

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

> Source/WebCore/dom/DocumentMarkerController.h:118
> +WEBCORE_EXPORT SimpleRange range(Node&, const DocumentMarker&);

I am not sure how I feel about this just being called "range", though I can see the arguments against making this an overload of makeSimpleRange as well.

> Source/WebKitLegacy/ios/WebCoreSupport/WebVisiblePosition.mm:479
> +    auto firstVP = [first _visiblePosition];
> +    auto secondVP = [second _visiblePosition];

Not new, but also don't think the abbreviations are great here.
Comment 8 EWS 2020-08-01 08:50:42 PDT 
Committed r265176: <https://trac.webkit.org/changeset/265176>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405709 [details].
Comment 9 Radar WebKit Bug Importer 2020-08-01 08:51:18 PDT 
<rdar://problem/66425375>
Comment 10 Darin Adler 2020-08-01 09:52:53 PDT 
Comment on attachment 405709 [details]
Patch

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

>> Source/WebCore/dom/DocumentMarkerController.h:118
>> +WEBCORE_EXPORT SimpleRange range(Node&, const DocumentMarker&);
> 
> I am not sure how I feel about this just being called "range", though I can see the arguments against making this an overload of makeSimpleRange as well.

I will rename to makeSimpleRange for now.

>> Source/WebKitLegacy/ios/WebCoreSupport/WebVisiblePosition.mm:479
>> +    auto secondVP = [second _visiblePosition];
> 
> Not new, but also don't think the abbreviations are great here.

I agree and almost changed them. Probably should have gone with firstPosition and secondPosition.
Comment 11 Darin Adler 2020-08-01 09:52:54 PDT 
Comment on attachment 405709 [details]
Patch

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

>> Source/WebCore/dom/DocumentMarkerController.h:118
>> +WEBCORE_EXPORT SimpleRange range(Node&, const DocumentMarker&);
> 
> I am not sure how I feel about this just being called "range", though I can see the arguments against making this an overload of makeSimpleRange as well.

I will rename to makeSimpleRange for now.

>> Source/WebKitLegacy/ios/WebCoreSupport/WebVisiblePosition.mm:479
>> +    auto secondVP = [second _visiblePosition];
> 
> Not new, but also don't think the abbreviations are great here.

I agree and almost changed them. Probably should have gone with firstPosition and secondPosition.