Bug 154829

Summary: Don't call NetworkProcess::singleton from WebProcess when using NetworkSession
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, beidson
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch andersca: review+

Alex Christensen
Reported 2016-02-29 12:02:55 PST
Don't call NetworkProcess::singleton from WebProcess when using NetworkSession
Attachments
Patch (4.80 KB, patch)
2016-02-29 12:08 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (4.86 KB, patch)
2016-02-29 12:33 PST, Alex Christensen 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2016-02-29 12:08:55 PST
Created attachment 272509 [details] Patch
Alex Christensen
Comment 2 2016-02-29 12:33:36 PST
Created attachment 272511 [details] Patch
Alex Christensen
Comment 3 2016-02-29 12:43:34 PST
Comment on attachment 272511 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272511&action=review > Source/WebKit2/WebProcess/WebProcess.cpp:194 > + NetworkSession::setCustomProtocolManager(nullptr); This doesn't do anything. Will land without it.
Alex Christensen
Comment 4 2016-02-29 12:47:52 PST
http://trac.webkit.org/changeset/197362
Darin Adler
Comment 5 2016-02-29 15:36:04 PST
Comment on attachment 272511 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272511&action=review > Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:221 > + NeverDestroyed<RefPtr<CustomProtocolManager>> gCustomProtocolManager; You really need “static” here. I’m not sure exactly what this will do without the static, but nothing good. I think in practice it means we will leak all the CustomProtocolManager objects and always have null for the protocol manager. Also, I don’t think you should name this local variable with a "g" prefix.
Alex Christensen
Comment 6 2016-02-29 15:40:01 PST
Sure enough. This regressed WebKit2CustomProtocolsTest when using NetworkSession for that very reason. Will fix.
Note You need to log in before you can comment on or make changes to this bug.