Bug 186301

Summary: [Cocoa] Update some JavaScriptCore code to be more ready for ARC
Product: WebKit Reporter: Darin Adler <darin>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, dbates, ews-watchlist, joepeck, keith_miller, mark.lam, mitz, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 221107    
Attachments:
Description Flags
Patch
none
Patch
none
Patch andersca: review+

Description Darin Adler 2018-06-04 22:51:09 PDT
[Cocoa] Update some JavaScriptCore code to be more ready for ARC
Comment 1 Darin Adler 2018-06-04 22:52:55 PDT
Created attachment 341955 [details]
Patch
Comment 2 Anders Carlsson 2018-06-05 07:20:17 PDT
Comment on attachment 341955 [details]
Patch

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

> Source/JavaScriptCore/API/JSAPIWrapperObject.mm:95
> -    m_wrappedObject = [static_cast<id>(wrappedObject) retain];
> +    m_wrappedObject = (__bridge id)wrappedObject;

Won't this lead to a memory leak when compiling without ARC now?
Comment 3 Darin Adler 2018-06-05 08:15:19 PDT
Comment on attachment 341955 [details]
Patch

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

>> Source/JavaScriptCore/API/JSAPIWrapperObject.mm:95
>> +    m_wrappedObject = (__bridge id)wrappedObject;
> 
> Won't this lead to a memory leak when compiling without ARC now?

Yes should have omitted this change; will remove it. I’ve been doing a verification pass on these “ARC prep” patches where I search for release and retain in the patch, which I think would have caught this.
Comment 4 Darin Adler 2018-06-05 08:50:49 PDT
Created attachment 341969 [details]
Patch
Comment 5 Darin Adler 2018-06-05 08:51:32 PDT
New patch omits the incorrect change Anders pointed out, and also fixes another leak elsewhere in the patch.
Comment 6 Darin Adler 2018-06-05 08:53:01 PDT
Created attachment 341970 [details]
Patch
Comment 7 Darin Adler 2018-06-05 09:16:07 PDT
This quite small patch should be straightforward and correct now.
Comment 8 Darin Adler 2018-06-05 10:25:05 PDT
Committed r232513: <https://trac.webkit.org/changeset/232513>
Comment 9 Radar WebKit Bug Importer 2018-06-05 10:26:26 PDT
<rdar://problem/40812844>