Bug 186301 - [Cocoa] Update some JavaScriptCore code to be more ready for ARC
Summary: [Cocoa] Update some JavaScriptCore code to be more ready for ARC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 221107
  Show dependency tree / graph
 
Reported: 2018-06-04 22:51 PDT by Darin Adler
Modified: 2021-01-28 14:58 PST (History)
10 users (show)

See Also:

Attachments
Patch (5.79 KB, patch)
2018-06-04 22:52 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2018-06-05 08:50 PDT, Darin Adler 		no flags Details | Formatted Diff | Diff
Patch (4.77 KB, patch)
2018-06-05 08:53 PDT, Darin Adler 		andersca: review+
Details | Formatted Diff | Diff
Show Obsolete (2) 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 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>