120792 2013-09-05 13:52:09 -0700 WKFullScreenWindowController extends lifetime of WKView, deleting it at a wrong time 2013-09-05 15:06:56 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ap ap eric.carlson jer.noble oldest_to_newest 925196 0 ap 2013-09-05 13:52:09 -0700 WKFullScreenWindowController has a strong reference to WKView, so when it is instantiated, the view is only deleted when the controller is deleted. This finally happens in WebPageProxy::resetStateAfterProcessExited() when it resets m_fullScreenManager. But further calls in this function rely on WKView and its associated clients still being there. So, we crash. I don't see why WKFullScreenWindowController needs to retain the view - when the view is deleted, there is no need for the controller anyway, everything is just closed normally. <rdar://problem/14884666> 925200 1 210659 ap 2013-09-05 14:00:44 -0700 Created attachment 210659 proposed fix 925234 2 210659 jer.noble 2013-09-05 14:54:46 -0700 Comment on attachment 210659 proposed fix LGTM, r=me with nit. The only thing I worry about is the lifetime of the WKFullScreenWindow being extended (due to being retained by something other than the WKView, or by a -performSelector:afterDelay:) somehow, and thus holding onto an invalid _webView pointer. Perhaps the _webView could stay unretained, but the -setWebView: method could also stay, and would be cleared out by WKView in its own -dealloc method? We could even add a "@property (assign) WKView* webView" declaration to make the semantic explicit. 925237 3 ap 2013-09-05 15:06:56 -0700 That's a great nit. I added an assignment to nil to -close, as this function is inevitably called when WKView is destroyed. Committed <http://trac.webkit.org/r155153>. 210659 2013-09-05 14:00:44 -0700 2013-09-05 14:54:46 -0700 proposed fix FullScreen.txt text/plain 4419 ap SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE1NTE0NCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIzIEBACisyMDEzLTA5LTA1ICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIFdLRnVsbFNjcmVlbldpbmRv d0NvbnRyb2xsZXIgZXh0ZW5kcyBsaWZldGltZSBvZiBXS1ZpZXcsIGRlbGV0aW5nIGl0IGF0IGEg d3JvbmcgdGltZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTIwNzkyCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xNDg4NDY2Nj4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvbWFj L1dLVmlldy5tbTogKC1bV0tWaWV3IGZ1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyXSk6CisgICAg ICAgIFVzZSBhIG5ld2x5IG1pbnRlZCBpbml0aWFsaXplciBmb3IgdGhlIGNvbnRyb2xsZXIuCisK KyAgICAgICAgKiBVSVByb2Nlc3MvbWFjL1dLRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIuaDog UmVtb3ZlZCB1bnVzZWQgd2ViIHZpZXcgYWNjZXNzb3JzLgorICAgICAgICBDaGFuZ2VkIHRoZSBj bGFzcyB0byB0YWtlIHdlYiB2aWV3IGF0IGluaXRpYWxpemF0aW9uIHRpbWUuCisKKyAgICAgICAg KiBVSVByb2Nlc3MvbWFjL1dLRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIubW06CisgICAgICAg ICgtW1dLRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIgaW5pdFdpdGhXaW5kb3c6d2ViVmlldzpd KTogSW5pdGlhbGl6ZSB0aGUgY29udHJvbGxlcgorICAgICAgICBpbm9uZSBzdGVwLgorICAgICAg ICAoLVtXS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyIGRlYWxsb2NdKTogV2ViVmlldyBpcyBu b3cgYSByYXcgcG9pbnRlciwgbm8gbmVlZAorICAgICAgICB0byB6ZXJvIGl0LgorCiAyMDEzLTA5 LTA1ICBBbmRlcnMgQ2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CiAKICAgICAgICAgUmVw bGFjZSB1c2VzIG9mIENMQU5HX1BSQUdNQSB3aXRoICNwcmFnbWEKSW5kZXg6IFNvdXJjZS9XZWJL aXQyL1VJUHJvY2Vzcy9BUEkvbWFjL1dLVmlldy5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9VSVByb2Nlc3MvQVBJL21hYy9XS1ZpZXcubW0JKHJldmlzaW9uIDE1NTEyMikKKysrIFNv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvbWFjL1dLVmlldy5tbQkod29ya2luZyBjb3B5KQpA QCAtMjkzNywxMCArMjkzNyw5IEBAIC0gKEJPT0wpaGFzRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xs ZXIKIAogLSAoV0tGdWxsU2NyZWVuV2luZG93Q29udHJvbGxlciopZnVsbFNjcmVlbldpbmRvd0Nv bnRyb2xsZXIKIHsKLSAgICBpZiAoIV9kYXRhLT5fZnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIp IHsKLSAgICAgICAgX2RhdGEtPl9mdWxsU2NyZWVuV2luZG93Q29udHJvbGxlciA9IGFkb3B0TlMo W1tXS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyIGFsbG9jXSBpbml0V2l0aFdpbmRvdzpbc2Vs ZiBjcmVhdGVGdWxsU2NyZWVuV2luZG93XV0pOwotICAgICAgICBbX2RhdGEtPl9mdWxsU2NyZWVu V2luZG93Q29udHJvbGxlci5nZXQoKSBzZXRXZWJWaWV3OnNlbGZdOwotICAgIH0KKyAgICBpZiAo IV9kYXRhLT5fZnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIpCisgICAgICAgIF9kYXRhLT5fZnVs bFNjcmVlbldpbmRvd0NvbnRyb2xsZXIgPSBhZG9wdE5TKFtbV0tGdWxsU2NyZWVuV2luZG93Q29u dHJvbGxlciBhbGxvY10gaW5pdFdpdGhXaW5kb3c6W3NlbGYgY3JlYXRlRnVsbFNjcmVlbldpbmRv d10gd2ViVmlldzpzZWxmXSk7CisKICAgICByZXR1cm4gX2RhdGEtPl9mdWxsU2NyZWVuV2luZG93 Q29udHJvbGxlci5nZXQoKTsKIH0KIApJbmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21h Yy9XS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyLmgKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL21hYy9XS0Z1bGxTY3JlZW5XaW5kb3dDb250cm9sbGVyLmgJKHJldmlz aW9uIDE1NTEyMikKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV0tGdWxsU2NyZWVu V2luZG93Q29udHJvbGxlci5oCSh3b3JraW5nIGNvcHkpCkBAIC00Niw3ICs0Niw3IEBAIHR5cGVk ZWYgZW51bSBGdWxsU2NyZWVuU3RhdGUgOiBOU0ludGVnZXIKIAogQGludGVyZmFjZSBXS0Z1bGxT Y3JlZW5XaW5kb3dDb250cm9sbGVyIDogTlNXaW5kb3dDb250cm9sbGVyPE5TV2luZG93RGVsZWdh dGU+IHsKIEBwcml2YXRlCi0gICAgV0tWaWV3ICpfd2ViVmlldzsKKyAgICBXS1ZpZXcgKl93ZWJW aWV3OyAvLyBDYW5ub3QgYmUgcmV0YWluZWQsIHNlZSA8cmRhcjovL3Byb2JsZW0vMTQ4ODQ2NjY+ LgogICAgIFJldGFpblB0cjxXZWJDb3JlRnVsbFNjcmVlblBsYWNlaG9sZGVyVmlldz4gX3dlYlZp ZXdQbGFjZWhvbGRlcjsKICAgICBSZXRhaW5QdHI8V2ViV2luZG93U2NhbGVBbmltYXRpb24+IF9z Y2FsZUFuaW1hdGlvbjsKICAgICBSZXRhaW5QdHI8V2ViV2luZG93RmFkZUFuaW1hdGlvbj4gX2Zh ZGVBbmltYXRpb247CkBAIC02MCw4ICs2MCw3IEBAIEBpbnRlcmZhY2UgV0tGdWxsU2NyZWVuV2lu ZG93Q29udHJvbGxlciAKICAgICBkb3VibGUgX3NhdmVkU2NhbGU7CiB9CiAKLS0gKFdLVmlldyop d2ViVmlldzsKLS0gKHZvaWQpc2V0V2ViVmlldzooV0tWaWV3Kil3ZWJWaWV3OworLSAoaWQpaW5p dFdpdGhXaW5kb3c6KE5TV2luZG93ICopd2luZG93IHdlYlZpZXc6KFdLVmlldyAqKXdlYlZpZXc7 CiAKIC0gKFdlYkNvcmVGdWxsU2NyZWVuUGxhY2Vob2xkZXJWaWV3Kil3ZWJWaWV3UGxhY2Vob2xk ZXI7CiAKSW5kZXg6IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV0tGdWxsU2NyZWVuV2lu ZG93Q29udHJvbGxlci5tbQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv bWFjL1dLRnVsbFNjcmVlbldpbmRvd0NvbnRyb2xsZXIubW0JKHJldmlzaW9uIDE1NTEyMikKKysr IFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV0tGdWxsU2NyZWVuV2luZG93Q29udHJvbGxl ci5tbQkod29ya2luZyBjb3B5KQpAQCAtODksNyArODksNyBAQCAtIChCT09MKWlzT25BY3RpdmVT cGFjZTsKIAogI3ByYWdtYSBtYXJrIC0KICNwcmFnbWEgbWFyayBJbml0aWFsaXphdGlvbgotLSAo aWQpaW5pdFdpdGhXaW5kb3c6KE5TV2luZG93ICopd2luZG93CistIChpZClpbml0V2l0aFdpbmRv dzooTlNXaW5kb3cgKil3aW5kb3cgd2ViVmlldzooV0tWaWV3ICopd2ViVmlldwogewogICAgIHNl bGYgPSBbc3VwZXIgaW5pdFdpdGhXaW5kb3c6d2luZG93XTsKICAgICBpZiAoIXNlbGYpCkBAIC05 NywxMyArOTcsMTMgQEAgLSAoaWQpaW5pdFdpdGhXaW5kb3c6KE5TV2luZG93ICopd2luZG93CiAg ICAgW3dpbmRvdyBzZXREZWxlZ2F0ZTpzZWxmXTsKICAgICBbd2luZG93IHNldENvbGxlY3Rpb25C ZWhhdmlvcjooW3dpbmRvdyBjb2xsZWN0aW9uQmVoYXZpb3JdIHwgTlNXaW5kb3dDb2xsZWN0aW9u QmVoYXZpb3JGdWxsU2NyZWVuUHJpbWFyeSldOwogICAgIFtzZWxmIHdpbmRvd0RpZExvYWRdOwor ICAgIF93ZWJWaWV3ID0gd2ViVmlldzsKICAgICAKICAgICByZXR1cm4gc2VsZjsKIH0KIAogLSAo dm9pZClkZWFsbG9jCiB7Ci0gICAgW3NlbGYgc2V0V2ViVmlldzpuaWxdOwogICAgIFtbc2VsZiB3 aW5kb3ddIHNldERlbGVnYXRlOm5pbF07CiAgICAgCiAgICAgW05TT2JqZWN0IGNhbmNlbFByZXZp b3VzUGVyZm9ybVJlcXVlc3RzV2l0aFRhcmdldDpzZWxmXTsKQEAgLTEyMiwxOCArMTIyLDYgQEAg LSAodm9pZCl3aW5kb3dEaWRMb2FkCiAjcHJhZ21hIG1hcmsgLQogI3ByYWdtYSBtYXJrIEFjY2Vz c29ycwogCi0tIChXS1ZpZXcqKXdlYlZpZXcKLXsKLSAgICByZXR1cm4gX3dlYlZpZXc7Ci19Ci0K LS0gKHZvaWQpc2V0V2ViVmlldzooV0tWaWV3ICopd2ViVmlldwotewotICAgIFt3ZWJWaWV3IHJl dGFpbl07Ci0gICAgW193ZWJWaWV3IHJlbGVhc2VdOwotICAgIF93ZWJWaWV3ID0gd2ViVmlldzsK LX0KLQogLSAoQk9PTClpc0Z1bGxTY3JlZW4KIHsKICAgICByZXR1cm4gX2Z1bGxTY3JlZW5TdGF0 ZSA9PSBXYWl0aW5nVG9FbnRlckZ1bGxTY3JlZW4K