Bug 1511493 - Ensure PushGlyphs uses the current transaction's IpcResourceUpdateQueue. r=emilio

WebRenderBridgeChild::GetFontKeyForScaledFont can currently cause a IpcResourceUpdateQueue race.
If we're in the middle of a transaction building a blob image, GetFontKeyForScaledFont is called
in the blob image building code using the transaction's IpcResourceUpdateQueue as expected, such
that resource updates are sent out when the transaction is finalized.

However, TextDrawTarget calls into PushGlyphs without passing along its IpcResourceUpdateQueue,
calling GetFontKeyForScaledFont without it, and causing it to immediately send out the resource
update.

So if a blob image uses a font key and submits a resource update, but a display list is built
after that also using the font key within the transaction, the display list will fail to send
the resource update because it thinks the blob image already did, even though the blob image
transaction has not yet been finalized.

The simple fix is to just pass IpcResourceUpdateQueue from TextDrawTarget into PushGlyphs, thus
ensuring the resource updates are properly ordered.

Differential Revision: https://phabricator.services.mozilla.com/D140438
This commit is contained in:
Lee Salzman 2022-03-05 23:35:16 +00:00
Родитель 9728affece
Коммит d22d724cc2
8 изменённых файлов: 27 добавлений и 43 удалений

Просмотреть файл

@ -197,12 +197,12 @@ void RenderRootStateManager::ReleaseTextureOfImage(const wr::ImageKey& aKey) {
}
Maybe<wr::FontInstanceKey> RenderRootStateManager::GetFontKeyForScaledFont(
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue* aResources) {
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue& aResources) {
return WrBridge()->GetFontKeyForScaledFont(aScaledFont, aResources);
}
Maybe<wr::FontKey> RenderRootStateManager::GetFontKeyForUnscaledFont(
gfx::UnscaledFont* aUnscaledFont, wr::IpcResourceUpdateQueue* aResources) {
gfx::UnscaledFont* aUnscaledFont, wr::IpcResourceUpdateQueue& aResources) {
return WrBridge()->GetFontKeyForUnscaledFont(aUnscaledFont, aResources);
}

Просмотреть файл

@ -65,11 +65,9 @@ class RenderRootStateManager {
/// It is used for recycling TextureClient.
void ReleaseTextureOfImage(const wr::ImageKey& aKey);
Maybe<wr::FontInstanceKey> GetFontKeyForScaledFont(
gfx::ScaledFont* aScaledFont,
wr::IpcResourceUpdateQueue* aResources = nullptr);
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue& aResources);
Maybe<wr::FontKey> GetFontKeyForUnscaledFont(
gfx::UnscaledFont* aUnscaledFont,
wr::IpcResourceUpdateQueue* aResources = nullptr);
gfx::UnscaledFont* aUnscaledFont, wr::IpcResourceUpdateQueue& aResources);
void FlushAsyncResourceUpdates();

Просмотреть файл

@ -233,14 +233,14 @@ static void WriteFontDescriptor(const uint8_t* aData, uint32_t aLength,
}
void WebRenderBridgeChild::PushGlyphs(
wr::DisplayListBuilder& aBuilder, Range<const wr::GlyphInstance> aGlyphs,
gfx::ScaledFont* aFont, const wr::ColorF& aColor,
const StackingContextHelper& aSc, const wr::LayoutRect& aBounds,
const wr::LayoutRect& aClip, bool aBackfaceVisible,
const wr::GlyphOptions* aGlyphOptions) {
wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources,
Range<const wr::GlyphInstance> aGlyphs, gfx::ScaledFont* aFont,
const wr::ColorF& aColor, const StackingContextHelper& aSc,
const wr::LayoutRect& aBounds, const wr::LayoutRect& aClip,
bool aBackfaceVisible, const wr::GlyphOptions* aGlyphOptions) {
MOZ_ASSERT(aFont);
Maybe<wr::WrFontInstanceKey> key = GetFontKeyForScaledFont(aFont);
Maybe<wr::WrFontInstanceKey> key = GetFontKeyForScaledFont(aFont, aResources);
MOZ_ASSERT(key.isSome());
if (key.isSome()) {
@ -250,7 +250,7 @@ void WebRenderBridgeChild::PushGlyphs(
}
Maybe<wr::FontInstanceKey> WebRenderBridgeChild::GetFontKeyForScaledFont(
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue* aResources) {
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue& aResources) {
MOZ_ASSERT(!mDestroyed);
MOZ_ASSERT(aScaledFont);
MOZ_ASSERT(aScaledFont->CanSerialize());
@ -258,10 +258,6 @@ Maybe<wr::FontInstanceKey> WebRenderBridgeChild::GetFontKeyForScaledFont(
return mFontInstanceKeys.WithEntryHandle(
aScaledFont, [&](auto&& entry) -> Maybe<wr::FontInstanceKey> {
if (!entry) {
Maybe<wr::IpcResourceUpdateQueue> resources =
aResources ? Nothing() : Some(wr::IpcResourceUpdateQueue(this));
aResources = resources.ptrOr(aResources);
Maybe<wr::FontKey> fontKey = GetFontKeyForUnscaledFont(
aScaledFont->GetUnscaledFont(), aResources);
if (fontKey.isNothing()) {
@ -276,13 +272,10 @@ Maybe<wr::FontInstanceKey> WebRenderBridgeChild::GetFontKeyForScaledFont(
aScaledFont->GetWRFontInstanceOptions(&options, &platformOptions,
&variations);
aResources->AddFontInstance(
aResources.AddFontInstance(
instanceKey, fontKey.value(), aScaledFont->GetSize(),
options.ptrOr(nullptr), platformOptions.ptrOr(nullptr),
Range<const FontVariation>(variations.data(), variations.size()));
if (resources.isSome()) {
UpdateResources(resources.ref());
}
entry.Insert(instanceKey);
}
@ -292,17 +285,14 @@ Maybe<wr::FontInstanceKey> WebRenderBridgeChild::GetFontKeyForScaledFont(
}
Maybe<wr::FontKey> WebRenderBridgeChild::GetFontKeyForUnscaledFont(
gfx::UnscaledFont* aUnscaled, wr::IpcResourceUpdateQueue* aResources) {
gfx::UnscaledFont* aUnscaled, wr::IpcResourceUpdateQueue& aResources) {
MOZ_ASSERT(!mDestroyed);
return mFontKeys.WithEntryHandle(
aUnscaled, [&](auto&& entry) -> Maybe<wr::FontKey> {
if (!entry) {
Maybe<wr::IpcResourceUpdateQueue> resources =
aResources ? Nothing() : Some(wr::IpcResourceUpdateQueue(this));
wr::FontKey fontKey = {wr::IdNamespace{0}, 0};
FontFileDataSink sink = {&fontKey, this, resources.ptrOr(aResources)};
FontFileDataSink sink = {&fontKey, this, &aResources};
// First try to retrieve a descriptor for the font, as this is much
// cheaper to send over IPC than the full raw font data. If this is
// not possible, then and only then fall back to getting the raw font
@ -313,10 +303,6 @@ Maybe<wr::FontKey> WebRenderBridgeChild::GetFontKeyForUnscaledFont(
return Nothing();
}
if (resources.isSome()) {
UpdateResources(resources.ref());
}
entry.Insert(fontKey);
}

Просмотреть файл

@ -143,6 +143,7 @@ class WebRenderBridgeChild final : public PWebRenderBridgeChild,
}
void PushGlyphs(wr::DisplayListBuilder& aBuilder,
wr::IpcResourceUpdateQueue& aResources,
Range<const wr::GlyphInstance> aGlyphs,
gfx::ScaledFont* aFont, const wr::ColorF& aColor,
const StackingContextHelper& aSc,
@ -151,11 +152,9 @@ class WebRenderBridgeChild final : public PWebRenderBridgeChild,
const wr::GlyphOptions* aGlyphOptions = nullptr);
Maybe<wr::FontInstanceKey> GetFontKeyForScaledFont(
gfx::ScaledFont* aScaledFont,
wr::IpcResourceUpdateQueue* aResources = nullptr);
gfx::ScaledFont* aScaledFont, wr::IpcResourceUpdateQueue& aResources);
Maybe<wr::FontKey> GetFontKeyForUnscaledFont(
gfx::UnscaledFont* aUnscaledFont,
wr::IpcResourceUpdateQueue* aResources = nullptr);
gfx::UnscaledFont* aUnscaledFont, wr::IpcResourceUpdateQueue& aResources);
void RemoveExpiredFontKeys(wr::IpcResourceUpdateQueue& aResources);
void BeginClearCachedResources();

Просмотреть файл

@ -628,8 +628,8 @@ struct DIGroup {
aStream.write((const char*)&count, sizeof(count));
for (auto& scaled : aScaledFonts) {
Maybe<wr::FontInstanceKey> key =
aWrManager->WrBridge()->GetFontKeyForScaledFont(
scaled, &aResources);
aWrManager->WrBridge()->GetFontKeyForScaledFont(scaled,
aResources);
if (key.isNothing()) {
validFonts = false;
break;
@ -2396,8 +2396,8 @@ WebRenderCommandBuilder::GenerateFallbackData(
aStream.write((const char*)&count, sizeof(count));
for (auto& scaled : aScaledFonts) {
Maybe<wr::FontInstanceKey> key =
mManager->WrBridge()->GetFontKeyForScaledFont(
scaled, &aResources);
mManager->WrBridge()->GetFontKeyForScaledFont(scaled,
aResources);
if (key.isNothing()) {
validFonts = false;
break;
@ -2600,7 +2600,7 @@ Maybe<wr::ImageMask> WebRenderCommandBuilder::BuildWrMaskImage(
for (auto& scaled : aScaledFonts) {
Maybe<wr::FontInstanceKey> key =
mManager->WrBridge()->GetFontKeyForScaledFont(scaled,
&aResources);
aResources);
if (key.isNothing()) {
validFonts = false;
break;

Просмотреть файл

@ -131,6 +131,7 @@ impl ApiResources {
}
}
ResourceUpdate::AddFontInstance(ref instance) => {
assert!(self.fonts.templates.contains_key(&instance.font_key));
// TODO(nical): Don't clone these.
self.fonts.instances.add_font_instance(
instance.key,

Просмотреть файл

@ -163,7 +163,7 @@ Maybe<BlobImageKeyData> BlobSurfaceProvider::RecordDrawing(
for (auto& scaled : aScaledFonts) {
Maybe<wr::FontInstanceKey> key =
wrBridge->GetFontKeyForScaledFont(scaled, &aResources);
wrBridge->GetFontKeyForScaledFont(scaled, aResources);
if (key.isNothing()) {
validFonts = false;
break;

Просмотреть файл

@ -211,9 +211,9 @@ class TextDrawTarget : public DrawTarget {
wr::ToFontRenderMode(aOptions.mAntialiasMode, GetPermitSubpixelAA());
glyphOptions.flags = mWRGlyphFlags;
mManager->WrBridge()->PushGlyphs(mBuilder, glyphs, aFont, color, *mSc,
mBoundsRect, ClipRect(), mBackfaceVisible,
&glyphOptions);
mManager->WrBridge()->PushGlyphs(mBuilder, *mResources, glyphs, aFont,
color, *mSc, mBoundsRect, ClipRect(),
mBackfaceVisible, &glyphOptions);
}
void PushClipRect(const Rect& aRect) override {