Skip to content

Commit

Permalink
jquery/attributes.html is flaky fail with StreamClientConnection ASSE…
Browse files Browse the repository at this point in the history
…RTION FAILED: !m_connection->isValid()

https://bugs.webkit.org/show_bug.cgi?id=261555
rdar://115494068

Reviewed by Antti Koivisto.

Fix two tests that create StreamClientConnection instances through
the JS IPC Testing API. The connections must be invalidated
if they are created.

Both allow and deny tests are run as normal JS business as usual, even
though the test runner automatically ends the deny test when GPUP
crashes. This means that the JS GC will clean up the stream connections
during the next tests, causing unrelated tests to fail with the
assertion.

Fix the allow test to actually test what it tests, e.g. to fail if
WebGPU is disabled.

* LayoutTests/ipc/restrictedendpoints/allow-access-webGPU-expected.txt:
* LayoutTests/ipc/restrictedendpoints/allow-access-webGPU.html:
* LayoutTests/ipc/restrictedendpoints/deny-access-webGPU.html:
* LayoutTests/resources/ipc.js:
(randomID):
(asyncFlush):
(syncFlush): Deleted.
* Source/WebKit/Shared/IPCTester.cpp:
(WebKit::IPCTester::asyncPing):
(WebKit::IPCTester::syncPing):
* Source/WebKit/Shared/IPCTester.h:
* Source/WebKit/Shared/IPCTester.messages.in:

Canonical link: https://commits.webkit.org/268086@main
  • Loading branch information
kkinnunen-apple committed Sep 18, 2023
1 parent ab8e3bc commit c0a9a95
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 122 deletions.
3 changes: 3 additions & 0 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -5285,6 +5285,9 @@ imported/w3c/web-platform-tests/css/motion/offset-path-shape-xywh-003.html [ Ima
# mac-only IPC test
ipc/webpageproxy-correctionpanel-no-crash.html [ Skip ]

# WebGPU settings need to be exposed through the test runner.
ipc/restrictedendpoints/allow-access-webGPU.html [ Pass Failure ]

# Restarted GPUP seems to crash.
webkit.org/b/239959 ipc/stream-sync-crash-no-timeout.html [ Skip ]

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
CONSOLE MESSAGE: PASS: Test runner did not detect GPUP crash.

108 changes: 56 additions & 52 deletions LayoutTests/ipc/restrictedendpoints/allow-access-webGPU.html
Original file line number Diff line number Diff line change
@@ -1,63 +1,67 @@
<!DOCTYPE html> <!-- webkit-test-runner [ IPCTestingAPIEnabled=true WebGPUEnabled=true ] -->
<title>Test that instantiating a remoteGPU is allowed if WebGPUEnabled</title>
<!DOCTYPE html> <!-- webkit-test-runner [ IPCTestingAPIEnabled=true IgnoreInvalidMessageWhenIPCTestingAPIEnabled=false WebGPUEnabled=true runSingly=true ] -->
<title>Test that instantiating a remoteGPU is allowed if WebGPUEnabled=true</title>
<script src="../../resources/ipc.js"></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<body>
<script>
if (window.IPC) {
function randomID() {
return Math.floor(Math.random() * 10000) + 1;
}

function sleep(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

let renderingBackendID = randomID();
let webgpuID = randomID();
let semaphore = IPC.createSemaphore();

let connectionIdentifier = IPC.createSharedMemory(0x1000);

let connectionPair = IPC.createConnectionPair();
let streamConnection = IPC.createStreamClientConnection(16);
let webgpuStreamConnection = IPC.createStreamClientConnection(16);
testRunner.dumpAsText();
if (window.IPC)
runTest();

function runTest() {
if (window.testRunner)
testRunner.waitUntilDone();
let rrbStreamConnection;
let rrbStreamConnectionHandle;
let webGPUStreamConnection;
let webGPUStreamConnectionHandle;
try {
[rrbStreamConnection, rrbStreamConnectionHandle] = IPC.createStreamClientConnection(16);
let renderingBackendID = randomIPCID();
IPC.sendMessage(
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRenderingBackend.name,
[
{ // creationParameters
type: 'RemoteRenderingBackendCreationParameters',
identifier: renderingBackendID,
pageProxyID: IPC.webPageProxyID,
pageID: IPC.pageID,
},
{ // connectionIdentifier
type: 'StreamServerConnectionHandle',
value: streamConnection[1],
}
]
);
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRenderingBackend.name,
[
{ // creationParameters
type: 'RemoteRenderingBackendCreationParameters',
identifier: renderingBackendID,
pageProxyID: IPC.webPageProxyID,
pageID: IPC.pageID,
},
{
type: 'StreamServerConnectionHandle',
value: rrbStreamConnectionHandle,
}
]);

let webGPUID = randomIPCID();
[webGPUStreamConnection, webGPUStreamConnectionHandle] = IPC.createStreamClientConnection(16);
var result = IPC.sendMessage(
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRemoteGPU.name,
[
{ type: 'uint64_t', value: webgpuID }, // identifier
{ type: 'uint64_t', value: renderingBackendID }, // renderingBackendIdentifier
{ type: 'StreamServerConnectionHandle', value: streamConnection[1] }, // stream
]
);
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRemoteGPU.name,
[
{ type: 'uint64_t', value: webGPUID }, // identifier
{ type: 'uint64_t', value: renderingBackendID }, // renderingBackendIdentifier
{ type: 'StreamServerConnectionHandle', value: webGPUStreamConnectionHandle }, // stream
]);

asyncFlush('GPU').then(() => {
testRunner.notifyDone();
});
} else {
testRunner.notifyDone();
const success = syncFlush('GPU');
if (!success)
console.log("FAIL: Failed to flush GPU process commands");
// FIXME: currently we cannot detect that GPUP crashes and test runner doesn't
// let the test continue if the subprocesses crash.
setTimeout(() => {
console.log("PASS: Test runner did not detect GPUP crash.");
if (window.testRunner)
testRunner.notifyDone()
}, 300);
} finally {
if (rrbStreamConnection)
rrbStreamConnection.invalidate();
if (webGPUStreamConnection)
webGPUStreamConnection.invalidate();
}
}
</script>
</body>
111 changes: 56 additions & 55 deletions LayoutTests/ipc/restrictedendpoints/deny-access-webGPU.html
Original file line number Diff line number Diff line change
@@ -1,66 +1,67 @@
<!DOCTYPE html> <!-- webkit-test-runner [ IPCTestingAPIEnabled=true IgnoreInvalidMessageWhenIPCTestingAPIEnabled=false WebGPUEnabled=false ] -->
<title>Test that instantiating a remoteGPU is allowed if WebGPUEnabled</title>
<!DOCTYPE html> <!-- webkit-test-runner [ IPCTestingAPIEnabled=true IgnoreInvalidMessageWhenIPCTestingAPIEnabled=false WebGPUEnabled=false runSingly=true ] -->
<title>Test that instantiating a remoteGPU is not allowed if WebGPUEnabled=false</title>
<script src="../../resources/ipc.js"></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<body>
<script>
testRunner.dumpAsText();

if (window.IPC) {
function randomID() {
return Math.floor(Math.random() * 10000) + 1;
}

function sleep(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

let renderingBackendID = randomID();
let webgpuID = randomID();
let semaphore = IPC.createSemaphore();

let connectionIdentifier = IPC.createSharedMemory(0x1000);

let connectionPair = IPC.createConnectionPair();
let streamConnection = IPC.createStreamClientConnection(16);
let webgpuStreamConnection = IPC.createStreamClientConnection(16);
console.log("HERE")
testRunner.dumpAsText();
if (window.IPC)
runTest();

function runTest() {
if (window.testRunner)
testRunner.waitUntilDone();
let rrbStreamConnection;
let rrbStreamConnectionHandle;
let webGPUStreamConnection;
let webGPUStreamConnectionHandle;
try {
[rrbStreamConnection, rrbStreamConnectionHandle] = IPC.createStreamClientConnection(16);
let renderingBackendID = randomIPCID();
IPC.sendMessage(
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRenderingBackend.name,
[
{ // creationParameters
type: 'RemoteRenderingBackendCreationParameters',
identifier: renderingBackendID,
pageProxyID: IPC.webPageProxyID,
pageID: IPC.pageID,
},
{ // connectionIdentifier
type: 'StreamServerConnectionHandle',
value: streamConnection[1],
}
]
);
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRenderingBackend.name,
[
{ // creationParameters
type: 'RemoteRenderingBackendCreationParameters',
identifier: renderingBackendID,
pageProxyID: IPC.webPageProxyID,
pageID: IPC.pageID,
},
{
type: 'StreamServerConnectionHandle',
value: rrbStreamConnectionHandle,
}
]);

let webGPUID = randomIPCID();
[webGPUStreamConnection, webGPUStreamConnectionHandle] = IPC.createStreamClientConnection(16);
var result = IPC.sendMessage(
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRemoteGPU.name,
[
{ type: 'uint64_t', value: webgpuID }, // identifier
{ type: 'uint64_t', value: renderingBackendID }, // renderingBackendIdentifier
{ type: 'StreamServerConnectionHandle', value: streamConnection[1] }, // stream
]
);
'GPU',
IPC.webPageProxyID,
IPC.messages.GPUConnectionToWebProcess_CreateRemoteGPU.name,
[
{ type: 'uint64_t', value: webGPUID }, // identifier
{ type: 'uint64_t', value: renderingBackendID }, // renderingBackendIdentifier
{ type: 'StreamServerConnectionHandle', value: webGPUStreamConnectionHandle }, // stream
]);

syncFlush('GPU');
console.log("PASS: Will pass if test runner ends the test prematurely.");

asyncFlush('GPU').then(() => {
console.log("Should have crashed!");
});
} else {
testRunner.notifyDone();
// FIXME: currently we cannot detect that GPUP crashes and test runner doesn't
// let the test continue if the subprocesses crash.
setTimeout(() => {
console.log("FAIL: test runner should have detected GPUP crash.");
if (window.testRunner)
testRunner.notifyDone()
}, 1000);
} finally {
if (rrbStreamConnection)
rrbStreamConnection.invalidate();
if (webGPUStreamConnection)
webGPUStreamConnection.invalidate();
}
}
</script>
</body>
2 changes: 1 addition & 1 deletion LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1740,6 +1740,7 @@ webkit.org/b/258142 fast/canvas/webgl/match-page-color-space-p3.html [ ImageOnly
webgl/webgl-via-metal-flag-on.html [ Skip ]
webgl/webgl-via-metal-flag-off.html [ Skip ]
webgl/webgl-backend-type.html [ Skip ]
ipc/restrictedendpoints/allow-access-webGPU.html [ Skip ]

# Failing with ANGLE backend
fast/canvas/webgl/draw-elements-out-of-bounds-uint-index.html [ Failure ]
Expand Down Expand Up @@ -3329,7 +3330,6 @@ webkit.org/b/236298 fast/text/strikethrough-int.html [ ImageOnlyFailure ]

http/tests/webgpu/webgpu/api/operation/memory_sync/buffer/multiple_buffers.html [ Skip ]


# NetworkStorageSession::deleteCookies() doesn't obey partitioning for glib ports.
http/wpt/clear-site-data/partitioning.html [ Skip ]

Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/platform/mac-wk1/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2530,7 +2530,7 @@ imported/w3c/web-platform-tests/geolocation-API/permission.https.html [ WontFix
http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ]

# IPC is for WK2
ipc/shared-video-frame-size.html [ Skip ]
ipc [ WontFix ]

# Displays blank on WK1
fullscreen/fullscreen-iframe-navigation.html [ Skip ]
Expand Down
14 changes: 9 additions & 5 deletions LayoutTests/resources/ipc.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
function randomIPCID() {
return Math.floor(Math.random() * 10000) + 1;
}

function asyncFlush(processTarget) {
if (!IPC.processTargets.includes(processTarget))
throw Error("Invalid processTarget passed to asyncFlush")
return IPC.sendMessage(processTarget, 0, IPC.messages.IPCTester_AsyncPing.name, [])
return IPC.sendMessage(processTarget, 0, IPC.messages.IPCTester_AsyncPing.name, [{type: "uint32_t", value: 88}]);
}

function syncFlush(processTarget) {
if (!IPC.processTargets.includes(processTarget))
throw Error("Invalid processTarget passed to syncFlush")
return new Promise((resolve) => {
IPC.sendSyncMessage(processTarget, 0, IPC.messages.IPCTester_SyncPing.name, 1000, []);
resolve();
})

let reply = IPC.sendSyncMessage(processTarget, 0, IPC.messages.IPCTester_SyncPing.name, 1000, [{type: "uint32_t", value: 77}]);
const firstResult = reply.arguments[0];
return firstResult.type == "uint32_t" && firstResult.value == 78;
}
8 changes: 4 additions & 4 deletions Source/WebKit/Shared/IPCTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,14 @@ void IPCTester::releaseConnectionTester(IPCConnectionTesterIdentifier identifier
completionHandler();
}

void IPCTester::asyncPing(IPC::Connection&, CompletionHandler<void()>&& completionHandler)
void IPCTester::asyncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&& completionHandler)
{
completionHandler();
completionHandler(value + 1);
}

void IPCTester::syncPing(IPC::Connection&, CompletionHandler<void()>&& completionHandler)
void IPCTester::syncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&& completionHandler)
{
completionHandler();
completionHandler(value + 1);
}

void IPCTester::stopIfNeeded()
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/Shared/IPCTester.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ class IPCTester final : public IPC::MessageReceiver {
void sendSameSemaphoreBack(IPC::Connection&, IPC::Semaphore&&);
void sendSemaphoreBackAndSignalProtocol(IPC::Connection&, IPC::Semaphore&&);
void sendAsyncMessageToReceiver(IPC::Connection&, uint32_t);
void asyncPing(IPC::Connection&, CompletionHandler<void()>&&);
void syncPing(IPC::Connection&, CompletionHandler<void()>&&);
void asyncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&&);
void syncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&&);

void stopIfNeeded();

Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/Shared/IPCTester.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ messages -> IPCTester NotRefCounted {
SendSameSemaphoreBack(IPC::Semaphore semaphore)
SendSemaphoreBackAndSignalProtocol(IPC::Semaphore semaphore)

AsyncPing() -> ()
SyncPing() -> () Synchronous
AsyncPing(uint32_t value) -> (uint32_t nextValue)
SyncPing(uint32_t value) -> (uint32_t nextValue) Synchronous

SendAsyncMessageToReceiver(uint32_t arg0)
}
Expand Down

0 comments on commit c0a9a95

Please sign in to comment.