Skip to content

Commit

Permalink
RemoteLayerTreeDrawingAreaProxy needs to hold use-count for IOSurface…
Browse files Browse the repository at this point in the history
…s until CA commits.

https://bugs.webkit.org/show_bug.cgi?id=260374
<rdar://112269436>

Reviewed by Simon Fraser.

CoreAnimation doesn't guarantee to have marked the IOSurface as in-use until the transaction is committed, not once the layer mutation happens.

This code was removed in 266557@main (rdar://111986083) with the belief that it was no longer necessary.

This adds it back, but replaces kCATransactionPhasePostSynchronize with kCATransactionPhasePostCommit.

Waiting for the synchronize phase was a performance regression, and is unnecessary with the latest CoreAnimation, but we
still need to wait for the commit to happen. WebKit doesn't explicitly commit the CoreAnimation transaction, so the implicit
transaction commit might happen significantly later, and we need to ensure that we continue to mark any IOSurfaces as in-use (by
retaining the wrapping mach_port) in the interim.

* Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):

Canonical link: https://commits.webkit.org/267204@main
  • Loading branch information
mattwoodrow committed Aug 23, 2023
1 parent 6c01e61 commit 346fdad
Showing 1 changed file with 19 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,27 @@

void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(IPC::Connection& connection, const Vector<std::pair<RemoteLayerTreeTransaction, RemoteScrollingCoordinatorTransaction>>& transactions)
{
Vector<MachSendRight> sendRights;
for (auto& transaction : transactions) {
// commitLayerTreeTransaction consumes the incoming buffers, so we need to grab them first.
for (auto& [layerID, properties] : transaction.first.changedLayerProperties()) {
const auto* backingStoreProperties = properties->backingStoreOrProperties.properties.get();
if (!backingStoreProperties)
continue;
if (const auto& backendHandle = backingStoreProperties->bufferHandle()) {
if (const auto* sendRight = std::get_if<MachSendRight>(&backendHandle.value()))
sendRights.append(*sendRight);
}
}
}

for (auto& transaction : transactions)
commitLayerTreeTransaction(connection, transaction.first, transaction.second);

// Keep IOSurface send rights alive until the transaction is commited, otherwise we will
// prematurely drop the only reference to them, and `inUse` will be wrong for a brief window.
if (!sendRights.isEmpty())
[CATransaction addCommitHandler:[sendRights = WTFMove(sendRights)]() { } forPhase:kCATransactionPhasePostCommit];
}

void RemoteLayerTreeDrawingAreaProxy::commitLayerTreeTransaction(IPC::Connection& connection, const RemoteLayerTreeTransaction& layerTreeTransaction, const RemoteScrollingCoordinatorTransaction& scrollingTreeTransaction)
Expand Down

0 comments on commit 346fdad

Please sign in to comment.