corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1749526 - Process partial WebRender transactions instead of discarding them partway. r=gfx-reviewers,bradwerth

Browse Source 
When we encounter an error updating resources, or processing parent
commands, we actually bail out midway. This is problematic because some
resource updates and commands depend on follow-ups to release resources
and such. Also, when the next display list update comes in, we may now
have an incomplete resource set and see display errors / crashes that
may not be easily traceable to the previous failure.

This patch makes us always finish processing a transaction, even if we
encountered errors. This will put us in a more consistent state at
least. It adds new asserts to try to catch these issues sooner and in
automation. It also ensures we do a print to the critical log in all of
these cases so we know this issue was encountered.

Differential Revision: https://phabricator.services.mozilla.com/D135613
This commit is contained in:
Andrew Osmond
2022-01-13 18:38:00 +00:00
parent c9b1c9b1e2
commit b38dfffb91
2 changed files with 119 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

200
gfx/layers/wr/WebRenderBridgeParent.cpp
View File

@@ -245,19 +245,31 @@ class WebRenderBridgeParent::ScheduleSharedSurfaceRelease final
explicit ScheduleSharedSurfaceRelease(WebRenderBridgeParent* aWrBridge)
: mWrBridge(aWrBridge), mSurfaces(20) {}
~ScheduleSharedSurfaceRelease() override {
if (!mSurfaces.IsEmpty()) {
MOZ_ASSERT_UNREACHABLE("Unreleased surfaces!");
gfxCriticalNote << "ScheduleSharedSurfaceRelease destroyed non-empty";
NotifyInternal(/* aFromCheckpoint */ false);
}
}
void Add(const wr::ImageKey& aKey, const wr::ExternalImageId& aId) {
mSurfaces.AppendElement(wr::ExternalImageKeyPair{aKey, aId});
}
void Notify(wr::Checkpoint) override {
CompositorThread()->Dispatch(
NewRunnableMethod<nsTArray<wr::ExternalImageKeyPair>>(
"ObserveSharedSurfaceRelease", mWrBridge,
&WebRenderBridgeParent::ObserveSharedSurfaceRelease,
std::move(mSurfaces)));
NotifyInternal(/* aFromCheckpoint */ true);
}
private:
void NotifyInternal(bool aFromCheckpoint) {
CompositorThread()->Dispatch(
NewRunnableMethod<nsTArray<wr::ExternalImageKeyPair>, bool>(
"ObserveSharedSurfaceRelease", mWrBridge,
&WebRenderBridgeParent::ObserveSharedSurfaceRelease,
std::move(mSurfaces), aFromCheckpoint));
}
RefPtr<WebRenderBridgeParent> mWrBridge;
nsTArray<wr::ExternalImageKeyPair> mSurfaces;
};
@@ -448,6 +460,8 @@ static bool ReadRawFont(const OpAddRawFont& aOp, wr::ShmSegmentsReader& aReader,
Maybe<Range<uint8_t>> ptr =
aReader.GetReadPointerOrCopy(aOp.bytes(), sourceBytes);
if (ptr.isNothing()) {
gfxCriticalNote << "No read pointer from reader for sanitizing font "
<< aOp.key().mHandle;
return false;
}
Range<uint8_t>& source = ptr.ref();
@@ -490,6 +504,7 @@ bool WebRenderBridgeParent::UpdateResources(
}
}
bool success = true;
for (const auto& cmd : aResourceUpdates) {
switch (cmd.type()) {
case OpUpdateResource::TOpAddImage: {
@@ -500,11 +515,12 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> bytes;
if (!reader.Read(op.bytes(), bytes)) {
if (reader.Read(op.bytes(), bytes)) {
aUpdates.AddImage(op.key(), op.descriptor(), bytes);
} else {
gfxCriticalNote << "TOpAddImage failed";
return false;
success = false;
}
aUpdates.AddImage(op.key(), op.descriptor(), bytes);
break;
}
case OpUpdateResource::TOpUpdateImage: {
@@ -515,11 +531,12 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> bytes;
if (!reader.Read(op.bytes(), bytes)) {
if (reader.Read(op.bytes(), bytes)) {
aUpdates.UpdateImageBuffer(op.key(), op.descriptor(), bytes);
} else {
gfxCriticalNote << "TOpUpdateImage failed";
return false;
success = false;
}
aUpdates.UpdateImageBuffer(op.key(), op.descriptor(), bytes);
break;
}
case OpUpdateResource::TOpAddBlobImage: {
@@ -530,13 +547,14 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> bytes;
if (!reader.Read(op.bytes(), bytes)) {
if (reader.Read(op.bytes(), bytes)) {
aUpdates.AddBlobImage(op.key(), op.descriptor(), mBlobTileSize, bytes,
wr::ToDeviceIntRect(op.visibleRect()));
} else {
gfxCriticalNote << "TOpAddBlobImage failed";
return false;
success = false;
}
aUpdates.AddBlobImage(op.key(), op.descriptor(), mBlobTileSize, bytes,
wr::ToDeviceIntRect(op.visibleRect()));
break;
}
case OpUpdateResource::TOpUpdateBlobImage: {
@@ -547,13 +565,14 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> bytes;
if (!reader.Read(op.bytes(), bytes)) {
if (reader.Read(op.bytes(), bytes)) {
aUpdates.UpdateBlobImage(op.key(), op.descriptor(), bytes,
wr::ToDeviceIntRect(op.visibleRect()),
wr::ToLayoutIntRect(op.dirtyRect()));
} else {
gfxCriticalNote << "TOpUpdateBlobImage failed";
return false;
success = false;
}
aUpdates.UpdateBlobImage(op.key(), op.descriptor(), bytes,
wr::ToDeviceIntRect(op.visibleRect()),
wr::ToLayoutIntRect(op.dirtyRect()));
break;
}
case OpUpdateResource::TOpSetBlobImageVisibleArea: {
@@ -576,7 +595,7 @@ bool WebRenderBridgeParent::UpdateResources(
const auto& op = cmd.get_OpAddSharedExternalImage();
// gfxCriticalNote is called on error
if (!AddSharedExternalImage(op.externalImageId(), op.key(), aUpdates)) {
return false;
success = false;
}
break;
}
@@ -587,7 +606,7 @@ bool WebRenderBridgeParent::UpdateResources(
// gfxCriticalNote is called on error
if (!PushExternalImageForTexture(op.externalImageId(), op.key(),
texture, op.isUpdate(), aUpdates)) {
return false;
success = false;
}
break;
}
@@ -603,14 +622,13 @@ bool WebRenderBridgeParent::UpdateResources(
if (!UpdateSharedExternalImage(op.externalImageId(), op.key(),
op.dirtyRect(), aUpdates,
scheduleRelease)) {
return false;
success = false;
}
break;
}
case OpUpdateResource::TOpAddRawFont: {
if (!ReadRawFont(cmd.get_OpAddRawFont(), reader, aUpdates)) {
gfxCriticalNote << "TOpAddRawFont failed";
return false;
success = false;
}
break;
}
@@ -622,11 +640,12 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> bytes;
if (!reader.Read(op.bytes(), bytes)) {
if (reader.Read(op.bytes(), bytes)) {
aUpdates.AddFontDescriptor(op.key(), bytes, op.fontIndex());
} else {
gfxCriticalNote << "TOpAddFontDescriptor failed";
return false;
success = false;
}
aUpdates.AddFontDescriptor(op.key(), bytes, op.fontIndex());
break;
}
case OpUpdateResource::TOpAddFontInstance: {
@@ -638,14 +657,15 @@ bool WebRenderBridgeParent::UpdateResources(
}
wr::Vec<uint8_t> variations;
if (!reader.Read(op.variations(), variations)) {
if (reader.Read(op.variations(), variations)) {
aUpdates.AddFontInstance(op.instanceKey(), op.fontKey(),
op.glyphSize(), op.options().ptrOr(nullptr),
op.platformOptions().ptrOr(nullptr),
variations);
} else {
gfxCriticalNote << "TOpAddFontInstance failed";
return false;
success = false;
}
aUpdates.AddFontInstance(op.instanceKey(), op.fontKey(), op.glyphSize(),
op.options().ptrOr(nullptr),
op.platformOptions().ptrOr(nullptr),
variations);
break;
}
case OpUpdateResource::TOpDeleteImage: {
@@ -703,7 +723,9 @@ bool WebRenderBridgeParent::UpdateResources(
: wr::Checkpoint::FrameTexturesUpdated;
aUpdates.Notify(when, std::move(scheduleRelease));
}
return true;
MOZ_ASSERT(success);
return success;
}
void WebRenderBridgeParent::AddPrivateExternalImage(
@@ -893,10 +915,22 @@ bool WebRenderBridgeParent::UpdateSharedExternalImage(
}
void WebRenderBridgeParent::ObserveSharedSurfaceRelease(
const nsTArray<wr::ExternalImageKeyPair>& aPairs) {
const nsTArray<wr::ExternalImageKeyPair>& aPairs,
const bool& aFromCheckpoint) {
if (!mDestroyed) {
Unused << SendWrReleasedImages(aPairs);
}
if (!aFromCheckpoint && mAsyncImageManager) {
// We failed to receive a checkpoint notification, so we are releasing these
// surfaces blind. Let's wait until the next epoch to complete releasing.
for (const auto& pair : aPairs) {
mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, pair.id);
}
return;
}
// We hit the checkpoint, so we know we can safely release the surfaces now.
for (const auto& pair : aPairs) {
SharedSurfacesParent::Release(pair.id);
}
@@ -944,12 +978,12 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvUpdateResources(
RollbackWrEpoch();
}
mApi->SendTransaction(txn);
if (!success) {
return IPC_FAIL(this, "Invalid WebRender resource data shmem or address.");
}
mApi->SendTransaction(txn);
return IPC_OK();
}
@@ -1118,10 +1152,8 @@ bool WebRenderBridgeParent::SetDisplayList(
const nsTArray<ipc::Shmem>& aLargeShmems, const TimeStamp& aTxnStartTime,
wr::TransactionBuilder& aTxn, wr::Epoch aWrEpoch,
bool aObserveLayersUpdate) {
if (NS_WARN_IF(!UpdateResources(aResourceUpdates, aSmallShmems, aLargeShmems,
aTxn))) {
return false;
}
bool success =
UpdateResources(aResourceUpdates, aSmallShmems, aLargeShmems, aTxn);
wr::Vec<uint8_t> dlItems(std::move(aDLItems));
wr::Vec<uint8_t> dlCache(std::move(aDLCache));
@@ -1159,7 +1191,7 @@ bool WebRenderBridgeParent::SetDisplayList(
// We will schedule generating a frame after the scene
// build is done, so we don't need to do it here.
return true;
return success;
}
bool WebRenderBridgeParent::ProcessDisplayListData(
@@ -1182,23 +1214,20 @@ bool WebRenderBridgeParent::ProcessDisplayListData(
sender.emplace(mApi, &txn);
}
if (NS_WARN_IF(
!ProcessWebRenderParentCommands(aDisplayList.mCommands, txn))) {
return false;
}
bool success = ProcessWebRenderParentCommands(aDisplayList.mCommands, txn);
if (aDisplayList.mDLItems && aDisplayList.mDLCache &&
aDisplayList.mDLSpatialTree && aValidTransaction &&
!SetDisplayList(aDisplayList.mRect,
std::move(aDisplayList.mDLItems.ref()),
std::move(aDisplayList.mDLCache.ref()),
std::move(aDisplayList.mDLSpatialTree.ref()),
aDisplayList.mDLDesc, aDisplayList.mResourceUpdates,
aDisplayList.mSmallShmems, aDisplayList.mLargeShmems,
aTxnStartTime, txn, aWrEpoch, aObserveLayersUpdate)) {
return false;
aDisplayList.mDLSpatialTree && aValidTransaction) {
success = SetDisplayList(
aDisplayList.mRect, std::move(aDisplayList.mDLItems.ref()),
std::move(aDisplayList.mDLCache.ref()),
std::move(aDisplayList.mDLSpatialTree.ref()),
aDisplayList.mDLDesc, aDisplayList.mResourceUpdates,
aDisplayList.mSmallShmems, aDisplayList.mLargeShmems,
aTxnStartTime, txn, aWrEpoch, aObserveLayersUpdate) &&
success;
}
return true;
return success;
}
mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
@@ -1245,12 +1274,8 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
bool validTransaction = aDisplayList.mIdNamespace == mIdNamespace;
bool observeLayersUpdate = ShouldParentObserveEpoch();
if (!ProcessDisplayListData(aDisplayList, wrEpoch, aTxnStartTime,
validTransaction, observeLayersUpdate)) {
wr::IpcResourceUpdateQueue::ReleaseShmems(this, aDisplayList.mSmallShmems);
wr::IpcResourceUpdateQueue::ReleaseShmems(this, aDisplayList.mLargeShmems);
return IPC_FAIL(this, "Failed to process DisplayListData.");
}
bool success = ProcessDisplayListData(aDisplayList, wrEpoch, aTxnStartTime,
validTransaction, observeLayersUpdate);
if (!validTransaction && observeLayersUpdate) {
mCompositorBridge->ObserveLayersUpdate(GetLayersId(),
@@ -1281,6 +1306,10 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvSetDisplayList(
wr::IpcResourceUpdateQueue::ReleaseShmems(this, aDisplayList.mSmallShmems);
wr::IpcResourceUpdateQueue::ReleaseShmems(this, aDisplayList.mLargeShmems);
if (!success) {
return IPC_FAIL(this, "Failed to process DisplayListData.");
}
return IPC_OK();
}
@@ -1300,16 +1329,14 @@ bool WebRenderBridgeParent::ProcessEmptyTransactionUpdates(
// AsyncImagePipelineManager.
Unused << GetNextWrEpoch();
if (aData.mIdNamespace == mIdNamespace &&
!UpdateResources(aData.mResourceUpdates, aData.mSmallShmems,
aData.mLargeShmems, txn)) {
return false;
bool success = true;
if (aData.mIdNamespace == mIdNamespace) {
success = UpdateResources(aData.mResourceUpdates, aData.mSmallShmems,
aData.mLargeShmems, txn);
}
if (!aData.mCommands.IsEmpty()) {
if (!ProcessWebRenderParentCommands(aData.mCommands, txn)) {
return false;
}
success = ProcessWebRenderParentCommands(aData.mCommands, txn) && success;
}
if (ShouldParentObserveEpoch()) {
@@ -1342,7 +1369,7 @@ bool WebRenderBridgeParent::ProcessEmptyTransactionUpdates(
mAsyncImageManager->SetWillGenerateFrame();
}
return true;
return success;
}
mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction(
@@ -1387,16 +1414,11 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction(
bool scheduleAnyComposite = false;
wr::RenderReasons renderReasons = wr::RenderReasons::NONE;
bool success = true;
if (aTransactionData) {
bool scheduleComposite = false;
if (!ProcessEmptyTransactionUpdates(*aTransactionData,
&scheduleComposite)) {
wr::IpcResourceUpdateQueue::ReleaseShmems(this,
aTransactionData->mSmallShmems);
wr::IpcResourceUpdateQueue::ReleaseShmems(this,
aTransactionData->mLargeShmems);
return IPC_FAIL(this, "Failed to process empty transaction update.");
}
success =
ProcessEmptyTransactionUpdates(*aTransactionData, &scheduleComposite);
scheduleAnyComposite = scheduleAnyComposite || scheduleComposite;
renderReasons |= wr::RenderReasons::RESOURCE_UPDATE;
}
@@ -1438,6 +1460,11 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvEmptyTransaction(
wr::IpcResourceUpdateQueue::ReleaseShmems(this,
aTransactionData->mLargeShmems);
}
if (!success) {
return IPC_FAIL(this, "Failed to process empty transaction update.");
}
return IPC_OK();
}
@@ -1460,11 +1487,13 @@ mozilla::ipc::IPCResult WebRenderBridgeParent::RecvParentCommands(
wr::TransactionBuilder txn(mApi);
txn.SetLowPriority(!IsRootWebRenderBridgeParent());
if (!ProcessWebRenderParentCommands(aCommands, txn)) {
bool success = ProcessWebRenderParentCommands(aCommands, txn);
mApi->SendTransaction(txn);
if (!success) {
return IPC_FAIL(this, "Invalid parent command found");
}
mApi->SendTransaction(txn);
return IPC_OK();
}
@@ -1476,6 +1505,7 @@ bool WebRenderBridgeParent::ProcessWebRenderParentCommands(
wr::TransactionBuilder txnForImageBridge(mApi);
wr::AutoTransactionSender sender(mApi, &txnForImageBridge);
bool success = true;
for (nsTArray<WebRenderParentCommand>::index_type i = 0;
i < aCommands.Length(); ++i) {
const WebRenderParentCommand& cmd = aCommands[i];
@@ -1530,7 +1560,9 @@ bool WebRenderBridgeParent::ProcessWebRenderParentCommands(
// process PID in the upper 32 bits of the id, verify that this is as
// expected.
if ((data.id() >> 32) != (uint64_t)OtherPid()) {
return false;
gfxCriticalNote << "TOpAddCompositorAnimations bad id";
success = false;
continue;
}
if (data.animations().Length()) {
if (RefPtr<OMTASampler> sampler = GetOMTASampler()) {
@@ -1552,7 +1584,9 @@ bool WebRenderBridgeParent::ProcessWebRenderParentCommands(
}
}
}
return true;
MOZ_ASSERT(success);
return success;
}
void WebRenderBridgeParent::FlushSceneBuilds() {