179849 2017-11-17 16:34:03 -0800 downcast to WebKit::FullscreenClient can sometimes fail. 2018-05-23 06:43:20 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jeremyj-wk jeremyj-wk commit-queue darin fred.wang webkit-bug-importer oldest_to_newest 1373402 0 jeremyj-wk 2017-11-17 16:34:03 -0800 downcast to WebKit::FullscreenClient can sometimes fail. 1373411 1 327273 jeremyj-wk 2017-11-17 16:38:56 -0800 Created attachment 327273 Patch 1374044 2 327273 darin 2017-11-21 09:06:17 -0800 Comment on attachment 327273 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327273&action=review I worry that the issue is deeper than just a bd cast. If _setFullscreenDelegate: does nothing because we are on a different fullscreen client, then the old fullscreen client might be left with the delegate still set. That could be a dangling pointer. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4778 > return nullptr; I think this should be return nil. 1375108 3 jeremyj-wk 2017-11-27 13:55:41 -0800 (In reply to Darin Adler from comment #2) > Comment on attachment 327273 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=327273&action=review > > I worry that the issue is deeper than just a bd cast. If > _setFullscreenDelegate: does nothing because we are on a different > fullscreen client, then the old fullscreen client might be left with the > delegate still set. That could be a dangling pointer. The various client objects are owned by the page. When the page clears those clients, those objects and references are destroyed. Looking over WKWebView, I see the same unsafe pattern that caused this crash also affects the find delegate: https://bugs.webkit.org/show_bug.cgi?id=180054 > > > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4778 > > return nullptr; > > I think this should be return nil. Done. 1375110 4 327674 jeremyj-wk 2017-11-27 13:57:10 -0800 Created attachment 327674 Patch for landing. 1375133 5 327674 commit-queue 2017-11-27 14:29:36 -0800 Comment on attachment 327674 Patch for landing. Clearing flags on attachment: 327674 Committed r225195: <https://trac.webkit.org/changeset/225195> 1426555 6 fred.wang 2018-05-23 06:42:50 -0700 Closing now since patch landed. 1426556 7 webkit-bug-importer 2018-05-23 06:43:20 -0700 <rdar://problem/40484031> 327273 2017-11-17 16:38:56 -0800 2017-11-21 09:06:17 -0800 Patch bug-179849-20171117163855.patch text/plain 2207 jeremyj-wk U3VidmVyc2lvbiBSZXZpc2lvbjogMjI0OTk5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGRmZTVjZWNmMTY5ZDNhZjJl YjI0ZGMzZTI2NDQ4Y2YwMTNiZGMxNGQuLjNlMGYyMmMyMGNkNjQ1ZWVjODI3MWU2NzY2NGNiODg4 YzgyMTM5YWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTctMTEtMTcgIEplcmVteSBK b25lcyAgPGplcmVteWpAYXBwbGUuY29tPgorCisgICAgICAgIGRvd25jYXN0IHRvIFdlYktpdDo6 RnVsbHNjcmVlbkNsaWVudCBjYW4gc29tZXRpbWVzIGZhaWwuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzk4NDkKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGVyZSBhcmUgY2FzZXMgZHVyaW5nIHRlYXJk b3duIHdoZXJlIGZ1bGxzY3JlZW5DbGllbnQoKSBoYXMgYmVlbiBjbGVhcmVkIGJhY2sgdG8gYW4g QVBJOjpGdWxsc2NyZWVuQ2xpZW50LgorICAgICAgICBCZWNhdXNlIHRob3NlIGNhc2VzLCBXS1dl YlZpZXcgc2hvdWxkIHRlc3QgYmVmb3JlIGRvd25jYXN0aW5nIHRvIFdlYktpdDo6RnVsbHNjcmVl bkNsaWVudC4KKworICAgICAgICBUaGlzIGlzIGNhdXNpbmcgYSBjcmFzaCB3aGVuIGZ1bGxzY3Jl ZW4gZGVsZWdhdGUgaXMgY2xlYXJlZCBhZnRlciBhIHBhZ2UgaXMgY2xvc2VkLgorCisgICAgICAg ICogVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW06CisgICAgICAgICgtW1dLV2ViVmll dyBfc2V0RnVsbHNjcmVlbkRlbGVnYXRlOl0pOgorICAgICAgICAoLVtXS1dlYlZpZXcgX2Z1bGxz Y3JlZW5EZWxlZ2F0ZV0pOgorCiAyMDE3LTExLTE2ICBZb3VzdWtlIEtpbW90byAgPHlvdXN1a2Uu a2ltb3RvQHNvbnkuY29tPgogCiAgICAgICAgIFtXaW5DYWlyb10gQWRkIG5ldHdvcmsgKGN1cmwp IGZpbGVzIGZvciB3aW5jYWlybyB3ZWJraXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQ cm9jZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9B UEkvQ29jb2EvV0tXZWJWaWV3Lm1tCmluZGV4IDcyYzdjNTRmYjVmZGQ5ZDM0MWM1Y2YwOTVlZmZm NmY1NGIxOGEzZjYuLmM0ZWRjYjgwZGY5ZWZiNmZhNjE3YTIxNGMxOWY1ZmFiY2JmOWU1NWQgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW0K KysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0NvY29hL1dLV2ViVmlldy5tbQpAQCAt NDc2NCwxNyArNDc2NCwxOCBAQCAtIChCT09MKV93ZWJQcm9jZXNzSXNSZXNwb25zaXZlCiAtICh2 b2lkKV9zZXRGdWxsc2NyZWVuRGVsZWdhdGU6KGlkPF9XS0Z1bGxzY3JlZW5EZWxlZ2F0ZT4pZGVs ZWdhdGUKIHsKICNpZiBFTkFCTEUoRlVMTFNDUkVFTl9BUEkpCi0gICAgZG93bmNhc3Q8V2ViS2l0 OjpGdWxsc2NyZWVuQ2xpZW50PihfcGFnZS0+ZnVsbHNjcmVlbkNsaWVudCgpKS5zZXREZWxlZ2F0 ZShkZWxlZ2F0ZSk7CisgICAgaWYgKGlzPFdlYktpdDo6RnVsbHNjcmVlbkNsaWVudD4oX3BhZ2Ut PmZ1bGxzY3JlZW5DbGllbnQoKSkpCisgICAgICAgIGRvd25jYXN0PFdlYktpdDo6RnVsbHNjcmVl bkNsaWVudD4oX3BhZ2UtPmZ1bGxzY3JlZW5DbGllbnQoKSkuc2V0RGVsZWdhdGUoZGVsZWdhdGUp OwogI2VuZGlmCiB9CiAKIC0gKGlkPF9XS0Z1bGxzY3JlZW5EZWxlZ2F0ZT4pX2Z1bGxzY3JlZW5E ZWxlZ2F0ZQogewogI2lmIEVOQUJMRShGVUxMU0NSRUVOX0FQSSkKLSAgICByZXR1cm4gZG93bmNh c3Q8V2ViS2l0OjpGdWxsc2NyZWVuQ2xpZW50PihfcGFnZS0+ZnVsbHNjcmVlbkNsaWVudCgpKS5k ZWxlZ2F0ZSgpLmF1dG9yZWxlYXNlKCk7Ci0jZWxzZQorICAgIGlmIChpczxXZWJLaXQ6OkZ1bGxz Y3JlZW5DbGllbnQ+KF9wYWdlLT5mdWxsc2NyZWVuQ2xpZW50KCkpKQorICAgICAgICByZXR1cm4g ZG93bmNhc3Q8V2ViS2l0OjpGdWxsc2NyZWVuQ2xpZW50PihfcGFnZS0+ZnVsbHNjcmVlbkNsaWVu dCgpKS5kZWxlZ2F0ZSgpLmF1dG9yZWxlYXNlKCk7CisjZW5kaWYKICAgICByZXR1cm4gbnVsbHB0 cjsKLSNlbmRpZgogfQogCiAtIChCT09MKV9pc0luRnVsbHNjcmVlbgo= 327674 2017-11-27 13:57:10 -0800 2017-11-27 14:29:36 -0800 Patch for landing. bug-179849-20171127135709.patch text/plain 2213 jeremyj-wk U3VidmVyc2lvbiBSZXZpc2lvbjogMjI0OTk5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGRmZTVjZWNmMTY5ZDNhZjJl YjI0ZGMzZTI2NDQ4Y2YwMTNiZGMxNGQuLmY3MjE5Nzg0Y2QyNGZjOWRkOTIyZGEwNDUwY2EwOGRj ZjE3ZWY1MmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTctMTEtMTcgIEplcmVteSBK b25lcyAgPGplcmVteWpAYXBwbGUuY29tPgorCisgICAgICAgIGRvd25jYXN0IHRvIFdlYktpdDo6 RnVsbHNjcmVlbkNsaWVudCBjYW4gc29tZXRpbWVzIGZhaWwuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNzk4NDkKKworICAgICAgICBSZXZpZXdlZCBi eSBEYXJpbiBBZGxlci4KKworICAgICAgICBUaGVyZSBhcmUgY2FzZXMgZHVyaW5nIHRlYXJkb3du IHdoZXJlIGZ1bGxzY3JlZW5DbGllbnQoKSBoYXMgYmVlbiBjbGVhcmVkIGJhY2sgdG8gYW4gQVBJ OjpGdWxsc2NyZWVuQ2xpZW50LgorICAgICAgICBCZWNhdXNlIHRob3NlIGNhc2VzLCBXS1dlYlZp ZXcgc2hvdWxkIHRlc3QgYmVmb3JlIGRvd25jYXN0aW5nIHRvIFdlYktpdDo6RnVsbHNjcmVlbkNs aWVudC4KKworICAgICAgICBUaGlzIGlzIGNhdXNpbmcgYSBjcmFzaCB3aGVuIGZ1bGxzY3JlZW4g ZGVsZWdhdGUgaXMgY2xlYXJlZCBhZnRlciBhIHBhZ2UgaXMgY2xvc2VkLgorCisgICAgICAgICog VUlQcm9jZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW06CisgICAgICAgICgtW1dLV2ViVmlldyBf c2V0RnVsbHNjcmVlbkRlbGVnYXRlOl0pOgorICAgICAgICAoLVtXS1dlYlZpZXcgX2Z1bGxzY3Jl ZW5EZWxlZ2F0ZV0pOgorCiAyMDE3LTExLTE2ICBZb3VzdWtlIEtpbW90byAgPHlvdXN1a2Uua2lt b3RvQHNvbnkuY29tPgogCiAgICAgICAgIFtXaW5DYWlyb10gQWRkIG5ldHdvcmsgKGN1cmwpIGZp bGVzIGZvciB3aW5jYWlybyB3ZWJraXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9BUEkv Q29jb2EvV0tXZWJWaWV3Lm1tCmluZGV4IDcyYzdjNTRmYjVmZGQ5ZDM0MWM1Y2YwOTVlZmZmNmY1 NGIxOGEzZjYuLmJkNzEzYmE4ZGE1NTZiMjNjN2QzZmQ5YzlkODUwZjM1ZjJiMGQ0N2YgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1dlYlZpZXcubW0KKysr IGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL0NvY29hL1dLV2ViVmlldy5tbQpAQCAtNDc2 NCwxNyArNDc2NCwxOCBAQCAtIChCT09MKV93ZWJQcm9jZXNzSXNSZXNwb25zaXZlCiAtICh2b2lk KV9zZXRGdWxsc2NyZWVuRGVsZWdhdGU6KGlkPF9XS0Z1bGxzY3JlZW5EZWxlZ2F0ZT4pZGVsZWdh dGUKIHsKICNpZiBFTkFCTEUoRlVMTFNDUkVFTl9BUEkpCi0gICAgZG93bmNhc3Q8V2ViS2l0OjpG dWxsc2NyZWVuQ2xpZW50PihfcGFnZS0+ZnVsbHNjcmVlbkNsaWVudCgpKS5zZXREZWxlZ2F0ZShk ZWxlZ2F0ZSk7CisgICAgaWYgKGlzPFdlYktpdDo6RnVsbHNjcmVlbkNsaWVudD4oX3BhZ2UtPmZ1 bGxzY3JlZW5DbGllbnQoKSkpCisgICAgICAgIGRvd25jYXN0PFdlYktpdDo6RnVsbHNjcmVlbkNs aWVudD4oX3BhZ2UtPmZ1bGxzY3JlZW5DbGllbnQoKSkuc2V0RGVsZWdhdGUoZGVsZWdhdGUpOwog I2VuZGlmCiB9CiAKIC0gKGlkPF9XS0Z1bGxzY3JlZW5EZWxlZ2F0ZT4pX2Z1bGxzY3JlZW5EZWxl Z2F0ZQogewogI2lmIEVOQUJMRShGVUxMU0NSRUVOX0FQSSkKLSAgICByZXR1cm4gZG93bmNhc3Q8 V2ViS2l0OjpGdWxsc2NyZWVuQ2xpZW50PihfcGFnZS0+ZnVsbHNjcmVlbkNsaWVudCgpKS5kZWxl Z2F0ZSgpLmF1dG9yZWxlYXNlKCk7Ci0jZWxzZQotICAgIHJldHVybiBudWxscHRyOworICAgIGlm IChpczxXZWJLaXQ6OkZ1bGxzY3JlZW5DbGllbnQ+KF9wYWdlLT5mdWxsc2NyZWVuQ2xpZW50KCkp KQorICAgICAgICByZXR1cm4gZG93bmNhc3Q8V2ViS2l0OjpGdWxsc2NyZWVuQ2xpZW50PihfcGFn ZS0+ZnVsbHNjcmVlbkNsaWVudCgpKS5kZWxlZ2F0ZSgpLmF1dG9yZWxlYXNlKCk7CiAjZW5kaWYK KyAgICByZXR1cm4gbmlsOwogfQogCiAtIChCT09MKV9pc0luRnVsbHNjcmVlbgo=