Skip to content

Commit

Permalink
Web process sends redundant information when establishing GPU process…
Browse files Browse the repository at this point in the history
… connection

https://bugs.webkit.org/show_bug.cgi?id=274307
rdar://128265618

Reviewed by Chris Dumez.

Reland:
Use HAVE(TASK_IDENTITY_TOKEN) instead of PLATFORM(COCOA)
to guard ASSERT(m_processIdentity), as the iOS Family simulators do
not have the task identity token.

Web process would send
 - the process identity for resource attribution
 - settings object

The settings were previously changled to be filled by the privileged
UI process. Remove this altogether, as it's insecure to let the WP
select anything here.

The process identity does not change once the WP is created. Thus
send this during WP initialization.

Moves the WebProcessProxy message sends to the WebProcessProxy.

* Source/WebKit/Scripts/webkit/messages.py:
(types_that_must_be_moved):
* Source/WebKit/UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::createGPUProcessConnection):
* Source/WebKit/UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::initializeIdentity):
(WebKit::WebProcessProxy::createGPUProcessConnection):
* Source/WebKit/UIProcess/WebProcessProxy.h:
* Source/WebKit/UIProcess/WebProcessProxy.messages.in:
* Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:
(WebKit::GPUProcessConnection::create):
(WebKit::getGPUProcessConnectionParameters): Deleted.
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::initializeWebProcess):

Canonical link: https://commits.webkit.org/279438@main
  • Loading branch information
kkinnunen-apple committed May 29, 2024
1 parent 5ab485a commit 4420d02
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 44 deletions.
1 change: 1 addition & 0 deletions Source/WebKit/Scripts/webkit/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ def types_that_must_be_moved():
'WebKit::WebGPU::ExternalTextureDescriptor',
'WebCore::GraphicsContextGL::ExternalImageSource',
'WebCore::GraphicsContextGL::ExternalSyncSource',
'WebCore::ProcessIdentity',
'WebKit::ConsumerSharedCARingBufferHandle',
'WebKit::GPUProcessConnectionParameters',
'WebKit::ModelProcessConnectionParameters',
Expand Down
9 changes: 1 addition & 8 deletions Source/WebKit/UIProcess/WebProcessPool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -555,16 +555,9 @@ void WebProcessPool::gpuProcessExited(ProcessID identifier, ProcessTerminationRe

void WebProcessPool::createGPUProcessConnection(WebProcessProxy& webProcessProxy, IPC::Connection::Handle&& connectionIdentifier, WebKit::GPUProcessConnectionParameters&& parameters)
{
#if ENABLE(IPC_TESTING_API)
parameters.ignoreInvalidMessageForTesting = webProcessProxy.ignoreInvalidMessageForTesting();
#endif

#if HAVE(AUDIT_TOKEN)
parameters.presentingApplicationAuditToken = configuration().presentingApplicationProcessToken();
#endif

parameters.isLockdownModeEnabled = webProcessProxy.lockdownMode() == WebProcessProxy::LockdownMode::Enabled;

ensureProtectedGPUProcess()->createGPUProcessConnection(webProcessProxy, WTFMove(connectionIdentifier), WTFMove(parameters));
}
#endif // ENABLE(GPU_PROCESS)
Expand Down Expand Up @@ -1011,7 +1004,7 @@ void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDa
if (websiteDataStore)
parameters.websiteDataStoreParameters = webProcessDataStoreParameters(process, *websiteDataStore);

process.send(Messages::WebProcess::InitializeWebProcess(WTFMove(parameters)), 0);
process.initializeWebProcess(WTFMove(parameters));

#if HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK)
setMediaAccessibilityPreferences(process);
Expand Down
21 changes: 19 additions & 2 deletions Source/WebKit/UIProcess/WebProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "APIUIClient.h"
#include "AuthenticatorManager.h"
#include "DownloadProxyMap.h"
#include "GPUProcessConnectionParameters.h"
#include "GoToBackForwardItemParameters.h"
#include "LoadParameters.h"
#include "Logging.h"
Expand Down Expand Up @@ -463,6 +464,14 @@ void WebProcessProxy::updateRegistrationWithDataStore()
}
}

void WebProcessProxy::initializeWebProcess(WebProcessCreationParameters&& parameters)
{
sendWithAsyncReply(Messages::WebProcess::InitializeWebProcess(WTFMove(parameters)), [weakThis = WeakPtr { *this }] (ProcessIdentity processIdentity) {
if (RefPtr protectedThis = weakThis.get())
protectedThis->m_processIdentity = WTFMove(processIdentity);
}, 0);
}

void WebProcessProxy::initializePreferencesForGPUAndNetworkProcesses(const WebPageProxy& page)
{
#if ENABLE(GPU_PROCESS)
Expand Down Expand Up @@ -1063,13 +1072,21 @@ void WebProcessProxy::getNetworkProcessConnection(CompletionHandler<void(Network

#if ENABLE(GPU_PROCESS)

void WebProcessProxy::createGPUProcessConnection(IPC::Connection::Handle&& connectionIdentifier, WebKit::GPUProcessConnectionParameters&& parameters)
void WebProcessProxy::createGPUProcessConnection(IPC::Connection::Handle&& connectionIdentifier)
{
WebKit::GPUProcessConnectionParameters parameters;
#if HAVE(TASK_IDENTITY_TOKEN)
ASSERT(m_processIdentity);
#endif
parameters.webProcessIdentity = m_processIdentity;
auto& gpuPreferences = preferencesForGPUProcess();
ASSERT(gpuPreferences);
if (gpuPreferences)
parameters.preferences = *gpuPreferences;

#if ENABLE(IPC_TESTING_API)
parameters.ignoreInvalidMessageForTesting = ignoreInvalidMessageForTesting();
#endif
parameters.isLockdownModeEnabled = lockdownMode() == WebProcessProxy::LockdownMode::Enabled;
protectedProcessPool()->createGPUProcessConnection(*this, WTFMove(connectionIdentifier), WTFMove(parameters));
}

Expand Down
7 changes: 6 additions & 1 deletion Source/WebKit/UIProcess/WebProcessProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <WebCore/MediaProducer.h>
#include <WebCore/PageIdentifier.h>
#include <WebCore/ProcessIdentifier.h>
#include <WebCore/ProcessIdentity.h>
#include <WebCore/RegistrableDomain.h>
#include <WebCore/SharedStringHash.h>
#include <pal/SessionID.h>
Expand Down Expand Up @@ -91,6 +92,7 @@ class ResourceRequest;
struct NotificationData;
struct PluginInfo;
struct PrewarmInformation;
struct WebProcessCreationParameters;
class SecurityOriginData;
enum class PermissionName : uint8_t;
enum class ThirdPartyCookieBlockingMode : uint8_t;
Expand Down Expand Up @@ -176,6 +178,8 @@ class WebProcessProxy : public AuxiliaryProcessProxy {
static void forWebPagesWithOrigin(PAL::SessionID, const WebCore::SecurityOriginData&, const Function<void(WebPageProxy&)>&);
static Vector<std::pair<WebCore::ProcessIdentifier, WebCore::RegistrableDomain>> allowedFirstPartiesForCookies();

void initializeWebProcess(WebProcessCreationParameters&&);

WebConnection* webConnection() const { return m_webConnection.get(); }
RefPtr<WebConnection> protectedWebConnection() const { return m_webConnection; }

Expand Down Expand Up @@ -575,7 +579,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy {
void getNetworkProcessConnection(CompletionHandler<void(NetworkProcessConnectionInfo&&)>&&);

#if ENABLE(GPU_PROCESS)
void createGPUProcessConnection(IPC::Connection::Handle&&, WebKit::GPUProcessConnectionParameters&&);
void createGPUProcessConnection(IPC::Connection::Handle&&);
#endif

#if ENABLE(MODEL_PROCESS)
Expand Down Expand Up @@ -792,6 +796,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy {
Seconds m_totalForegroundTime;
Seconds m_totalBackgroundTime;
Seconds m_totalSuspendedTime;
WebCore::ProcessIdentity m_processIdentity;
};

WTF::TextStream& operator<<(WTF::TextStream&, const WebProcessProxy&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebProcessProxy.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ messages -> WebProcessProxy LegacyReceiver {
GetNetworkProcessConnection() -> (struct WebKit::NetworkProcessConnectionInfo connectionInfo) Synchronous

#if ENABLE(GPU_PROCESS)
CreateGPUProcessConnection(IPC::ConnectionHandle connectionHandle, struct WebKit::GPUProcessConnectionParameters parameters) AllowedWhenWaitingForSyncReply
CreateGPUProcessConnection(IPC::ConnectionHandle connectionHandle) AllowedWhenWaitingForSyncReply
#endif

#if ENABLE(MODEL_PROCESS)
Expand Down
29 changes: 4 additions & 25 deletions Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "GPUConnectionToWebProcessMessages.h"
#include "GPUProcessConnectionInfo.h"
#include "GPUProcessConnectionMessages.h"
#include "GPUProcessConnectionParameters.h"
#include "LibWebRTCCodecs.h"
#include "LibWebRTCCodecsMessages.h"
#include "Logging.h"
Expand Down Expand Up @@ -103,35 +102,15 @@
namespace WebKit {
using namespace WebCore;

static GPUProcessConnectionParameters getGPUProcessConnectionParameters()
Ref<GPUProcessConnection> GPUProcessConnection::create(Ref<IPC::Connection>&& connection)
{
GPUProcessConnectionParameters parameters;
#if PLATFORM(COCOA)
parameters.webProcessIdentity = ProcessIdentity { ProcessIdentity::CurrentProcess };
#endif
return parameters;
}

RefPtr<GPUProcessConnection> GPUProcessConnection::create(IPC::Connection& parentConnection)
{
auto connectionIdentifiers = IPC::Connection::createConnectionIdentifierPair();
if (!connectionIdentifiers)
return nullptr;

RELEASE_ASSERT_WITH_MESSAGE(WebProcess::singleton().hasEverHadAnyWebPages(), "GPUProcess preferences come from the pages");
parentConnection.send(Messages::WebProcessProxy::CreateGPUProcessConnection(WTFMove(connectionIdentifiers->client), getGPUProcessConnectionParameters()), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);

auto instance = adoptRef(*new GPUProcessConnection(WTFMove(connectionIdentifiers->server)));
#if ENABLE(IPC_TESTING_API)
if (parentConnection.ignoreInvalidMessageForTesting())
instance->connection().setIgnoreInvalidMessageForTesting();
#endif
Ref instance = adoptRef(*new GPUProcessConnection(WTFMove(connection)));
RELEASE_LOG(Process, "GPUProcessConnection::create - %p", instance.ptr());
return instance;
}

GPUProcessConnection::GPUProcessConnection(IPC::Connection::Identifier&& connectionIdentifier)
: m_connection(IPC::Connection::createServerConnection(connectionIdentifier))
GPUProcessConnection::GPUProcessConnection(Ref<IPC::Connection>&& connection)
: m_connection(WTFMove(connection))
{
m_connection->open(*this);

Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/GPU/GPUProcessConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class RemoteVideoFrameObjectHeapProxy;

class GPUProcessConnection : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<GPUProcessConnection>, public IPC::Connection::Client {
public:
static RefPtr<GPUProcessConnection> create(IPC::Connection& parentConnection);
static Ref<GPUProcessConnection> create(Ref<IPC::Connection>&&);
~GPUProcessConnection();

IPC::Connection& connection() { return m_connection.get(); }
Expand Down Expand Up @@ -117,7 +117,7 @@ class GPUProcessConnection : public ThreadSafeRefCountedAndCanMakeThreadSafeWeak

static constexpr Seconds defaultTimeout = 3_s;
private:
GPUProcessConnection(IPC::Connection::Identifier&&);
GPUProcessConnection(Ref<IPC::Connection>&&);
bool waitForDidInitialize();
void invalidate();

Expand Down
15 changes: 12 additions & 3 deletions Source/WebKit/WebProcess/WebProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,11 @@ static void scheduleLogMemoryStatistics(LogMemoryStatisticsReason reason)
});
}

void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters)
void WebProcess::initializeWebProcess(WebProcessCreationParameters&& parameters, CompletionHandler<void(ProcessIdentity)>&& completionHandler)
{
TraceScope traceScope(InitializeWebProcessStart, InitializeWebProcessEnd);
// Reply immediately so that the identity is available as soon as possible.
completionHandler(ProcessIdentity { ProcessIdentity::CurrentProcess });

ASSERT(m_pageMap.isEmpty());

Expand Down Expand Up @@ -1370,9 +1372,16 @@ GPUProcessConnection& WebProcess::ensureGPUProcessConnection()

// If we've lost our connection to the GPU process (e.g. it crashed) try to re-establish it.
if (!m_gpuProcessConnection) {
m_gpuProcessConnection = GPUProcessConnection::create(Ref { *parentProcessConnection() });
if (!m_gpuProcessConnection)
auto connectionIdentifiers = IPC::Connection::createConnectionIdentifierPair();
if (!connectionIdentifiers)
CRASH();
protectedParentProcessConnection()->send(Messages::WebProcessProxy::CreateGPUProcessConnection(WTFMove(connectionIdentifiers->client)), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
Ref gpuConnection = IPC::Connection::createServerConnection(WTFMove(connectionIdentifiers->server));
#if ENABLE(IPC_TESTING_API)
if (gpuConnection->ignoreInvalidMessageForTesting())
gpuConnection->setIgnoreInvalidMessageForTesting();
#endif
m_gpuProcessConnection = GPUProcessConnection::create(WTFMove(gpuConnection));
for (auto& page : m_pageMap.values()) {
// If page is null, then it is currently being constructed.
if (page)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ class WebProcess : public AuxiliaryProcess
WebProcess();
~WebProcess();

void initializeWebProcess(WebProcessCreationParameters&&);
void initializeWebProcess(WebProcessCreationParameters&&, CompletionHandler<void(WebCore::ProcessIdentity)>&&);
void platformInitializeWebProcess(WebProcessCreationParameters&);
void setWebsiteDataStoreParameters(WebProcessDataStoreParameters&&);
void platformSetWebsiteDataStoreParameters(WebProcessDataStoreParameters&&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebProcess.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

messages -> WebProcess LegacyReceiver NotRefCounted {
InitializeWebProcess(struct WebKit::WebProcessCreationParameters processCreationParameters)
InitializeWebProcess(struct WebKit::WebProcessCreationParameters processCreationParameters) -> (WebCore::ProcessIdentity processIdentity)
SetWebsiteDataStoreParameters(struct WebKit::WebProcessDataStoreParameters parameters)

CreateWebPage(WebCore::PageIdentifier newPageID, struct WebKit::WebPageCreationParameters pageCreationParameters)
Expand Down

0 comments on commit 4420d02

Please sign in to comment.