REGRESSION(r267763) NetworkProcess never terminates
Created attachment 425168 [details] Patch
<rdar://problem/76124590>
Comment on attachment 425168 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425168&action=review > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1289 > + terminate(); Why are we calling terminate() instead of clearing the defaultNetworkProcess (which should deallocate the NetworkProcessProxy and cause the process to go away)? While both are fairly similar, I think clearing the default network process might be nicer for a few reasons: 1. This is what we used to do. When destroying the WebProcessPool, we would deallocate the NetworkProcessProxy and it would just go away (no termination involved) 2. There is a slight risk that there could be code somewhere with a RefPtr<> to a NetworkProcess (e.g. for some kind of transient operation) and terminate would abruptly interrupt the operation instead of letting it finish. 3. Youl will likely run more code doing a termination (due to termination handling) when simple destruction. > Source/WebKit/UIProcess/WebProcessPool.cpp:381 > + NetworkProcessProxy::defaultNetworkProcess()->terminate(); Ditto.
Created attachment 425178 [details] Patch
Comment on attachment 425178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425178&action=review r=me with naming change. > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:104 > +RefPtr<NetworkProcessProxy>& NetworkProcessProxy::defaultNetworkProcessPtr() I think the WebKit pattern is: defaultNetworkProcessIfExists() I don't like the Ptr() naming. When I saw it, it really wasn't clear to me what it meant. If you don't like defaultNetworkProcessIfExists(), I guess we could call this one defaultNetworkProcess() and the one below: ensureDefaultNetworkProcess().
Created attachment 425181 [details] Patch
Committed r275487: <https://commits.webkit.org/r275487> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425181 [details].