WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
215004
Remove Range::create and many more uses of live ranges
https://bugs.webkit.org/show_bug.cgi?id=215004
Summary
Remove Range::create and many more uses of live ranges
Darin Adler
Reported
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
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 attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2020-07-30 20:10:55 PDT
Comment hidden (obsolete)
Created
attachment 405654
[details]
Patch
Darin Adler
Comment 2
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.
Darin Adler
Comment 3
2020-07-30 21:06:52 PDT
Comment hidden (obsolete)
Created
attachment 405660
[details]
Patch
Darin Adler
Comment 4
2020-07-30 21:57:51 PDT
Comment hidden (obsolete)
Created
attachment 405665
[details]
Patch
Darin Adler
Comment 5
2020-07-31 11:00:29 PDT
Created
attachment 405709
[details]
Patch
Darin Adler
Comment 6
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.
Sam Weinig
Comment 7
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.
EWS
Comment 8
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]
.
Radar WebKit Bug Importer
Comment 9
2020-08-01 08:51:18 PDT
<
rdar://problem/66425375
>
Darin Adler
Comment 10
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.
Darin Adler
Comment 11
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug