219752 2020-12-10 11:46:47 -0800 [GPU Process] Crash when loading drinktrade.com 2020-12-10 14:02:03 -0800 1 1 1 Unclassified WebKit Images WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jonlee wenson_hsieh bfulgham sabouhallawa simon.fraser thorton webkit-bug-importer wenson_hsieh zalan oldest_to_newest 1713639 0 jonlee 2020-12-10 11:46:47 -0800 Loading drinktrade.com causes crashes, based on a ToT build on r270635. From Wenson: Detected over-release of a CFTypeRef 0x10c8cad80 (317 / CGImage) Thread 0 name: Dispatch queue: com.apple.main-thread Thread 0 Crashed: 0 CoreFoundation 0x00000001813269b0 _CFRelease.cold.3 + 92 1 CoreFoundation 0x00000001813269a8 _CFRelease.cold.3 + 84 2 CoreFoundation 0x0000000181230a64 _CFRelease + 1444 3 WebCore 0x0000000108f12efc WebCore::NativeImage::~NativeImage() + 208 4 WebCore 0x0000000108f909c8 WebCore::DisplayList::DisplayList::clear() + 256 5 WebKit 0x000000010323f478 WebKit::RemoteImageBufferProxy<WebKit::ImageBufferShareableIOSurfaceBackend>::flushDrawingContextAndCommit() + 192 1713640 1 webkit-bug-importer 2020-12-10 11:47:12 -0800 <rdar://problem/72190569> 1713661 2 wenson_hsieh 2020-12-10 12:20:08 -0800 It seems we're just missing a retain here, since cgImage isn't a newly created object we should be taking ownership of: diff --git a/Source/WebCore/platform/graphics/displaylists/DisplayListDrawGlyphsRecorderCoreText.cpp b/Source/WebCore/platform/graphics/displaylists/DisplayListDrawGlyphsRecorderCoreText.cpp index 8bc00142820e..6c01c41599a6 100644 --- a/Source/WebCore/platform/graphics/displaylists/DisplayListDrawGlyphsRecorderCoreText.cpp +++ b/Source/WebCore/platform/graphics/displaylists/DisplayListDrawGlyphsRecorderCoreText.cpp @@ -352,7 +352,7 @@ void DrawGlyphsRecorder::recordDrawImage(CGRenderingStateRef, CGGStateRef gstate m_owner.translate(0, rect.size.height + 2 * rect.origin.y); m_owner.scale(FloatSize(1, -1)); - auto image = NativeImage::create(adoptCF(cgImage)); + auto image = NativeImage::create(cgImage); m_owner.drawNativeImage(*image, image->size(), FloatRect(rect), FloatRect {{ }, image->size()}, ImagePaintingOptions { ImageOrientation::OriginTopLeft }); // Undo the above y-flip to restore the context. I suspect this is probably covered by some existing layout test when GPU process is enabled... 1713687 3 wenson_hsieh 2020-12-10 13:08:28 -0800 (In reply to Wenson Hsieh from comment #2) > It seems we're just missing a retain here, since cgImage isn't a newly > created object we should be taking ownership of: > > diff --git > a/Source/WebCore/platform/graphics/displaylists/ > DisplayListDrawGlyphsRecorderCoreText.cpp > b/Source/WebCore/platform/graphics/displaylists/ > DisplayListDrawGlyphsRecorderCoreText.cpp > index 8bc00142820e..6c01c41599a6 100644 > --- > a/Source/WebCore/platform/graphics/displaylists/ > DisplayListDrawGlyphsRecorderCoreText.cpp > +++ > b/Source/WebCore/platform/graphics/displaylists/ > DisplayListDrawGlyphsRecorderCoreText.cpp > @@ -352,7 +352,7 @@ void > DrawGlyphsRecorder::recordDrawImage(CGRenderingStateRef, CGGStateRef gstate > m_owner.translate(0, rect.size.height + 2 * rect.origin.y); > m_owner.scale(FloatSize(1, -1)); > > - auto image = NativeImage::create(adoptCF(cgImage)); > + auto image = NativeImage::create(cgImage); > m_owner.drawNativeImage(*image, image->size(), FloatRect(rect), > FloatRect {{ }, image->size()}, ImagePaintingOptions { > ImageOrientation::OriginTopLeft }); > > // Undo the above y-flip to restore the context. > > I suspect this is probably covered by some existing layout test when GPU > process is enabled... Looks like fast/text/emoji.html should have us covered here. 1713691 4 415919 wenson_hsieh 2020-12-10 13:19:46 -0800 Created attachment 415919 Patch 1713709 5 ews-feeder 2020-12-10 14:02:02 -0800 Committed r270653: <https://trac.webkit.org/changeset/270653> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415919. 415919 2020-12-10 13:19:46 -0800 2020-12-10 14:02:03 -0800 Patch bug-219752-20201210131945.patch text/plain 2127 wenson_hsieh U3VidmVyc2lvbiBSZXZpc2lvbjogMjcwNjExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTZjOWMxZDFhMTFhMWRi OGJkYzA0MWQ4MTUyOTk4OTA3MWRlNDJkZC4uNGE1ZDJkYmI5Njk1NDE3ZWE4ZDJhYWM1ZDRlODAz OTlhMzJhYWI0OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIwLTEyLTEwICBXZW5z b24gSHNpZWggIDx3ZW5zb25faHNpZWhAYXBwbGUuY29tPgorCisgICAgICAgIFtHUFUgUHJvY2Vz c10gQ3Jhc2ggd2hlbiBsb2FkaW5nIGRyaW5rdHJhZGUuY29tCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTk3NTIKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBGaXggYW4gb3ZlcnJlbGVhc2Ugd2hlbiByZW5k ZXJpbmcgY29sb3IgZW1vamkgZm9udHMgYnkgcmVtb3ZpbmcgYW4gZXh0cmFuZW91cyBgYWRvcHRD RmAgYXJvdW5kIGEgYENHSW1hZ2VSZWZgLgorICAgICAgICBUaGlzIGBDR0ltYWdlUmVmYCBpcyBu b3QgYSByZXRhaW5lZCAoKzEpIG9iamVjdCwgc28gd2UgbmVlZCB0byBtYWtlIHN1cmUgdGhhdCBp dCBpcyByZXRhaW5lZCB3aGVuIHRyYWNraW5nIGl0IGluCisgICAgICAgIHRoZSBkaXNwbGF5IGxp c3QncyBuYXRpdmUgaW1hZ2UgY2FjaGUuCisKKyAgICAgICAgTGF5b3V0IHRlc3Q6IGZhc3QvdGV4 dC9lbW9qaS5odG1sCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9kaXNwbGF5bGlzdHMv RGlzcGxheUxpc3REcmF3R2x5cGhzUmVjb3JkZXJDb3JlVGV4dC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpEaXNwbGF5TGlzdDo6RHJhd0dseXBoc1JlY29yZGVyOjpyZWNvcmREcmF3SW1hZ2UpOgor CiAyMDIwLTEyLTEwICBXZW5zb24gSHNpZWggIDx3ZW5zb25faHNpZWhAYXBwbGUuY29tPgogCiAg ICAgICAgIERpc3BsYXlMaXN0OjpSZWNvcmRlcjo6ZHJhd0ltYWdlQnVmZmVyIHNob3VsZCBmbHVz aCB0aGUgZ2l2ZW4gaW1hZ2UgYnVmZmVyIGJlZm9yZSByZWNvcmRpbmcKZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Rpc3BsYXlsaXN0cy9EaXNwbGF5TGlzdERy YXdHbHlwaHNSZWNvcmRlckNvcmVUZXh0LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL2Rpc3BsYXlsaXN0cy9EaXNwbGF5TGlzdERyYXdHbHlwaHNSZWNvcmRlckNvcmVUZXh0 LmNwcAppbmRleCA4YmMwMDE0MjgyMGU5NzM5NjVlYmE1YTkzZDA2YTAzMDliODY5NWQzLi42YzAx YzQxNTk5YTY4NGJmNDc0ODAzM2E2OWYxMDg4M2M1OTEwYmI5IDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9kaXNwbGF5bGlzdHMvRGlzcGxheUxpc3REcmF3R2x5 cGhzUmVjb3JkZXJDb3JlVGV4dC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvZGlzcGxheWxpc3RzL0Rpc3BsYXlMaXN0RHJhd0dseXBoc1JlY29yZGVyQ29yZVRleHQu Y3BwCkBAIC0zNTIsNyArMzUyLDcgQEAgdm9pZCBEcmF3R2x5cGhzUmVjb3JkZXI6OnJlY29yZERy YXdJbWFnZShDR1JlbmRlcmluZ1N0YXRlUmVmLCBDR0dTdGF0ZVJlZiBnc3RhdGUKICAgICBtX293 bmVyLnRyYW5zbGF0ZSgwLCByZWN0LnNpemUuaGVpZ2h0ICsgMiAqIHJlY3Qub3JpZ2luLnkpOwog ICAgIG1fb3duZXIuc2NhbGUoRmxvYXRTaXplKDEsIC0xKSk7CiAKLSAgICBhdXRvIGltYWdlID0g TmF0aXZlSW1hZ2U6OmNyZWF0ZShhZG9wdENGKGNnSW1hZ2UpKTsKKyAgICBhdXRvIGltYWdlID0g TmF0aXZlSW1hZ2U6OmNyZWF0ZShjZ0ltYWdlKTsKICAgICBtX293bmVyLmRyYXdOYXRpdmVJbWFn ZSgqaW1hZ2UsIGltYWdlLT5zaXplKCksIEZsb2F0UmVjdChyZWN0KSwgRmxvYXRSZWN0IHt7IH0s IGltYWdlLT5zaXplKCl9LCBJbWFnZVBhaW50aW5nT3B0aW9ucyB7IEltYWdlT3JpZW50YXRpb246 Ok9yaWdpblRvcExlZnQgfSk7CiAKICAgICAvLyBVbmRvIHRoZSBhYm92ZSB5LWZsaXAgdG8gcmVz dG9yZSB0aGUgY29udGV4dC4K