Bug 210142 - Simplify and fortify network getNetworkProcessConnection and getGPUProcessConnection
Summary: Simplify and fortify network getNetworkProcessConnection and getGPUProcessCon...
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: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-07 12:40 PDT by Alex Christensen
Modified: 2020-04-07 17:04 PDT (History)
3 users (show)

See Also:


Attachments
Patch (20.17 KB, patch)
2020-04-07 12:50 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (20.47 KB, patch)
2020-04-07 13:14 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (23.42 KB, patch)
2020-04-07 14:14 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2020-04-07 12:40:45 PDT
Simplify and fortify network getNetworkProcessConnection and getGPUProcessConnection
Comment 1 Alex Christensen 2020-04-07 12:50:03 PDT
Created attachment 395726 [details]
Patch
Comment 2 youenn fablet 2020-04-07 12:58:15 PDT
Comment on attachment 395726 [details]
Patch

Test?

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

> Source/WebKit/ChangeLog:20
> +        I do the same changes to NetworkProcess and GPUProcess because they are intended to be the same.  The latter is based on the former.

If there is a lot of common code between the two, maybe we should refactor the code to share more between the two.

> Source/WebKit/UIProcess/WebProcessPool.cpp:745
> +    m_networkProcess->getNetworkProcessConnection(webProcessProxy, [this, protectedThis = makeRef(*this), webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {

weakThis instead of protectedThis might be better.

> Source/WebKit/UIProcess/WebProcessPool.cpp:749
> +            m_networkProcess->getNetworkProcessConnection(*webProcessProxy, WTFMove(reply));

return to not have any else

> Source/WebKit/UIProcess/WebProcessPool.cpp:767
> +    GPUProcessProxy::singleton().getGPUProcessConnection(webProcessProxy, [this, protectedThis = makeRef(*this), webProcessProxy = makeWeakPtr(webProcessProxy), reply = WTFMove(reply)] (auto& connectionInfo) mutable {

Ditto.

> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:186
> +    sendWithAsyncReply(Messages::GPUProcess::CreateGPUConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply), activity = throttler().foregroundActivity("GPUProcessProxy::getGPUProcessConnection"_s)](auto&& connectionIdentifier) mutable {

A background activity might be better here than a foreground activity, and in sync with release logging.

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:141
> +    sendWithAsyncReply(Messages::NetworkProcess::CreateNetworkConnectionToWebProcess { webProcessProxy.coreProcessIdentifier(), webProcessProxy.sessionID() }, [this, weakThis = makeWeakPtr(*this), reply = WTFMove(reply), activity = throttler().backgroundActivity("NetworkProcessProxy::getNetworkProcessConnection"_s)](auto&& connectionIdentifier, HTTPCookieAcceptPolicy cookieAcceptPolicy) mutable {

s/HTTPCookieAcceptPolicy/auto
Comment 3 Alex Christensen 2020-04-07 13:14:47 PDT
Created attachment 395728 [details]
Patch
Comment 4 Alex Christensen 2020-04-07 14:14:31 PDT
Created attachment 395736 [details]
Patch
Comment 5 EWS 2020-04-07 15:08:00 PDT
Committed r259673: <https://trac.webkit.org/changeset/259673>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395736 [details].
Comment 6 Radar WebKit Bug Importer 2020-04-07 15:08:14 PDT
<rdar://problem/61412490>