Summary: | CRASH at WebCore::HTMLSourceElement::scheduleErrorEvent | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, ademar, ap, darin, ddkilzer, eric.carlson, eric, joepeck, simon.fraser, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 51249 | ||||||||
Attachments: |
|
Description
Joseph Pecoraro
2010-09-28 19:28:02 PDT
I believe this could happen if: - The markup has a <video> with <source src="unsupported-video.video"> - ENABLE(PLUGIN_PROXY_FOR_VIDEO) is enabled Sequence of events: - HTMLMediaElement::loadNextSourceChild triggers a load - m_loadState = LoadingFromSourceElement - Somehow documentWillBecomeInactive or userCancelledLoad is triggered - m_currentSourceNode = 0 - m_readyState = HAVE_NOTHING - A web plugin post media player notification comes in and triggers setNetworkState calling: if (m_readyState < HAVE_METADATA && m_loadState == LoadingFromSourceElement) { m_currentSourceNode->scheduleErrorEvent(); // <-- with a null pointer I haven't been able to create a test case for this. However, it looks like m_loadState shouldn't have LoadingFromSourceElement if the current source node is null. In numerous other places when the currentSourceNode is set to 0 the load state is also changed to WaitingForSource. I think the case in userCancelledLoad is missing this change. Created attachment 69155 [details] [PATCH] Clear Load State in userCancelledLoad See comment #1 above for details on how this might have happened and why I think this fix is correct. Thanks! Committed r68682 M WebCore/ChangeLog M WebCore/html/HTMLMediaElement.cpp r68682 = 717390534b65d85cd07cc3df4992c84918a38e9f http://trac.webkit.org/changeset/68682 Hmm, this is the stack trace I saw on test case from bug 46763 in debug mode. > Thread 2 Crashed: ?! (In reply to comment #4) > Hmm, this is the stack trace I saw on test case from bug 46763 in debug mode. Really? Thats good to know, because I created that test case on that bug specifically to try and reproduce _this_ crash. However I never reproduced it, just always hit that crash. So if that is the case, than although I don't have a reduction, it reaffirms that this was the correct fix. If you test with a debug nightly past r68682 can you reproduce the crash? Its possible the same fix would need to be applied to: HTMLMediaElement::prepareForLoad Setting the m_loadState = WaitingForSource whenever we set m_currentSourceNode to 0. I'm still seeing this stack trace on the test from bug 46763. These two bugs are in confusing state - something was fixed without a test case, but the one with test case still shows a similar problem to the one that should have been fixed. Created attachment 70455 [details]
[PATCH] Another State and ASSERT / Early Return to stop Crashes
This is another patch, to catch a similar unbalanced state. It also adds an ASSERT and early
return to prevent crashes.
Comment on attachment 70455 [details]
[PATCH] Another State and ASSERT / Early Return to stop Crashes
The bug fix looks fine, but I don’t see a test case. Can we create a test case for this?
(In reply to comment #9) > (From update of attachment 70455 [details]) > The bug fix looks fine, but I don’t see a test case. Can we create a test case for this? I haven't been able to reproduce the crash myself. Alexey was able to reproduce it (with a test case that I created to try to reproduce this) using MallocScribble and a debug build. I tried to use the same settings and still was not able to reproduce it. It may be based on timers. Committed r69514 M WebCore/ChangeLog M WebCore/html/HTMLMediaElement.cpp r69514 = 41102af89bdf6311aed9cf2d153ab8bc422caea5 http://trac.webkit.org/changeset/69514 might have broken GTK Linux 32-bit Release The following tests are not passing: fast/js/basic-strict-mode.html Revision r68682 cherry-picked into qtwebkit-2.2 with commit 9211999 <http://gitorious.org/webkit/qtwebkit/commit/9211999> Revision r68682 cherry-picked into qtwebkit-2.2 with commit 9211999 <http://gitorious.org/webkit/qtwebkit/commit/9211999> Revision r69514 cherry-picked into qtwebkit-2.2 with commit 3a5ef1b <http://gitorious.org/webkit/qtwebkit/commit/3a5ef1b> |