Bug 238583 - Drop bad adoptNS() in NetworkProcess.LaunchOnlyWhenNecessary
Summary: Drop bad adoptNS() in NetworkProcess.LaunchOnlyWhenNecessary
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-30 17:58 PDT by Chris Dumez
Modified: 2022-03-31 01:10 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.68 KB, patch)
2022-03-30 17:59 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2022-03-30 17:58:01 PDT
Drop bad adoptNS() in NetworkProcess.LaunchOnlyWhenNecessary. It is incorrect as the WKWebViewConfiguration.websiteDataStore getter is not ref'ing the store for us.
Comment 1 Chris Dumez 2022-03-30 17:59:20 PDT
Created attachment 456190 [details]
Patch
Comment 2 Alex Christensen 2022-03-30 20:25:06 PDT
If only there were some kind of language feature we could use that would do all this for us...
Comment 3 Darin Adler 2022-03-30 20:29:27 PDT
A sort of automatic release counting system?
Comment 4 Chris Dumez 2022-03-30 20:30:25 PDT
(In reply to Darin Adler from comment #3)
> A sort of automatic release counting system?

Do we have ARC enabled in the API tests?
Comment 5 Darin Adler 2022-03-30 20:40:52 PDT
I don’t think we do, but I think we could.

Thinking about it, it seems to me that calls to adoptNS should fail to compile under ARC; not important to be able to write it. Should be easy enough to make that change to RetainPtr.h if it’s not already done. Then the question wouldn’t need to be asked. If this was under ARC then it would fail to compile.

A while back, once WebKit had a new enough minimum supported version of iOS and macOS, I tried to start converting everything to ARC and it was hard to get it all done. My patches introduced storage leaks because presumably I created a reference cycle somewhere by accident. But probably it wasn’t as hard as I made it. It seems safer for the future if we could get over that hump and convert all our code.

Should be easy to switch over all our tests, though, and less risky than doing it in the framework code. So maybe we should do that really soon.
Comment 6 EWS 2022-03-31 01:08:42 PDT
Committed r292144 (249051@main): <https://commits.webkit.org/249051@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456190 [details].
Comment 7 Radar WebKit Bug Importer 2022-03-31 01:10:18 PDT
<rdar://problem/91091275>