goto http://ia600208.us.archive.org/13/items/WiththeMarinesatTarawa/With_the_Marines_at_Tarawa_1944.mpeg Select share via email Webkit will crash
Result of MainDocumentLoader refactoring? Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x000000010286c3e8 WebCore::MainResourceLoader::setDataBufferingPolicy(WebCore::DataBufferingPolicy) + 8 1 com.apple.WebCore 0x00000001021bd8de WebCore::MediaDocumentParser::createDocumentStructure() + 894 2 com.apple.WebCore 0x000000010287cd4f WebCore::MediaDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long) + 31 3 com.apple.WebCore 0x0000000102383f0c WebCore::DocumentLoader::commitData(char const*, unsigned long) + 508 4 com.apple.WebCore 0x0000000102383cc7 WebCore::DocumentLoader::maybeCreateArchive() + 455 5 com.apple.WebCore 0x0000000101dd65f1 WebCore::DocumentLoader::finishedLoading() + 81 6 com.apple.WebCore 0x0000000101e47f28 WebCore::MainResourceLoader::didFinishLoading(double) + 312 7 com.apple.WebCore 0x0000000101e42311 WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) + 1281 8 com.apple.WebCore 0x000000010286bc50 WebCore::MainResourceLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) + 1696
<rdar://problem/13235726>
Created attachment 189151 [details] patch Tested manually, don't know enough about Safari's sharing feature to know whether it's possible to convert this to a layout test.
I suspect that this happens when loading a WebArchive with a <video> in it. Perhaps that would be possible to test?
(In reply to comment #4) > I suspect that this happens when loading a WebArchive with a <video> in it. Perhaps that would be possible to test? I can't find an example of a case where the webarchive is the test, rather than the output. Am I missing something obvious?
I think that there is a bunch of tests in webarchive/loading, e.g. webarchive/loading/test-loading-archive.html. It does take two steps, as DRT doesn't look for tests with webarchive extension.
(In reply to comment #6) > I think that there is a bunch of tests in webarchive/loading, e.g. webarchive/loading/test-loading-archive.html. It does take two steps, as DRT doesn't look for tests with webarchive extension. Thanks for the pointer. Looks like I can make that style of test work, though it'll take a little bit of messing around to get it just right.
Created attachment 189411 [details] + test
Nightly build r143504 did not display the menu for sharing the email of http://ia600208.us.archive.org/13/items/WiththeMarinesatTarawa/With_the_Marines_at_Tarawa_1944.mpeg Opening the current released version of Safari to the same site would not display the video but did display the share menu. Returning to WebKit r143504 the video was visible, the share email menu was not… then suddenly Webkit crashed
Comment on attachment 189411 [details] + test View in context: https://bugs.webkit.org/attachment.cgi?id=189411&action=review > Source/WebCore/ChangeLog:28 > +2013-02-19 Nate Chapin <japhet@chromium.org> > + > + > + > + Reviewed by NOBODY (OOPS!). > + > + * loader/MainResourceLoader.cpp: > + (WebCore::MainResourceLoader::setDataBufferingPolicy): m_resource can be null > + in a reasonable case, handle it rather than asserting that it can't be null. Parts of a previous ChangeLog here. > LayoutTests/webarchive/loading/video-in-webarchive-expected.txt:13 > +frame "<!--framePath //<!--frame0-->-->" - didFailLoadWithError What fails here? Is that expected?
Created attachment 189420 [details] Additional information from latest build r143504 Sorry.. put this in without the attachment. Nightly build r143504 did not display the menu for sharing the email of http://ia600208.us.archive.org/13/items/WiththeMarinesatTarawa/With_the_Marines_at_Tarawa_1944.mpeg Opening the current released version of Safari to the same site would not display the video but did display the share menu. Returning to WebKit r143504 the video was visible, the share email menu was not… then suddenly Webkit crashed
Comment on attachment 189411 [details] + test View in context: https://bugs.webkit.org/attachment.cgi?id=189411&action=review >> LayoutTests/webarchive/loading/video-in-webarchive-expected.txt:13 >> +frame "<!--framePath //<!--frame0-->-->" - didFailLoadWithError > > What fails here? Is that expected? The iframe in the webarchive points to a 0-byte mpeg. I'd guess that's the cause, but I'm not 100% sure.
Created attachment 189426 [details] Fix ChangeLog
Comment on attachment 189426 [details] Fix ChangeLog > The iframe in the webarchive points to a 0-byte mpeg. I'd guess that's the cause, but I'm not 100% sure. This looks more like a bug than like correct behavior. This happens because we cancel main resource load when switching to a media document, and that somehow translates into a frame loader failure. But it's a resource load cancellation, not a frame load failure. // If the document is a stand-alone media document, now is the right time to cancel the WebKit load. // FIXME: This code should be shared across all ports. <http://webkit.org/b/48762>. if (m_frame->coreFrame()->document()->isMediaDocument()) loader->cancelMainResourceLoad(pluginWillHandleLoadError(loader->response())); #0 0x000000010128afcc in WebKit::WebFrameLoaderClient::dispatchDidFailLoading(WebCore::DocumentLoader*, unsigned long, WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:249 #1 0x00000001046df8af in WebCore::MainResourceLoader::receivedError(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:99 #2 0x00000001046dfb27 in WebCore::MainResourceLoader::cancel(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:132 #3 0x000000010399a11f in WebCore::DocumentLoader::cancelMainResourceLoad(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:913 #4 0x000000010128db82 in WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:852 #5 0x0000000103997b20 in WebCore::DocumentLoader::commitLoad(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:318 #6 0x00000001039980cb in WebCore::DocumentLoader::receivedData(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:397 #7 0x00000001046e1f4a in WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:522 #8 0x00000001046e0e20 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:365 #9 0x00000001046e1266 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:381 #10 0x00000001046e11ab in WebCore::MainResourceLoader::callContinueAfterContentPolicy(void*, WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:373 #11 0x00000001046e18c6 in WebCore::MainResourceLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:450 #12 0x00000001046df594 in WebCore::MainResourceLoader::handleSubstituteDataLoadNow(WebCore::RunLoopTimer<WebCore::MainResourceLoader>*) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:628 I don't know if that's a long-standing behavior, or something that got broken with MainResourceLoader refactoring recently. Would you be willing to take a look? r+ for this patch.
Thank you for taking the effort to make a test - it's great that it helped you discover and fix an additional issue.
(In reply to comment #14) > (From update of attachment 189426 [details]) > > The iframe in the webarchive points to a 0-byte mpeg. I'd guess that's the cause, but I'm not 100% sure. > > This looks more like a bug than like correct behavior. This happens because we cancel main resource load when switching to a media document, and that somehow translates into a frame loader failure. But it's a resource load cancellation, not a frame load failure. > > > // If the document is a stand-alone media document, now is the right time to cancel the WebKit load. > // FIXME: This code should be shared across all ports. <http://webkit.org/b/48762>. > if (m_frame->coreFrame()->document()->isMediaDocument()) > loader->cancelMainResourceLoad(pluginWillHandleLoadError(loader->response())); > > > #0 0x000000010128afcc in WebKit::WebFrameLoaderClient::dispatchDidFailLoading(WebCore::DocumentLoader*, unsigned long, WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:249 > #1 0x00000001046df8af in WebCore::MainResourceLoader::receivedError(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:99 > #2 0x00000001046dfb27 in WebCore::MainResourceLoader::cancel(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:132 > #3 0x000000010399a11f in WebCore::DocumentLoader::cancelMainResourceLoad(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:913 > #4 0x000000010128db82 in WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:852 > #5 0x0000000103997b20 in WebCore::DocumentLoader::commitLoad(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:318 > #6 0x00000001039980cb in WebCore::DocumentLoader::receivedData(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:397 > #7 0x00000001046e1f4a in WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:522 > #8 0x00000001046e0e20 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:365 > #9 0x00000001046e1266 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:381 > #10 0x00000001046e11ab in WebCore::MainResourceLoader::callContinueAfterContentPolicy(void*, WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:373 > #11 0x00000001046e18c6 in WebCore::MainResourceLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:450 > #12 0x00000001046df594 in WebCore::MainResourceLoader::handleSubstituteDataLoadNow(WebCore::RunLoopTimer<WebCore::MainResourceLoader>*) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:628 > > I don't know if that's a long-standing behavior, or something that got broken with MainResourceLoader refactoring recently. Would you be willing to take a look? > > r+ for this patch. Sure, I'll take a look. My best guess is that this is a long standing issue, because cancelMainResourceLoad() should trigger a full cancel with all the relevant callbacks both before and after the recent refactorings.
Comment on attachment 189426 [details] Fix ChangeLog Clearing flags on attachment: 189426 Committed r143630: <http://trac.webkit.org/changeset/143630>
All reviewed patches have been landed. Closing bug.
Created attachment 190074 [details] Shared menu is greyed out. Decided to check on the progress of the repair efforts for the crash problem when trying to share via email. The menu is now greyed out. So took a Command Key 4 screen shot. That deep ended the display... perpetual beach ball. Switch to another tab results in an all white window. Resetting Webkit ... no help Reloading a page ... no help Still have beach ball on the Tarawa page and white pages in all other tabs. Cure.... quit Webkit Using build r143902
> The menu is now greyed out. Can you please reboot and try to reproduce this again? If it's reproducible, please file a new bug.
(In reply to comment #16) > (In reply to comment #14) > > (From update of attachment 189426 [details] [details]) > > > The iframe in the webarchive points to a 0-byte mpeg. I'd guess that's the cause, but I'm not 100% sure. > > > > This looks more like a bug than like correct behavior. This happens because we cancel main resource load when switching to a media document, and that somehow translates into a frame loader failure. But it's a resource load cancellation, not a frame load failure. > > > > > > // If the document is a stand-alone media document, now is the right time to cancel the WebKit load. > > // FIXME: This code should be shared across all ports. <http://webkit.org/b/48762>. > > if (m_frame->coreFrame()->document()->isMediaDocument()) > > loader->cancelMainResourceLoad(pluginWillHandleLoadError(loader->response())); > > > > > > #0 0x000000010128afcc in WebKit::WebFrameLoaderClient::dispatchDidFailLoading(WebCore::DocumentLoader*, unsigned long, WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:249 > > #1 0x00000001046df8af in WebCore::MainResourceLoader::receivedError(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:99 > > #2 0x00000001046dfb27 in WebCore::MainResourceLoader::cancel(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:132 > > #3 0x000000010399a11f in WebCore::DocumentLoader::cancelMainResourceLoad(WebCore::ResourceError const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:913 > > #4 0x000000010128db82 in WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:852 > > #5 0x0000000103997b20 in WebCore::DocumentLoader::commitLoad(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:318 > > #6 0x00000001039980cb in WebCore::DocumentLoader::receivedData(char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/DocumentLoader.cpp:397 > > #7 0x00000001046e1f4a in WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:522 > > #8 0x00000001046e0e20 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:365 > > #9 0x00000001046e1266 in WebCore::MainResourceLoader::continueAfterContentPolicy(WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:381 > > #10 0x00000001046e11ab in WebCore::MainResourceLoader::callContinueAfterContentPolicy(void*, WebCore::PolicyAction) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:373 > > #11 0x00000001046e18c6 in WebCore::MainResourceLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:450 > > #12 0x00000001046df594 in WebCore::MainResourceLoader::handleSubstituteDataLoadNow(WebCore::RunLoopTimer<WebCore::MainResourceLoader>*) at /Users/ap/Safari/OpenSource/Source/WebCore/loader/MainResourceLoader.cpp:628 > > > > I don't know if that's a long-standing behavior, or something that got broken with MainResourceLoader refactoring recently. Would you be willing to take a look? > > > > r+ for this patch. > > Sure, I'll take a look. My best guess is that this is a long standing issue, because cancelMainResourceLoad() should trigger a full cancel with all the relevant callbacks both before and after the recent refactorings. The more I think about and look at this, the less convinced I am that there's a good way to fix this. I don't think that the loader has a concept of a main resource load cancellation that isn't also a frame load failure. And if I understand how media document load works, from WebCore's perspective, this is really a failure: the frame load is getting ignored and processed externally. Is that reasonable?
I'm fine with not pouring more effort into this now. Thanks for looking into it!