148442 2015-08-25 13:09:20 -0700 Fix crash due to animationDidEnd called on deallocated RemoteLayerTreeHost 2015-08-26 14:55:01 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 wenson_hsieh webkit-unassigned commit-queue thorton webkit-bug-importer oldest_to_newest 1120694 0 wenson_hsieh 2015-08-25 13:09:20 -0700 WebKit may crash when calling -[PlatformCAAnimationRemote animationDidStop] if the PlatformCAAnimationRemote's layer tree host has been deallocated. 1120696 1 wenson_hsieh 2015-08-25 13:10:18 -0700 <rdar://problem/21609257> 1121039 2 wenson_hsieh 2015-08-26 13:33:50 -0700 Reproducible nearly 100% of the time -- visit http://whsieh.github.io/examples/layerhost-crash.html and click the link. Works best when running an iOS simulator on a debug build where you can see the stack trace upon the crash. 1121040 3 259975 wenson_hsieh 2015-08-26 13:35:17 -0700 Created attachment 259975 Test case that reproduces the bug Click the link to open a new tab. The new tab will crash while exiting. 1121043 4 259977 wenson_hsieh 2015-08-26 13:41:53 -0700 Created attachment 259977 Patch 1121046 5 259977 thorton 2015-08-26 13:48:31 -0700 Comment on attachment 259977 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259977&action=review > Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:145 > m_layers.remove(layerID); 1) You can do the hash table lookup only once. 2) Should you also cover the case in clearLayers()? 1121050 6 259977 wenson_hsieh 2015-08-26 13:57:44 -0700 Comment on attachment 259977 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259977&action=review >> Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:145 >> m_layers.remove(layerID); > > 1) You can do the hash table lookup only once. > > 2) Should you also cover the case in clearLayers()? 1. Good point. Fixed. 2. clearLayers is only invoked in the destructor of RemoteLayerTreeHost, which ensures that all animation delegates are first invalidated. It might be better to lift the invalidation logic from ~RemoteLayerTreeHost to clearLayers() instead though. 1121051 7 thorton 2015-08-26 14:00:18 -0700 (In reply to comment #6) > Comment on attachment 259977 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259977&action=review > > >> Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:145 > >> m_layers.remove(layerID); > > > > 1) You can do the hash table lookup only once. > > > > 2) Should you also cover the case in clearLayers()? > > 1. Good point. Fixed. > > 2. clearLayers is only invoked in the destructor of RemoteLayerTreeHost, > which ensures that all animation delegates are first invalidated. It might > be better to lift the invalidation logic from ~RemoteLayerTreeHost to > clearLayers() instead though. Ah! No, that's probably fine. OK. 1121054 8 259980 wenson_hsieh 2015-08-26 14:05:58 -0700 Created attachment 259980 Patch 1121056 9 thorton 2015-08-26 14:07:38 -0700 Please consider making your test a layout test (it seems like a pretty easy one). 1121067 10 259980 commit-queue 2015-08-26 14:54:57 -0700 Comment on attachment 259980 Patch Clearing flags on attachment: 259980 Committed r188991: <http://trac.webkit.org/changeset/188991> 1121068 11 commit-queue 2015-08-26 14:55:01 -0700 All reviewed patches have been landed. Closing bug. 259975 2015-08-26 13:35:17 -0700 2015-08-26 13:35:17 -0700 Test case that reproduces the bug layerhost-crash.zip application/zip 872 wenson_hsieh UEsDBBQACAAIAOBqGkcAAAAAAAAAAAAAAAAYABAAbGF5ZXJob3N0LWNyYXNoLXN1Yi5odG1sVVgM AL8i3lUkIN5V9QEUAI1Ty26DMBC88xUWVSWQEkiuKcmhVQ+959KjwRuwYryRvYSiKv9eG/IgiarE F7zjmWVWHmcV1WoVBFkFXKwC5lZmCyN3NBSbRhckUTML1OyimP0GPX51RqZb44fhtuoJbLT23DAh 92zJBBZNDZqSEuhTgd++d18iCgkshfHblcxJEkudgqSFfCtpbbi2GzS1axSSLxQn+I7mu584fEIq e59OO7cMuIWp1FNs6EbqZlzLGtxBdJrtbqCH7jRquOn72NY/omcN+dVKLbBNCoUWovi+12HCZjew g+azMXg470Z/vtzumD5Qs/QUlmN0/IRDcnIU3chqzU0p9YLNTvL+8+Ivf8TKebEtDTZaTAtUaBYs Vw1cDLZSULXwNl4vYAWyrGiMns0Ndo7hdinvTaFWyMUyPEY6dAQfUemgPourLHW1Q1NP948jHV7J H1BLBwjmIoDDSgEAAC0DAABQSwMEFAAIAAgA4GoaRwAAAAAAAAAAAAAAABQAEABsYXllcmhvc3Qt Y3Jhc2guaHRtbFVYDAC/It5VJCDeVfUBFABNUMtug0AMvPMV7p4SqQT1HODSj+jZWVytFdZGu6YU Rfn38orUOdljeWbsOljs26KoA2HXFrCgjmQIgpEa98M0DZrMgVcxEmscCxtjX2aPPTUf7xDxl+MY X4Q7VLJPPNjefI/ijVVAB5Ivlk6n0xke22zFtFGXdXpyPc6UgmYrfcIcyjzeLmtKd75uC89dv3oZ HNGXG27azYc7gorv2d8b99/z6trPlYVAiQANZh0T6CSQON/f6goXvWrXWYr9OX9QSwcIcTXpCsAA AAAkAQAAUEsBAhUDFAAIAAgA4GoaR+YigMNKAQAALQMAABgADAAAAAAAAAAAQKSBAAAAAGxheWVy aG9zdC1jcmFzaC1zdWIuaHRtbFVYCAC/It5VJCDeVVBLAQIVAxQACAAIAOBqGkdxNekKwAAAACQB AAAUAAwAAAAAAAAAAECkgaABAABsYXllcmhvc3QtY3Jhc2guaHRtbFVYCAC/It5VJCDeVVBLBQYA AAAAAgACAKAAAACyAgAAAAA= 259977 2015-08-26 13:41:53 -0700 2015-08-26 14:05:53 -0700 Patch bug-148442-20150826134142.patch text/plain 1818 wenson_hsieh U3VidmVyc2lvbiBSZXZpc2lvbjogMTg4OTMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMmJhM2QwYzEyMDJlMTVj NTYxMWNjNmRkYzkxY2ZjZTliODBjYmU5NC4uYmJkNmUxMzZkNDliMjQ5OTgyZmFlZTVlOWVhNzQ5 MWJmNmYzM2ZhZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA4LTI2ICBXZW5z b24gSHNpZWggIDx3ZW5zb25faHNpZWhAYXBwbGUuY29tPgorCisgICAgICAgIEZpeCBjcmFzaCBk dWUgdG8gYW5pbWF0aW9uRGlkRW5kIGNhbGxlZCBvbiBkZWFsbG9jYXRlZCBSZW1vdGVMYXllclRy ZWVIb3N0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NDg0NDIKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzIxNjA5MjU3PgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEEgUGxhdGZvcm1DQUFuaW1hdGlvblJl bW90ZSdzIGJhY2twb2ludGVyIHRvIGEgZGVhbGxvY2F0ZWQgUmVtb3RlTGF5ZXJUcmVlSG9zdCBp cyBub3QKKyAgICAgICAgaW52YWxpZGF0ZWQgd2hlbiBpdHMgaG9zdCByZW1vdmVzIGl0cyByZWZl cmVuY2UgdG8gaXQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvbWFjL1JlbW90ZUxheWVyVHJlZUhv c3QubW06CisgICAgICAgIChXZWJLaXQ6OlJlbW90ZUxheWVyVHJlZUhvc3Q6OmxheWVyV2lsbEJl UmVtb3ZlZCk6IEludmFsaWRhdGUgYSBiYWNrcG9pbnRlciBmcm9tIHRoZQorICAgICAgICAgICAg UGxhdGZvcm1DQUFuaW1hdGlvblJlbW90ZXMgdG8gdGhlIFJlbW90ZUxheWVyVHJlZUhvc3QuCisK IDIwMTUtMDgtMjUgIEJldGggRGFraW4gIDxiZGFraW5AYXBwbGUuY29tPgogCiAgICAgICAgIExv bmcgcHJlc3MgZ2VzdHVyZSByZWNvZ25pemVyIHNob3VsZCBhZGp1c3QgZGVsYXkgYmFzZWQgb24g b3RoZXIgcmVjb2duaXplcnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9t YWMvUmVtb3RlTGF5ZXJUcmVlSG9zdC5tbSBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMv UmVtb3RlTGF5ZXJUcmVlSG9zdC5tbQppbmRleCA0N2Q0OTYzNTIyMDI1Mjc4ZDQzODE0MWYzZTEw Mjg4OTllMmJjOWViLi44OTYyOTNlZjkxMjNiNjlkYzUyMmFhMjBjMDhjNTVhNzVlOGU1Y2QyIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1JlbW90ZUxheWVyVHJlZUhv c3QubW0KKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21hYy9SZW1vdGVMYXllclRyZWVI b3N0Lm1tCkBAIC0xMzgsNyArMTM4LDEwIEBAIExheWVyT3JWaWV3ICpSZW1vdGVMYXllclRyZWVI b3N0OjpnZXRMYXllcihHcmFwaGljc0xheWVyOjpQbGF0Zm9ybUxheWVySUQgbGF5ZXJJCiAKIHZv aWQgUmVtb3RlTGF5ZXJUcmVlSG9zdDo6bGF5ZXJXaWxsQmVSZW1vdmVkKFdlYkNvcmU6OkdyYXBo aWNzTGF5ZXI6OlBsYXRmb3JtTGF5ZXJJRCBsYXllcklEKQogewotICAgIG1fYW5pbWF0aW9uRGVs ZWdhdGVzLnJlbW92ZShsYXllcklEKTsKKyAgICBpZiAobV9hbmltYXRpb25EZWxlZ2F0ZXMuY29u dGFpbnMobGF5ZXJJRCkpIHsKKyAgICAgICAgW21fYW5pbWF0aW9uRGVsZWdhdGVzLmdldChsYXll cklEKSBpbnZhbGlkYXRlXTsKKyAgICAgICAgbV9hbmltYXRpb25EZWxlZ2F0ZXMucmVtb3ZlKGxh eWVySUQpOworICAgIH0KICAgICBtX2xheWVycy5yZW1vdmUobGF5ZXJJRCk7CiB9CiAK 259980 2015-08-26 14:05:58 -0700 2015-08-26 14:54:57 -0700 Patch bug-148442-20150826140547.patch text/plain 1842 wenson_hsieh U3VidmVyc2lvbiBSZXZpc2lvbjogMTg4OTMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMmJhM2QwYzEyMDJlMTVj NTYxMWNjNmRkYzkxY2ZjZTliODBjYmU5NC4uYmJkNmUxMzZkNDliMjQ5OTgyZmFlZTVlOWVhNzQ5 MWJmNmYzM2ZhZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA4LTI2ICBXZW5z b24gSHNpZWggIDx3ZW5zb25faHNpZWhAYXBwbGUuY29tPgorCisgICAgICAgIEZpeCBjcmFzaCBk dWUgdG8gYW5pbWF0aW9uRGlkRW5kIGNhbGxlZCBvbiBkZWFsbG9jYXRlZCBSZW1vdGVMYXllclRy ZWVIb3N0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NDg0NDIKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzIxNjA5MjU3PgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEEgUGxhdGZvcm1DQUFuaW1hdGlvblJl bW90ZSdzIGJhY2twb2ludGVyIHRvIGEgZGVhbGxvY2F0ZWQgUmVtb3RlTGF5ZXJUcmVlSG9zdCBp cyBub3QKKyAgICAgICAgaW52YWxpZGF0ZWQgd2hlbiBpdHMgaG9zdCByZW1vdmVzIGl0cyByZWZl cmVuY2UgdG8gaXQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvbWFjL1JlbW90ZUxheWVyVHJlZUhv c3QubW06CisgICAgICAgIChXZWJLaXQ6OlJlbW90ZUxheWVyVHJlZUhvc3Q6OmxheWVyV2lsbEJl UmVtb3ZlZCk6IEludmFsaWRhdGUgYSBiYWNrcG9pbnRlciBmcm9tIHRoZQorICAgICAgICAgICAg UGxhdGZvcm1DQUFuaW1hdGlvblJlbW90ZXMgdG8gdGhlIFJlbW90ZUxheWVyVHJlZUhvc3QuCisK IDIwMTUtMDgtMjUgIEJldGggRGFraW4gIDxiZGFraW5AYXBwbGUuY29tPgogCiAgICAgICAgIExv bmcgcHJlc3MgZ2VzdHVyZSByZWNvZ25pemVyIHNob3VsZCBhZGp1c3QgZGVsYXkgYmFzZWQgb24g b3RoZXIgcmVjb2duaXplcnMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9t YWMvUmVtb3RlTGF5ZXJUcmVlSG9zdC5tbSBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMv UmVtb3RlTGF5ZXJUcmVlSG9zdC5tbQppbmRleCA0N2Q0OTYzNTIyMDI1Mjc4ZDQzODE0MWYzZTEw Mjg4OTllMmJjOWViLi5iZmE5ODRiOThlNTc4Y2MzMmIwYzM0MTFiNGQ2M2MzZjYxYmU0ZWRlIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1JlbW90ZUxheWVyVHJlZUhv c3QubW0KKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21hYy9SZW1vdGVMYXllclRyZWVI b3N0Lm1tCkBAIC0xMzgsNyArMTM4LDExIEBAIExheWVyT3JWaWV3ICpSZW1vdGVMYXllclRyZWVI b3N0OjpnZXRMYXllcihHcmFwaGljc0xheWVyOjpQbGF0Zm9ybUxheWVySUQgbGF5ZXJJCiAKIHZv aWQgUmVtb3RlTGF5ZXJUcmVlSG9zdDo6bGF5ZXJXaWxsQmVSZW1vdmVkKFdlYkNvcmU6OkdyYXBo aWNzTGF5ZXI6OlBsYXRmb3JtTGF5ZXJJRCBsYXllcklEKQogewotICAgIG1fYW5pbWF0aW9uRGVs ZWdhdGVzLnJlbW92ZShsYXllcklEKTsKKyAgICBhdXRvIGl0ZXIgPSBtX2FuaW1hdGlvbkRlbGVn YXRlcy5maW5kKGxheWVySUQpOworICAgIGlmIChpdGVyICE9IG1fYW5pbWF0aW9uRGVsZWdhdGVz LmVuZCgpKSB7CisgICAgICAgIFtpdGVyLT52YWx1ZSBpbnZhbGlkYXRlXTsKKyAgICAgICAgbV9h bmltYXRpb25EZWxlZ2F0ZXMucmVtb3ZlKGl0ZXIpOworICAgIH0KICAgICBtX2xheWVycy5yZW1v dmUobGF5ZXJJRCk7CiB9CiAK