211160 2020-04-28 19:33:24 -0700 REGRESSION (r260407): Over-release of NSGraphicsContext in WebKit::convertPlatformImageToBitmap() 2020-05-08 21:09:35 -0700 1 1 1 Unclassified WebKit WebKit2 Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=210814 https://bugs.webkit.org/show_bug.cgi?id=211274 https://bugs.webkit.org/show_bug.cgi?id=211660 InRadar P2 Normal --- 208891 1 ddkilzer ddkilzer darin nmouchtaris thorton webkit-bug-importer oldest_to_newest 1646796 0 ddkilzer 2020-04-28 19:33:24 -0700 Over-release of NSGraphicsContext in WebKit::convertPlatformImageToBitmap(). Occurred in r260407 for Bug 208891: auto savedContext = adoptNS([NSGraphicsContext currentContext]); 1646797 1 397917 ddkilzer 2020-04-28 19:34:06 -0700 Created attachment 397917 Patch v1 1646798 2 397917 ddkilzer 2020-04-28 19:43:35 -0700 Comment on attachment 397917 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=397917&action=review > Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:408 > - auto savedContext = adoptNS([NSGraphicsContext currentContext]); > + RetainPtr<NSGraphicsContext> savedContext = [NSGraphicsContext currentContext]; Do we have an RAII object to save NSGraphicsContext.currentContext? Seems like it might be useful. 1646799 3 397917 ddkilzer 2020-04-28 19:51:01 -0700 Comment on attachment 397917 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=397917&action=review >> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:408 >> + RetainPtr<NSGraphicsContext> savedContext = [NSGraphicsContext currentContext]; > > Do we have an RAII object to save NSGraphicsContext.currentContext? Seems like it might be useful. Looks like we have this in WebCore, but it's all CG-based: ALLOW_DEPRECATED_DECLARATIONS_BEGIN CGContextRef cgContext = (CGContextRef)[[NSGraphicsContext currentContext] graphicsPort]; ALLOW_DEPRECATED_DECLARATIONS_END CGContextStateSaver stateSaver(cgContext); Via drawCellFocusRingWithFrameAtTime() in Source/WebCore/platform/mac/ThemeMac.mm. 1646802 4 397917 darin 2020-04-28 19:54:11 -0700 Comment on attachment 397917 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=397917&action=review >>> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:408 >>> + RetainPtr<NSGraphicsContext> savedContext = [NSGraphicsContext currentContext]; >> >> Do we have an RAII object to save NSGraphicsContext.currentContext? Seems like it might be useful. > > Looks like we have this in WebCore, but it's all CG-based: > > ALLOW_DEPRECATED_DECLARATIONS_BEGIN > CGContextRef cgContext = (CGContextRef)[[NSGraphicsContext currentContext] graphicsPort]; > ALLOW_DEPRECATED_DECLARATIONS_END > > CGContextStateSaver stateSaver(cgContext); > > Via drawCellFocusRingWithFrameAtTime() in Source/WebCore/platform/mac/ThemeMac.mm. Instead of writing out RetainPtr, I suggest you just change the adoptNS() to retainPtr(). 1646805 5 397917 ddkilzer 2020-04-28 19:57:48 -0700 Comment on attachment 397917 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=397917&action=review >>>> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:408 >>>> + RetainPtr<NSGraphicsContext> savedContext = [NSGraphicsContext currentContext]; >>> >>> Do we have an RAII object to save NSGraphicsContext.currentContext? Seems like it might be useful. >> >> Looks like we have this in WebCore, but it's all CG-based: >> >> ALLOW_DEPRECATED_DECLARATIONS_BEGIN >> CGContextRef cgContext = (CGContextRef)[[NSGraphicsContext currentContext] graphicsPort]; >> ALLOW_DEPRECATED_DECLARATIONS_END >> >> CGContextStateSaver stateSaver(cgContext); >> >> Via drawCellFocusRingWithFrameAtTime() in Source/WebCore/platform/mac/ThemeMac.mm. > > Instead of writing out RetainPtr, I suggest you just change the adoptNS() to retainPtr(). Thanks, I had forgotten about that helper. 1646808 6 397918 ddkilzer 2020-04-28 20:01:12 -0700 Created attachment 397918 Patch for landing 1646814 7 thorton 2020-04-28 20:11:30 -0700 I think Darin might have been referring to LocalCurrentGraphicsContext? 1646824 8 397918 ddkilzer 2020-04-28 21:09:25 -0700 Comment on attachment 397918 Patch for landing Setting cq+ with win bot orange as this code isn't used on Windows. 1646827 9 ews-feeder 2020-04-28 21:26:17 -0700 Committed r260864: <https://trac.webkit.org/changeset/260864> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397918. 1646828 10 webkit-bug-importer 2020-04-28 21:27:15 -0700 <rdar://problem/62573162> 1647082 11 darin 2020-04-29 11:39:54 -0700 (In reply to Tim Horton from comment #7) > I think Darin might have been referring to LocalCurrentGraphicsContext? It does seem like LocalCurrentGraphicsContext would be usable here with a tiny bit of refactoring. 397917 2020-04-28 19:34:06 -0700 2020-04-29 04:26:37 -0700 Patch v1 bug-211160-20200428193636.patch text/plain 1869 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwODQ4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDk0MDA0MzhlZDVlMTk0ZTI3 NjUwNDYzYzEyYTljM2I2NjhmNzliYjAuLjcyNjI0MDI3NGI2YTZhYjc5NTQ1OTU1MGMxYjFkZmYw Y2QzZDQ4YjMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMjAtMDQtMjggIERhdmlkIEtp bHplciAgPGRka2lsemVyQGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OIChyMjYwNDA3 KTogT3Zlci1yZWxlYXNlIG9mIE5TR3JhcGhpY3NDb250ZXh0IGluIFdlYktpdDo6Y29udmVydFBs YXRmb3JtSW1hZ2VUb0JpdG1hcCgpCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8yMTEx NjA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBV SVByb2Nlc3MvQ29jb2EvV2ViUGFnZVByb3h5Q29jb2EubW06CisgICAgICAgIChXZWJLaXQ6OmNv bnZlcnRQbGF0Zm9ybUltYWdlVG9CaXRtYXApOiBEb24ndCB1c2UgYWRvcHROUygpIG9uCisgICAg ICAgIGFuIGF1dG9yZWxlYXNlZCByZXR1cm4gdmFsdWUgc2luY2UgaXQgY2F1c2VzIGFuIG92ZXIt cmVsZWFzZS4KKwogMjAyMC0wNC0yOCAgRGF2aWQgS2lsemVyICA8ZGRraWx6ZXJAYXBwbGUuY29t PgogCiAgICAgICAgIElQQzo6RGVjb2Rlcjo6aXNWYWxpZCgpIHNob3VsZCBqdXN0IGJlIGEgbnVs bHB0ciBjaGVjawpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQ29jb2EvV2Vi UGFnZVByb3h5Q29jb2EubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9XZWJQYWdl UHJveHlDb2NvYS5tbQppbmRleCA3MjNhM2RlZWFiYTAzNTY0MjlkMTc2ZDljNDFlMmY4NGMyN2Nm OWJhLi45YmZjOTIzNTc4YzcwMmQ3YmEzNjljOTkwMDUxMTE2N2IwNWE2N2I4IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9XZWJQYWdlUHJveHlDb2NvYS5tbQorKysg Yi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9XZWJQYWdlUHJveHlDb2NvYS5tbQpAQCAt NDA1LDcgKzQwNSw3IEBAIHN0YXRpYyBSZWZQdHI8V2ViS2l0OjpTaGFyZWFibGVCaXRtYXA+IGNv bnZlcnRQbGF0Zm9ybUltYWdlVG9CaXRtYXAoQ29jb2FJbWFnZSAqCiAgICAgW2ltYWdlIGRyYXdJ blJlY3Q6Q0dSZWN0TWFrZSgwLCAwLCBiaXRtYXAtPnNpemUoKS53aWR0aCgpLCBiaXRtYXAtPnNp emUoKS5oZWlnaHQoKSldOwogICAgIFVJR3JhcGhpY3NQb3BDb250ZXh0KCk7CiAjZWxpZiBQTEFU Rk9STShNQUMpCi0gICAgYXV0byBzYXZlZENvbnRleHQgPSBhZG9wdE5TKFtOU0dyYXBoaWNzQ29u dGV4dCBjdXJyZW50Q29udGV4dF0pOworICAgIFJldGFpblB0cjxOU0dyYXBoaWNzQ29udGV4dD4g c2F2ZWRDb250ZXh0ID0gW05TR3JhcGhpY3NDb250ZXh0IGN1cnJlbnRDb250ZXh0XTsKIAogICAg IFtOU0dyYXBoaWNzQ29udGV4dCBzZXRDdXJyZW50Q29udGV4dDpbTlNHcmFwaGljc0NvbnRleHQg Z3JhcGhpY3NDb250ZXh0V2l0aENHQ29udGV4dDpncmFwaGljc0NvbnRleHQtPnBsYXRmb3JtQ29u dGV4dCgpIGZsaXBwZWQ6WUVTXV07CiAgICAgW2ltYWdlIGRyYXdJblJlY3Q6TlNNYWtlUmVjdCgw LCAwLCBiaXRtYXAtPnNpemUoKS53aWR0aCgpLCBiaXRtYXAtPnNpemUoKS5oZWlnaHQoKSkgZnJv bVJlY3Q6TlNaZXJvUmVjdCBvcGVyYXRpb246TlNDb21wb3NpdGluZ09wZXJhdGlvblNvdXJjZU92 ZXIgZnJhY3Rpb246MSByZXNwZWN0RmxpcHBlZDpZRVMgaGludHM6bmlsXTsK 397918 2020-04-28 20:01:12 -0700 2020-04-28 21:26:19 -0700 Patch for landing bug-211160-20200428200343.patch text/plain 1830 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwODQ4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDk0MDA0MzhlZDVlMTk0ZTI3 NjUwNDYzYzEyYTljM2I2NjhmNzliYjAuLjI3MWQ5NDA3NmI3ZjAwYWMzNmZkODZiMzY5NGY1NWZl YjhlOTI1MDEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMjAtMDQtMjggIERhdmlkIEtp bHplciAgPGRka2lsemVyQGFwcGxlLmNvbT4KKworICAgICAgICBSRUdSRVNTSU9OIChyMjYwNDA3 KTogT3Zlci1yZWxlYXNlIG9mIE5TR3JhcGhpY3NDb250ZXh0IGluIFdlYktpdDo6Y29udmVydFBs YXRmb3JtSW1hZ2VUb0JpdG1hcCgpCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8yMTEx NjA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCisKKyAgICAgICAgKiBVSVBy b2Nlc3MvQ29jb2EvV2ViUGFnZVByb3h5Q29jb2EubW06CisgICAgICAgIChXZWJLaXQ6OmNvbnZl cnRQbGF0Zm9ybUltYWdlVG9CaXRtYXApOiBVc2UgcmV0YWluUHRyKCkgaW5zdGVhZAorICAgICAg ICBvZiBhZG9wdE5TKCkgdG8gZml4IHRoZSBvdmVyLXJlbGVhc2UuCisKIDIwMjAtMDQtMjggIERh dmlkIEtpbHplciAgPGRka2lsemVyQGFwcGxlLmNvbT4KIAogICAgICAgICBJUEM6OkRlY29kZXI6 OmlzVmFsaWQoKSBzaG91bGQganVzdCBiZSBhIG51bGxwdHIgY2hlY2sKZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJLaXQvVUlQcm9jZXNzL0NvY29hL1dlYlBhZ2VQcm94eUNvY29hLm1tIGIvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvQ29jb2EvV2ViUGFnZVByb3h5Q29jb2EubW0KaW5kZXggNzIzYTNk ZWVhYmEwMzU2NDI5ZDE3NmQ5YzQxZTJmODRjMjdjZjliYS4uOWY1YjBhMDU5MjUxYmZiYTQ5MjJm ZGE5NTJjNDVhZGVkYjZiNGU2MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3Mv Q29jb2EvV2ViUGFnZVByb3h5Q29jb2EubW0KKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3Mv Q29jb2EvV2ViUGFnZVByb3h5Q29jb2EubW0KQEAgLTQwNSw3ICs0MDUsNyBAQCBzdGF0aWMgUmVm UHRyPFdlYktpdDo6U2hhcmVhYmxlQml0bWFwPiBjb252ZXJ0UGxhdGZvcm1JbWFnZVRvQml0bWFw KENvY29hSW1hZ2UgKgogICAgIFtpbWFnZSBkcmF3SW5SZWN0OkNHUmVjdE1ha2UoMCwgMCwgYml0 bWFwLT5zaXplKCkud2lkdGgoKSwgYml0bWFwLT5zaXplKCkuaGVpZ2h0KCkpXTsKICAgICBVSUdy YXBoaWNzUG9wQ29udGV4dCgpOwogI2VsaWYgUExBVEZPUk0oTUFDKQotICAgIGF1dG8gc2F2ZWRD b250ZXh0ID0gYWRvcHROUyhbTlNHcmFwaGljc0NvbnRleHQgY3VycmVudENvbnRleHRdKTsKKyAg ICBhdXRvIHNhdmVkQ29udGV4dCA9IHJldGFpblB0cihbTlNHcmFwaGljc0NvbnRleHQgY3VycmVu dENvbnRleHRdKTsKIAogICAgIFtOU0dyYXBoaWNzQ29udGV4dCBzZXRDdXJyZW50Q29udGV4dDpb TlNHcmFwaGljc0NvbnRleHQgZ3JhcGhpY3NDb250ZXh0V2l0aENHQ29udGV4dDpncmFwaGljc0Nv bnRleHQtPnBsYXRmb3JtQ29udGV4dCgpIGZsaXBwZWQ6WUVTXV07CiAgICAgW2ltYWdlIGRyYXdJ blJlY3Q6TlNNYWtlUmVjdCgwLCAwLCBiaXRtYXAtPnNpemUoKS53aWR0aCgpLCBiaXRtYXAtPnNp emUoKS5oZWlnaHQoKSkgZnJvbVJlY3Q6TlNaZXJvUmVjdCBvcGVyYXRpb246TlNDb21wb3NpdGlu Z09wZXJhdGlvblNvdXJjZU92ZXIgZnJhY3Rpb246MSByZXNwZWN0RmxpcHBlZDpZRVMgaGludHM6 bmlsXTsK