185288 2018-05-03 20:09:13 -0700 Release assert in ScriptController::canExecuteScripts via HTMLMediaElement::~HTMLMediaElement() 2019-10-29 20:53:18 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=185284 InRadar P2 Normal --- 203600 1 rniwa rniwa eric.carlson jer.noble koivisto sabouhallawa zalan oldest_to_newest 1420534 0 rniwa 2018-05-03 20:09:13 -0700 e.g. Thread 0 Crashed: 0 WebCore 0x000000018af6e574 WebCore::ScriptController::canExecuteScripts(WebCore::ReasonForCallingCanExecuteScripts) + 552 (Source/WebCore/bindings/js/ScriptController.cpp:672) 1 WebCore 0x000000018af6e380 WebCore::ScriptController::canExecuteScripts(WebCore::ReasonForCallingCanExecuteScripts) + 52 (Source/WebCore/bindings/js/ScriptController.cpp:672) 2 WebCore 0x000000018b9a0b0c WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) + 316 (Source/WebCore/bindings/js/JSEventListener.cpp:113) 3 WebCore 0x000000018bbdd674 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>) + 760 (Source/WebCore/dom/EventTarget.cpp:289) 4 WebCore 0x000000018bbd922c WebCore::EventTarget::fireEventListeners(WebCore::Event&) + 596 (Source/WebCore/dom/EventTarget.cpp:231) 5 WebCore 0x000000018bf12140 WebCore::DOMWindow::dispatchEvent(WebCore::Event&, WebCore::EventTarget*) + 284 (Source/WebCore/page/DOMWindow.cpp:2053) 6 WebCore 0x000000018aff6b78 WebCore::DOMWindow::dispatchLoadEvent() + 160 (Source/WebCore/page/DOMWindow.cpp:2005) 7 WebCore 0x000000018afb3218 WebCore::Document::implicitClose() + 440 (Source/WebCore/dom/Document.cpp:4326) 8 WebCore 0x000000018afb282c WebCore::FrameLoader::checkCompleted() + 476 (Source/WebCore/loader/FrameLoader.cpp:910) 9 WebCore 0x000000018bef2e80 WebCore::CachedResourceLoader::loadDone(bool) + 84 (Source/WebCore/loader/cache/CachedResourceLoader.cpp:1287) 10 WebCore 0x000000018afe401c WebCore::SubresourceLoader::didCancel(WebCore::ResourceError const&) + 128 (Source/WebCore/loader/SubresourceLoader.cpp:699) 11 WebCore 0x000000018afe39d4 WebCore::ResourceLoader::cancel(WebCore::ResourceError const&) + 492 (Source/WebCore/loader/ResourceLoader.cpp:642) 12 WebCore 0x000000018afe3744 WebCore::ResourceLoader::cancel() + 64 (Source/WebCore/loader/ResourceLoader.cpp:598) 13 WebCore 0x000000018beec9b4 WebCore::CachedResource::removeClient(WebCore::CachedResourceClient&) + 264 (Source/WebCore/loader/cache/CachedResource.cpp:573) 14 WebCore 0x000000018beb0fec WebCore::MediaResource::~MediaResource() + 64 (Source/WebCore/loader/MediaResourceLoader.cpp:132) 15 WebCore 0x000000018beb109c WebCore::MediaResource::~MediaResource() + 12 (Source/WebCore/loader/MediaResourceLoader.cpp:122) ... 25 WebCore 0x000000018b165774 WebCore::MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC() + 336 (Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:539) 26 WebCore 0x000000018b166274 WebCore::MediaPlayerPrivateAVFoundationObjC::~MediaPlayerPrivateAVFoundationObjC() + 12 (Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:521) 27 WebCore 0x000000018c04bd54 WebCore::MediaPlayer::~MediaPlayer() + 244 (/BuildRoot/Applications/Xcode.app/Contents/Developer/Toolchains/iOS11.4.xctoolchain/usr/include/c++/v1/memory:2239) 28 WebCore 0x000000018c04bdbc WebCore::MediaPlayer::~MediaPlayer() + 12 (Source/WebCore/platform/graphics/MediaPlayer.cpp:376) 29 WebCore 0x000000018bd32a68 WebCore::HTMLMediaElement::~HTMLMediaElement() + 1316 (/BuildRoot/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS11.4.Internal.sdk/usr/local/include/wtf/RefCounted.h:145) 30 WebCore 0x000000018bd9192c WebCore::HTMLVideoElement::~HTMLVideoElement() + 176 (Source/WebCore/html/HTMLVideoElement.h:38) 1420536 1 rniwa 2018-05-03 20:19:57 -0700 <rdar://problem/39738580> 1420541 2 339514 rniwa 2018-05-03 20:29:38 -0700 Created attachment 339514 Speculative fix 1420702 3 339514 sabouhallawa 2018-05-04 09:32:54 -0700 Comment on attachment 339514 Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=339514&action=review > Source/WebCore/ChangeLog:12 > + In long term, ResourceLoader::cancel should never fire load event synchronously: webkit.org/b/185284. This is confusing. How does cancelling loading the resource fire the load event? Should not it fire the abort event? > Source/WebCore/html/HTMLMediaElement.cpp:584 > + return !!s_destructorCount; I think there is no need for !!. > Source/WebCore/html/HTMLMediaElement.cpp:589 > + HTMLMediaElementDestructorScope() { ++s_destructorCount; } Do we expect s_destructorCount to be more than 1? Should not s_destructorCount be a bool, e.g. s_inHTMLMediaElementDestructor? Will the destructor of HTMLMediaElement call the destructor of itself or other HTMLMediaElements? I think neither should ever happen. 1420863 4 rniwa 2018-05-04 13:54:30 -0700 (In reply to Said Abou-Hallawa from comment #3) > Comment on attachment 339514 [details] > Speculative fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339514&action=review > > > Source/WebCore/ChangeLog:12 > > + In long term, ResourceLoader::cancel should never fire load event synchronously: webkit.org/b/185284. > > This is confusing. How does cancelling loading the resource fire the load > event? Should not it fire the abort event? When the last resource that had been blocking document's load event gets canceled, we fire load event on the document. > > Source/WebCore/html/HTMLMediaElement.cpp:589 > > + HTMLMediaElementDestructorScope() { ++s_destructorCount; } > > Do we expect s_destructorCount to be more than 1? Should not > s_destructorCount be a bool, e.g. s_inHTMLMediaElementDestructor? Will the > destructor of HTMLMediaElement call the destructor of itself or other > HTMLMediaElements? I think neither should ever happen. Hopefully that's the case but the media code is so complicated that such a guarantee is not self-evident. Because we're trying to create a very safe fix here, I'm opt'ing to use the safest approach, which is to use a counter. 1420904 5 339514 jer.noble 2018-05-04 15:13:45 -0700 Comment on attachment 339514 Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=339514&action=review r=me with nit. > Source/WebCore/loader/FrameLoader.cpp:811 > + if (HTMLMediaElement::isRunningDestructor()) { > + scheduleCheckCompleted(); We could really use an ASSERT_NOT_REACHED() here so that it crashes in debug builds. 1420910 6 rniwa 2018-05-04 15:26:17 -0700 (In reply to Jer Noble from comment #5) > Comment on attachment 339514 [details] > Speculative fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339514&action=review > > r=me with nit. > > > Source/WebCore/loader/FrameLoader.cpp:811 > > + if (HTMLMediaElement::isRunningDestructor()) { > > + scheduleCheckCompleted(); > > We could really use an ASSERT_NOT_REACHED() here so that it crashes in debug > builds. Sure, will do. 1420966 7 rniwa 2018-05-04 16:58:06 -0700 Committed r231392: <https://trac.webkit.org/changeset/231392> 339514 2018-05-03 20:29:38 -0700 2018-05-04 15:13:45 -0700 Speculative fix bug-185288-20180503202938.patch text/plain 4000 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIzMTMzNCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBACisyMDE4LTA1LTAzICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIFJlbGVhc2UgYXNzZXJ0IGluIFNj cmlwdENvbnRyb2xsZXI6OmNhbkV4ZWN1dGVTY3JpcHRzIHZpYSBIVE1MTWVkaWFFbGVtZW50Ojp+ SFRNTE1lZGlhRWxlbWVudCgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xODUyODgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGUgY3Jhc2ggaXMgY2F1c2VkIGJ5IEhUTUxNZWRpYUVsZW1lbnQ6On5IVE1M TWVkaWFFbGVtZW50IGNhbmNlbGluZyB0aGUgcmVzb3VyY2UgbG9hZCB2aWEgQ2FjaGVkUmVzb3Vy Y2UKKyAgICAgICAgd2hpY2ggZW5kcyB1cCBjYWxsaW5nIEZyYW1lTG9hZGVyOjpjaGVja0NvbXBs ZXRlZCgpIGFuZCBmaXJlIGxvYWQgZXZlbnQgc3luY2hyb25vdXNseS4gU3BlY3VsYXRpdmVseSBm aXgKKyAgICAgICAgdGhlIGNyYXNoIGJ5IHNjaGVkdWxpbmcgdGhlIGNoZWNrIGluc3RlYWQuCisK KyAgICAgICAgSW4gbG9uZyB0ZXJtLCBSZXNvdXJjZUxvYWRlcjo6Y2FuY2VsIHNob3VsZCBuZXZl ciBmaXJlIGxvYWQgZXZlbnQgc3luY2hyb25vdXNseTogd2Via2l0Lm9yZy9iLzE4NTI4NC4KKwor ICAgICAgICBVbmZvcnR1bmF0ZWx5LCBubyBuZXcgdGVzdHMuIEkgY2FuJ3QgZ2V0IE1lZGlhUmVz b3VyY2UgdG8gZ2V0IGRlc3RydWN0ZWQgYXQgdGhlIHJpZ2h0IHRpbWUgOigKKworICAgICAgICAq IGh0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFF bGVtZW50Ojppc1J1bm5pbmdEZXN0cnVjdG9yKTogQWRkZWQgdG8gZGV0ZWN0IHRoaXMgc3BlY2lm aWMgY2FzZS4KKyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnREZXN0cnVjdG9yU2Nv cGUpOiBBZGRlZC4KKyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnREZXN0cnVjdG9y U2NvcGU6OkhUTUxNZWRpYUVsZW1lbnREZXN0cnVjdG9yU2NvcGUpOiBBZGRlZC4KKyAgICAgICAg KFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnREZXN0cnVjdG9yU2NvcGU6On5IVE1MTWVkaWFFbGVt ZW50RGVzdHJ1Y3RvclNjb3BlKTogQWRkZWQuCisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFF bGVtZW50Ojp+SFRNTE1lZGlhRWxlbWVudCk6IEluc3RhbnRpYXRlIEhUTUxNZWRpYUVsZW1lbnQu CisgICAgICAgICogaHRtbC9IVE1MTWVkaWFFbGVtZW50Lmg6CisgICAgICAgICogbG9hZGVyL0Zy YW1lTG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkZyYW1lTG9hZGVyOjpjaGVja0NvbXBs ZXRlZCk6IENhbGwgc2NoZWR1bGVDaGVja0NvbXBsZXRlZCBpbnN0ZWFkIG9mIHN5bmNocm9ub3Vz bHkgY2FsbGluZworICAgICAgICBjaGVja0NvbXBsZXRlZCBpZiB3ZSdyZSBpbiB0aGUgbWlkZGxl IG9mIGRlc3RydWN0aW5nIGEgSFRNTE1lZGlhRWxlbWVudC4KKwogMjAxOC0wNS0wMyAgWXVzdWtl IFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWlsLmNvbT4KIAogICAgICAgICBVc2UgcG9pbnRlciBp bnN0ZWFkIG9mIHN0ZDo6b3B0aW9uYWw8VCY+CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1sL0hU TUxNZWRpYUVsZW1lbnQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2h0bWwvSFRN TE1lZGlhRWxlbWVudC5jcHAJKHJldmlzaW9uIDIzMTMzNCkKKysrIFNvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTU3Niw4ICs1NzYsMjMg QEAgdm9pZCBIVE1MTWVkaWFFbGVtZW50OjpmaW5pc2hJbml0aWFsaXphdAogICAgIG1lZGlhU2Vz c2lvbigpLmNsaWVudFdpbGxCZWdpbkF1dG9wbGF5aW5nKCk7CiB9CiAKKy8vIEZJWE1FOiBSZW1v dmUgdGhpcyBjb2RlIG9uY2UgaHR0cHM6Ly93ZWJraXQub3JnL2IvMTg1Mjg0IGlzIGZpeGVkLgor c3RhdGljIHVuc2lnbmVkIHNfZGVzdHJ1Y3RvckNvdW50ID0gMDsKKworYm9vbCBIVE1MTWVkaWFF bGVtZW50Ojppc1J1bm5pbmdEZXN0cnVjdG9yKCkKK3sKKyAgICByZXR1cm4gISFzX2Rlc3RydWN0 b3JDb3VudDsKK30KKworY2xhc3MgSFRNTE1lZGlhRWxlbWVudERlc3RydWN0b3JTY29wZSB7Citw dWJsaWM6CisgICAgSFRNTE1lZGlhRWxlbWVudERlc3RydWN0b3JTY29wZSgpIHsgKytzX2Rlc3Ry dWN0b3JDb3VudDsgfQorICAgIH5IVE1MTWVkaWFFbGVtZW50RGVzdHJ1Y3RvclNjb3BlKCkgeyAt LXNfZGVzdHJ1Y3RvckNvdW50OyB9Cit9OworCiBIVE1MTWVkaWFFbGVtZW50Ojp+SFRNTE1lZGlh RWxlbWVudCgpCiB7CisgICAgSFRNTE1lZGlhRWxlbWVudERlc3RydWN0b3JTY29wZSBkZXN0cnVj dG9yU2NvcGU7CiAgICAgQUxXQVlTX0xPRyhMT0dJREVOVElGSUVSKTsKIAogICAgIGJlZ2luSWdu b3JpbmdUcmFja0Rpc3BsYXlVcGRhdGVSZXF1ZXN0cygpOwpJbmRleDogU291cmNlL1dlYkNvcmUv aHRtbC9IVE1MTWVkaWFFbGVtZW50LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRt bC9IVE1MTWVkaWFFbGVtZW50LmgJKHJldmlzaW9uIDIzMTMzNCkKKysrIFNvdXJjZS9XZWJDb3Jl L2h0bWwvSFRNTE1lZGlhRWxlbWVudC5oCSh3b3JraW5nIGNvcHkpCkBAIC0xNTcsNiArMTU3LDgg QEAgcHVibGljOgogCiAgICAgc3RhdGljIEhUTUxNZWRpYUVsZW1lbnQqIGJlc3RNZWRpYUVsZW1l bnRGb3JTaG93aW5nUGxheWJhY2tDb250cm9sc01hbmFnZXIoTWVkaWFFbGVtZW50U2Vzc2lvbjo6 UGxheWJhY2tDb250cm9sc1B1cnBvc2UpOwogCisgICAgc3RhdGljIGJvb2wgaXNSdW5uaW5nRGVz dHJ1Y3RvcigpOworCiAgICAgV0VCQ09SRV9FWFBPUlQgdm9pZCByZXdpbmQoZG91YmxlIHRpbWVE ZWx0YSk7CiAgICAgV0VCQ09SRV9FWFBPUlQgdm9pZCByZXR1cm5Ub1JlYWx0aW1lKCkgb3ZlcnJp ZGU7CiAKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2xvYWRlci9GcmFtZUxvYWRlci5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmNwcAkocmV2aXNpb24g MjMxMzM0KQorKysgU291cmNlL1dlYkNvcmUvbG9hZGVyL0ZyYW1lTG9hZGVyLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtODA1LDYgKzgwNSwxMiBAQCB2b2lkIEZyYW1lTG9hZGVyOjpjaGVja0NvbXBs ZXRlZCgpCiAgICAgLy8gSGF2ZSB3ZSBjb21wbGV0ZWQgYmVmb3JlPwogICAgIGlmIChtX2lzQ29t cGxldGUpCiAgICAgICAgIHJldHVybjsKKyAgICAKKyAgICAvLyBGSVhNRTogUmVtb3ZlIHRoaXMg Y29kZSBvbmNlIGh0dHBzOi8vd2Via2l0Lm9yZy9iLzE4NTI4NCBpcyBmaXhlZC4KKyAgICBpZiAo SFRNTE1lZGlhRWxlbWVudDo6aXNSdW5uaW5nRGVzdHJ1Y3RvcigpKSB7CisgICAgICAgIHNjaGVk dWxlQ2hlY2tDb21wbGV0ZWQoKTsKKyAgICAgICAgcmV0dXJuOworICAgIH0KIAogICAgIC8vIEZJ WE1FOiBJdCB3b3VsZCBiZSBiZXR0ZXIgaWYgcmVzb3VyY2UgbG9hZHMgd2VyZSBraWNrZWQgb2Zm IGFmdGVyIHJlbmRlciB0cmVlIHVwZGF0ZSAob3IgZGlkbid0IGNvbXBsZXRlIHN5bmNocm9ub3Vz bHkpLgogICAgIC8vICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTcxNzI5Cg==