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)
<rdar://problem/39738580>
Created attachment 339514 [details] Speculative fix
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? > 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.
(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.
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.
(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.
Committed r231392: <https://trac.webkit.org/changeset/231392>