Bug 215070 – Remove some member functions of Range and many calls to createLiveRange

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Adler]

Remove some member functions of Range and many calls to createLiveRange

[Darin Adler]

Created attachment 405803 [details]
Patch

[Darin Adler]

Created attachment 405804 [details]
Patch

[Darin Adler]

Created attachment 405809 [details]
Patch

[Darin Adler]

Passing all tests now; given recent history I’m guessing review will be Sam?

[Sam Weinig]

Comment on attachment 405809 [details]
Patch

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

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2342
> +// FIXME: No reason for this to be a method instead of a function.
> +- (NSRange)_convertToNSRange:(const SimpleRange&)range

Reminds me we should investigate using direct methods / members at some point (https://reviews.llvm.org/D69991 / https://nshipster.com/direct/).

> Source/WebCore/page/TextIndicator.cpp:306
> +        textRects = RenderObject::collectSelectionRects(range).map([&](const auto& rect) -> FloatRect {

Is the explicit return type needed here?

> Source/WebCore/rendering/RenderObject.h:551
> +    WEBCORE_EXPORT static Vector<SelectionRect> collectSelectionRects(const SimpleRange&);
> +    WEBCORE_EXPORT static Vector<SelectionRect> collectSelectionRectsWithoutUnionInteriorLines(const SimpleRange&);

I'm a little sad to be adding more to RenderObject, but given that there is already collectSelectionRects functions here, it seems ok. Do they use the fact that they are static member functions in anyway?

[EWS]

Committed r265190: <https://trac.webkit.org/changeset/265190>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405809 [details].

[Radar WebKit Bug Importer]

<rdar://problem/66454378>

[Darin Adler]

Comment on attachment 405809 [details]
Patch

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

>> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:2342
>> +- (NSRange)_convertToNSRange:(const SimpleRange&)range
> 
> Reminds me we should investigate using direct methods / members at some point (https://reviews.llvm.org/D69991 / https://nshipster.com/direct/).

Sure, that would apply to even more methods, including ones that, unlike this one, do use self!

>> Source/WebCore/page/TextIndicator.cpp:306
>> +        textRects = RenderObject::collectSelectionRects(range).map([&](const auto& rect) -> FloatRect {
> 
> Is the explicit return type needed here?

Yes, because otherwise it’s an IntRect.

>> Source/WebCore/rendering/RenderObject.h:551
>> +    WEBCORE_EXPORT static Vector<SelectionRect> collectSelectionRectsWithoutUnionInteriorLines(const SimpleRange&);
> 
> I'm a little sad to be adding more to RenderObject, but given that there is already collectSelectionRects functions here, it seems ok. Do they use the fact that they are static member functions in anyway?

They do not. These functions can live anywhere appropriate. I am happy to move them out of RenderObject to a better home. When moving them out of Range I chose to put them next to the underlying RenderObject functions they are based on, but would be happy to put them somewhere else.

Clearly they are part of "rendering".

Really open to moving them again! It won’t be hard.