Bug 225908

Summary:

Use RefPtr for local ref counted objects of FrameSelection::setSelectionWithoutUpdatingAppearance

Product:

WebKit

Reporter:

Frédéric Wang (:fredw) <fred.wang>

Component:

HTML Editing

Assignee:

Frédéric Wang (:fredw) <fred.wang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, ews-feeder, product-security, rniwa, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

225219

Bug Blocks:

Attachments:

Description	Flags
Patch	rniwa: review+
Patch for landing	none

Description Frédéric Wang (:fredw) 2021-05-17 23:10:12 PDT

Comment 1 Radar WebKit Bug Importer 2021-05-17 23:10:26 PDT

<rdar://problem/78140308>

Comment 2 Frédéric Wang (:fredw) 2021-05-17 23:11:27 PDT

Created attachment 428915 [details]
Patch

Comment 3 Ryosuke Niwa 2021-05-18 01:49:21 PDT

Comment on attachment 428915 [details]
Patch

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

> Source/WebCore/editing/FrameSelection.cpp:1944
> -    ContainerNode* ownerElementParent = ownerElement->parentNode();
> +    RefPtr<ContainerNode> ownerElementParent = ownerElement->parentNode();

Use makeRefPtr?

Comment 4 Ryosuke Niwa 2021-05-18 01:49:37 PDT

I don't think this needs to be in the security component unless there is an actual security bug we're fixing.

Comment 5 Frédéric Wang (:fredw) 2021-05-18 01:54:07 PDT

Comment on attachment 428915 [details]
Patch

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

>> Source/WebCore/editing/FrameSelection.cpp:1944
>> +    RefPtr<ContainerNode> ownerElementParent = ownerElement->parentNode();
> 
> Use makeRefPtr?

This needs to be cast to RefPtr<ContainerNode>. Not sure how we do that in our code... or maybe you mean adding makeRefPtr in any case?

Comment 6 Frédéric Wang (:fredw) 2021-05-18 01:55:34 PDT

(In reply to Ryosuke Niwa from comment #4)
> I don't think this needs to be in the security component unless there is an
> actual security bug we're fixing.

Yeah, reading the code, I don't think any of this use is "dangerous".

Comment 7 Ryosuke Niwa 2021-05-18 02:03:51 PDT

Comment on attachment 428915 [details]
Patch

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

>>> Source/WebCore/editing/FrameSelection.cpp:1944
>>> +    RefPtr<ContainerNode> ownerElementParent = ownerElement->parentNode();
>> 
>> Use makeRefPtr?
> 
> This needs to be cast to RefPtr<ContainerNode>. Not sure how we do that in our code... or maybe you mean adding makeRefPtr in any case?

I'm confused. Given Node::parentNode() returns ContainerNode, calling makeRefPtr will surely make it RefPtr<ContainerNode>?

Comment 8 Frédéric Wang (:fredw) 2021-05-18 02:31:44 PDT

Comment on attachment 428915 [details]
Patch

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

>>>> Source/WebCore/editing/FrameSelection.cpp:1944
>>>> +    RefPtr<ContainerNode> ownerElementParent = ownerElement->parentNode();
>>> 
>>> Use makeRefPtr?
>> 
>> This needs to be cast to RefPtr<ContainerNode>. Not sure how we do that in our code... or maybe you mean adding makeRefPtr in any case?
> 
> I'm confused. Given Node::parentNode() returns ContainerNode, calling makeRefPtr will surely make it RefPtr<ContainerNode>?

OK, I stand corrected. I had build errors when I tried yesterday, but probably that was something else. Will update the patch and land it.

Comment 9 Frédéric Wang (:fredw) 2021-05-18 02:34:32 PDT

Created attachment 428923 [details]
Patch for landing

Comment 10 EWS 2021-05-18 03:01:34 PDT

Committed r277647 (237853@main): <https://commits.webkit.org/237853@main>

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