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+

Description Alex Christensen 2016-02-29 12:02:55 PST
Don't call NetworkProcess::singleton from WebProcess when using NetworkSession
Comment 1 Alex Christensen 2016-02-29 12:08:55 PST
Created attachment 272509 [details]
Patch
Comment 2 Alex Christensen 2016-02-29 12:33:36 PST
Created attachment 272511 [details]
Patch
Comment 3 Alex Christensen 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.
Comment 4 Alex Christensen 2016-02-29 12:47:52 PST
http://trac.webkit.org/changeset/197362
Comment 5 Darin Adler 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.
Comment 6 Alex Christensen 2016-02-29 15:40:01 PST
Sure enough.  This regressed WebKit2CustomProtocolsTest when using NetworkSession for that very reason.  Will fix.