Bug 213701 - [iOS] Clean up visible position comparisons in WebPage::requestDocumentEditingContext()
Summary: [iOS] Clean up visible position comparisons in WebPage::requestDocumentEditin...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-28 11:46 PDT by Daniel Bates
Modified: 2020-06-28 12:17 PDT (History)
2 users (show)

See Also:

Attachments
Patch (3.58 KB, patch)
2020-06-28 11:49 PDT, Daniel Bates 		no flags Details | Formatted Diff | Diff
To Land (3.51 KB, patch)
2020-06-28 12:15 PDT, Daniel Bates 		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 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>