184718 2018-04-17 17:02:39 -0700 WebKit::DisplayLink maintains a strong reference to WebPageProxy, creating a reference cycle 2018-04-17 20:07:17 -0700 1 1 1 Unclassified WebKit WebKit API WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P1 Normal --- 1 conrad_shultz conrad_shultz commit-queue conrad_shultz pvollan thorton oldest_to_newest 1415353 0 conrad_shultz 2018-04-17 17:02:39 -0700 WebKit::DisplayLink maintains a strong reference to WebPageProxy, creating a reference cycle. 1415355 1 conrad_shultz 2018-04-17 17:03:31 -0700 <rdar://problem/39468022> 1415356 2 conrad_shultz 2018-04-17 17:03:46 -0700 rdar://problem/39468022 1415357 3 conrad_shultz 2018-04-17 17:04:11 -0700 <rdar://problem/39468022> 1415362 4 conrad_shultz 2018-04-17 17:06:32 -0700 rdar://problem/39468022 1415364 5 conrad_shultz 2018-04-17 17:07:45 -0700 rdar://problem/39468022 1415368 6 conrad_shultz 2018-04-17 17:08:55 -0700 Oops... 1415382 7 338167 conrad_shultz 2018-04-17 17:25:23 -0700 Created attachment 338167 Patch 1415392 8 338167 pvollan 2018-04-17 17:41:18 -0700 Comment on attachment 338167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338167&action=review Thanks for finding and fixing this! > Source/WebKit/UIProcess/mac/DisplayLink.cpp:-39 > - : m_webPageProxy(webPageProxy) The address of the WebPageProxy object is passed as a parameter to the function CVDisplayLinkSetOutputCallback below. This pointer is then passed as a void* parameter in the DisplayLink::displayLinkCallback method where it is casted to a WebPageProxy object. If it is guaranteed that we will not receive any callbacks after CVDisplayLinkStop is called in the destructor, then this patch is correct. Otherwise I think we have to use a weak pointer instead. 1415396 9 338167 thorton 2018-04-17 17:44:05 -0700 Comment on attachment 338167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338167&action=review >> Source/WebKit/UIProcess/mac/DisplayLink.cpp:-39 >> - : m_webPageProxy(webPageProxy) > > The address of the WebPageProxy object is passed as a parameter to the function CVDisplayLinkSetOutputCallback below. This pointer is then passed as a void* parameter in the DisplayLink::displayLinkCallback method where it is casted to a WebPageProxy object. If it is guaranteed that we will not receive any callbacks after CVDisplayLinkStop is called in the destructor, then this patch is correct. Otherwise I think we have to use a weak pointer instead. Good point 1415449 10 338167 commit-queue 2018-04-17 20:07:16 -0700 Comment on attachment 338167 Patch Clearing flags on attachment: 338167 Committed r230747: <https://trac.webkit.org/changeset/230747> 1415450 11 commit-queue 2018-04-17 20:07:17 -0700 All reviewed patches have been landed. Closing bug. 338167 2018-04-17 17:25:23 -0700 2018-04-17 20:07:16 -0700 Patch bug-184718-20180417172523.patch text/plain 2202 conrad_shultz U3VidmVyc2lvbiBSZXZpc2lvbjogMjMwNzQwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDYyOTc4MWM2NWZhMDVhYmQy N2RjZDdlYWEyOTViZjBhMjBmMDE4MjMuLjNjYjMwNjQ5MzRjNGRiMTJhZmZhYzNhYzc1NmQxMjA1 ZDRmZWNhNDggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTgtMDQtMTcgIENvbnJhZCBT aHVsdHogIDxjb25yYWRfc2h1bHR6QGFwcGxlLmNvbT4KKworICAgICAgICBXZWJLaXQ6OkRpc3Bs YXlMaW5rIG1haW50YWlucyBhIHN0cm9uZyByZWZlcmVuY2UgdG8gV2ViUGFnZVByb3h5LCBjcmVh dGluZyBhIHJlZmVyZW5jZSBjeWNsZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MTg0NzE4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgSXQgdHVybnMgb3V0IHRoYXQgdGhlIG1fd2ViUGFnZVByb3h5IGJhY2st cmVmZXJlbmNlIGluIERpc3BsYXlMaW5rLCB3aGljaCB3YXMgY3JlYXRpbmcgYQorICAgICAgICBy ZWZlcmVuY2UgY3ljbGUsIHdhc24ndCBldmVyIHJlYWQsIHNvIHdlIGNhbiBqdXN0IHJlbW92ZSBp dC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9tYWMvRGlzcGxheUxpbmsuY3BwOgorICAgICAgICAo V2ViS2l0OjpEaXNwbGF5TGluazo6RGlzcGxheUxpbmspOgorICAgICAgICAqIFVJUHJvY2Vzcy9t YWMvRGlzcGxheUxpbmsuaDoKKwogMjAxOC0wNC0xNyAgQWRyaWFuIFBlcmV6IGRlIENhc3RybyAg PGFwZXJlekBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtHVEtdW1dQRV0gQnVpbGQgZmFpbHVyZSBk dWUgdG8gcHJlc2VuY2Ugb2YgQXZhaGkncyA8ZG5zX3NkLmg+IGhlYWRlcgpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL0Rpc3BsYXlMaW5rLmNwcCBiL1NvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL21hYy9EaXNwbGF5TGluay5jcHAKaW5kZXggOWU2MWRkYjQwMWM4MTFiYzE3 ODQwMGEwOTA5MzdhOTAyMzNhYzFkOS4uYjRmYTZkZTI1Zjc2ZTg0Nzc4MTg1NzZjNzAzZGZhODg2 NDI4OGZkYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL0Rpc3BsYXlM aW5rLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9tYWMvRGlzcGxheUxpbmsuY3Bw CkBAIC0zNiw3ICszNiw2IEBACiBuYW1lc3BhY2UgV2ViS2l0IHsKICAgICAKIERpc3BsYXlMaW5r OjpEaXNwbGF5TGluayhXZWJDb3JlOjpQbGF0Zm9ybURpc3BsYXlJRCBkaXNwbGF5SUQsIFdlYlBh Z2VQcm94eSYgd2ViUGFnZVByb3h5KQotICAgIDogbV93ZWJQYWdlUHJveHkod2ViUGFnZVByb3h5 KQogewogICAgIFJFTEVBU0VfQVNTRVJUKGhhc1Byb2Nlc3NQcml2aWxlZ2UoUHJvY2Vzc1ByaXZp bGVnZTo6Q2FuQ29tbXVuaWNhdGVXaXRoV2luZG93U2VydmVyKSk7CiAgICAgQ1ZSZXR1cm4gZXJy b3IgPSBDVkRpc3BsYXlMaW5rQ3JlYXRlV2l0aENHRGlzcGxheShkaXNwbGF5SUQsICZtX2Rpc3Bs YXlMaW5rKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL21hYy9EaXNwbGF5 TGluay5oIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFjL0Rpc3BsYXlMaW5rLmgKaW5kZXgg MTBhMTdiN2VjNTQzMGZkOGQ0N2I2MTFhZjUxYzdiOWI5MjMxMzkyMi4uOGZlOWFkZDkwNDRkNjkz NTRmYjg0NWIxMmUzYmZkZDJjMDUzMWFiZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvbWFjL0Rpc3BsYXlMaW5rLmgKKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbWFj L0Rpc3BsYXlMaW5rLmgKQEAgLTQ5LDcgKzQ5LDYgQEAgcHJpdmF0ZToKICAgICBzdGF0aWMgQ1ZS ZXR1cm4gZGlzcGxheUxpbmtDYWxsYmFjayhDVkRpc3BsYXlMaW5rUmVmLCBjb25zdCBDVlRpbWVT dGFtcCosIGNvbnN0IENWVGltZVN0YW1wKiwgQ1ZPcHRpb25GbGFncywgQ1ZPcHRpb25GbGFncyos IHZvaWQqIGRhdGEpOwogICAgIAogICAgIENWRGlzcGxheUxpbmtSZWYgbV9kaXNwbGF5TGluayB7 IG51bGxwdHIgfTsKLSAgICBSZWY8V2ViUGFnZVByb3h5PiBtX3dlYlBhZ2VQcm94eTsKICAgICBI YXNoU2V0PHVuc2lnbmVkPiBtX29ic2VydmVyczsKIH07CiAK