Bug 223823 - WebFeature class fails to copy/retain its instance variables
Summary: WebFeature class fails to copy/retain its instance variables
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-26 15:56 PDT by David Kilzer (:ddkilzer)
Modified: 2021-03-27 10:00 PDT (History)
2 users (show)

See Also:

Attachments
Patch v1 (2.41 KB, patch)
2021-03-26 15:58 PDT, David Kilzer (:ddkilzer) 		no flags Details | Formatted Diff | Diff
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 David Kilzer (:ddkilzer) 2021-03-26 15:56:41 PDT 
WebFeature class fails to copy/retain its instance variables despite what its @property declarations state.

To fix this, I'm just going to add -copy methods in the init method, and add a -dealloc method that can be deleted when switching to ARC.

Using RetainPtr<> for this class seems overkill.
Comment 1 David Kilzer (:ddkilzer) 2021-03-26 15:58:20 PDT 
Created attachment 424412 [details]
Patch v1
Comment 2 Sam Weinig 2021-03-26 17:27:04 PDT 
Comment on attachment 424412 [details]
Patch v1

I think Chris is trying to use RetainPtr everywhere, so we probably should here too, but you don't have to in this patch. I can do so in a follow up.
Comment 3 EWS 2021-03-26 21:47:07 PDT 
Committed r275131: <https://commits.webkit.org/r275131>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424412 [details].
Comment 4 Radar WebKit Bug Importer 2021-03-26 21:48:12 PDT 
<rdar://problem/75913890>
Comment 5 David Kilzer (:ddkilzer) 2021-03-27 10:00:12 PDT 
(In reply to Sam Weinig from comment #2)
> Comment on attachment 424412 [details]
> Patch v1
> 
> I think Chris is trying to use RetainPtr everywhere, so we probably should
> here too, but you don't have to in this patch. I can do so in a follow up.

Those were some speculative fixes to try to resolve some CoreIPC crashes if I remember correctly.  I appreciate you offering to follow up on this.