Bug 154829 - Don't call NetworkProcess::singleton from WebProcess when using NetworkSession
Summary: Don't call NetworkProcess::singleton from WebProcess when using NetworkSession
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-29 12:02 PST by Alex Christensen
Modified: 2016-02-29 15:40 PST (History)
2 users (show)

See Also:

Attachments
Patch (4.80 KB, patch)
2016-02-29 12:08 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (4.86 KB, patch)
2016-02-29 12:33 PST, Alex Christensen 		andersca: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 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.