Bug 193484 - Remove more NetworkProcess::singleton use
Summary: Remove more NetworkProcess::singleton use
Status: NEW
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: 2019-01-15 20:10 PST by Alex Christensen
Modified: 2019-01-15 20:58 PST (History)
2 users (show)

See Also:


Attachments
Patch (8.18 KB, patch)
2019-01-15 20:13 PST, Alex Christensen
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-01-15 20:10:20 PST
Remove more NetworkProcess::singleton use
Comment 1 Alex Christensen 2019-01-15 20:13:40 PST
Created attachment 359251 [details]
Patch
Comment 2 Geoffrey Garen 2019-01-15 20:40:12 PST
Comment on attachment 359251 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359251&action=review

r=me

> Source/WebKit/NetworkProcess/CustomProtocols/Cocoa/LegacyCustomProtocolManagerCocoa.mm:51
> +void LegacyCustomProtocolManager::networkProcessCreated(NetworkProcess& networkProcess)
> +{
> +    newestNetworkProcess() = &networkProcess;
> +}

I think we expect clients that use LegacyCustomProtocolManager never to make more than one NetworkProcess, and we expect clients that make more than one NetworkProcess never to use LegacyCustomProtocolManager. Is that right?

If so, I think it would be clearer to remember the *first* NetworkProcess created, and ASSERT that, if another one is created, LegacyCustomProtocolManager has no registered protocols.
Comment 3 Alex Christensen 2019-01-15 20:58:28 PST
(In reply to Geoffrey Garen from comment #2)
> Comment on attachment 359251 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359251&action=review
> 
> r=me
> 
> > Source/WebKit/NetworkProcess/CustomProtocols/Cocoa/LegacyCustomProtocolManagerCocoa.mm:51
> > +void LegacyCustomProtocolManager::networkProcessCreated(NetworkProcess& networkProcess)
> > +{
> > +    newestNetworkProcess() = &networkProcess;
> > +}
> 
> I think we expect clients that use LegacyCustomProtocolManager never to make
> more than one NetworkProcess, and we expect clients that make more than one
> NetworkProcess never to use LegacyCustomProtocolManager. Is that right?
That will be right.
> 
> If so, I think it would be clearer to remember the *first* NetworkProcess
> created, and ASSERT that, if another one is created,
> LegacyCustomProtocolManager has no registered protocols.
Done!

http://trac.webkit.org/r240030