RESOLVED FIXED Bug 213701
[iOS] Clean up visible position comparisons in WebPage::requestDocumentEditingContext() 
https://bugs.webkit.org/show_bug.cgi?id=213701
Summary [iOS] Clean up visible position comparisons in WebPage::requestDocumentEditin...
Daniel Bates
Reported 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.
Attachments
Patch (3.58 KB, patch)
2020-06-28 11:49 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
To Land (3.51 KB, patch)
2020-06-28 12:15 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2020-06-28 11:49:03 PDT
Created attachment 403003 [details] Patch
Daniel Bates
Comment 2 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.
Geoffrey Garen
Comment 3 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>.
Daniel Bates
Comment 4 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.
Daniel Bates
Comment 5 2020-06-28 12:15:06 PDT
Created attachment 403004 [details] To Land
Daniel Bates
Comment 6 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>
Daniel Bates
Comment 7 2020-06-28 12:16:11 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2020-06-28 12:17:11 PDT
<rdar://problem/64865522>
Note You need to log in before you can comment on or make changes to this bug.