Bug 213701

Summary: [iOS] Clean up visible position comparisons in WebPage::requestDocumentEditingContext()
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebKit Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
To Land none

Description Daniel Bates 2020-06-28 11:46:38 PDT
Use std::min<VisiblePosition> and std::max<VisiblePosition> when the result needs to be copied instead of doing the comparison by hand to make the code a tiny bit more clear. Also, don't copy a VisiblePosition when doing the min/max in-place.
Comment 1 Daniel Bates 2020-06-28 11:49:03 PDT
Created attachment 403003 [details]
Patch
Comment 2 Daniel Bates 2020-06-28 11:51:08 PDT
Comment on attachment 403003 [details]
Patch

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

> Source/WebKit/ChangeLog:12
> +        are unlikely to effect code generation. If they do, they make things a tiny bit more
> +        efficient.

To be precise, only the latter makes things a tiny bit more efficient.
Comment 3 Geoffrey Garen 2020-06-28 12:07:24 PDT
Comment on attachment 403003 [details]
Patch

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

r=me

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:4183
> +        startOfRangeOfInterestInSelection = std::max<VisiblePosition>(rangeOfInterestStart, selectionStart);
> +        endOfRangeOfInterestInSelection = std::min<VisiblePosition>(rangeOfInterestEnd, selectionEnd);

No need to specify <VisiblePosition> when both arguments are <VisiblePosition>.
Comment 4 Daniel Bates 2020-06-28 12:13:21 PDT
Comment on attachment 403003 [details]
Patch

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

Thanks for the review, Geoff!

>> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:4183
>> +        endOfRangeOfInterestInSelection = std::min<VisiblePosition>(rangeOfInterestEnd, selectionEnd);
> 
> No need to specify <VisiblePosition> when both arguments are <VisiblePosition>.

OK.
Comment 5 Daniel Bates 2020-06-28 12:15:06 PDT
Created attachment 403004 [details]
To Land
Comment 6 Daniel Bates 2020-06-28 12:16:09 PDT
Comment on attachment 403004 [details]
To Land

Clearing flags on attachment: 403004

Committed r263634: <https://trac.webkit.org/changeset/263634>
Comment 7 Daniel Bates 2020-06-28 12:16:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2020-06-28 12:17:11 PDT
<rdar://problem/64865522>