RESOLVED FIXED 186301
[Cocoa] Update some JavaScriptCore code to be more ready for ARC 
https://bugs.webkit.org/show_bug.cgi?id=186301
Summary [Cocoa] Update some JavaScriptCore code to be more ready for ARC
Darin Adler
Reported 2018-06-04 22:51:09 PDT
[Cocoa] Update some JavaScriptCore code to be more ready for ARC
Attachments
Patch (5.79 KB, patch)
2018-06-04 22:52 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (5.62 KB, patch)
2018-06-05 08:50 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (4.77 KB, patch)
2018-06-05 08:53 PDT, Darin Adler 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2018-06-04 22:52:55 PDT
Created attachment 341955 [details] Patch
Anders Carlsson
Comment 2 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?
Darin Adler
Comment 3 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.
Darin Adler
Comment 4 2018-06-05 08:50:49 PDT
Created attachment 341969 [details] Patch
Darin Adler
Comment 5 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.
Darin Adler
Comment 6 2018-06-05 08:53:01 PDT
Created attachment 341970 [details] Patch
Darin Adler
Comment 7 2018-06-05 09:16:07 PDT
This quite small patch should be straightforward and correct now.
Darin Adler
Comment 8 2018-06-05 10:25:05 PDT
Committed r232513: <https://trac.webkit.org/changeset/232513>
Radar WebKit Bug Importer
Comment 9 2018-06-05 10:26:26 PDT
<rdar://problem/40812844>
Note You need to log in before you can comment on or make changes to this bug.