46777 2010-09-28 19:28:02 -0700 CRASH at WebCore::HTMLSourceElement::scheduleErrorEvent 2011-01-24 13:07:56 -0800 1 1 1 Unclassified WebKit Media 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 51249 1 joepeck webkit-unassigned abarth ademar ap darin ddkilzer eric.carlson eric joepeck simon.fraser webkit.review.bot oldest_to_newest 286707 0 joepeck 2010-09-28 19:28:02 -0700 Non-reproducible crash seen at: Thread 2 Crashed: 0 WebCore 0x31c958ec WebCore::HTMLSourceElement::scheduleErrorEvent() (HTMLSourceElement.cpp:94) 1 WebCore 0x31a710ac WebCore::HTMLMediaElement::setNetworkState(WebCore::MediaPlayer::NetworkState) (HTMLMediaElement.cpp:852) 2 WebCore 0x31c8bfac non-virtual thunk to WebCore::HTMLMediaElement::mediaPlayerNetworkStateChanged(WebCore::MediaPlayer*) + 36 3 WebCore 0x31a71010 WebCore::MediaPlayer::networkStateChanged() (MediaPlayer.cpp:607) 4 WebCore 0x31a70fac WebCore::MediaPlayerPrivateX::deliverNotification(MediaPlayerProxyNotificationType) (MediaPlayerPrivateX.mm:713) 5 WebCore 0x31a70cf4 WebCore::MediaPlayer::deliverNotification(MediaPlayerProxyNotificationType) (MediaPlayer.cpp:556) 6 WebCore 0x31a70cbc WebCore::HTMLMediaElement::deliverNotification(MediaPlayerProxyNotificationType) (HTMLMediaElement.cpp:2097) 7 WebKit 0x305849ac -[WebPluginController _webPluginContainerPostMediaPlayerNotification:forElement:] (WebPluginController.mm:651) ... 286709 1 joepeck 2010-09-28 19:35:29 -0700 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. 286711 2 69155 joepeck 2010-09-28 19:45:33 -0700 Created attachment 69155 [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. 287108 3 joepeck 2010-09-29 14:00:03 -0700 Thanks! Committed r68682 M WebCore/ChangeLog M WebCore/html/HTMLMediaElement.cpp r68682 = 717390534b65d85cd07cc3df4992c84918a38e9f http://trac.webkit.org/changeset/68682 287750 4 ap 2010-09-30 12:52:57 -0700 Hmm, this is the stack trace I saw on test case from bug 46763 in debug mode. > Thread 2 Crashed: ?! 287796 5 joepeck 2010-09-30 13:56:28 -0700 (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? 287806 6 joepeck 2010-09-30 14:08:16 -0700 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. 287963 7 ap 2010-09-30 17:41:05 -0700 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. 292563 8 70455 joepeck 2010-10-11 11:10:01 -0700 Created attachment 70455 [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. 292565 9 70455 darin 2010-10-11 11:12:10 -0700 Comment on attachment 70455 [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? 292569 10 joepeck 2010-10-11 11:17:35 -0700 (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. 292599 11 joepeck 2010-10-11 12:03:43 -0700 Committed r69514 M WebCore/ChangeLog M WebCore/html/HTMLMediaElement.cpp r69514 = 41102af89bdf6311aed9cf2d153ab8bc422caea5 292601 12 joepeck 2010-10-11 12:07:28 -0700 http://trac.webkit.org/changeset/69514 292654 13 webkit.review.bot 2010-10-11 14:29:21 -0700 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 339008 14 ademar 2011-01-24 07:45:26 -0800 Revision r68682 cherry-picked into qtwebkit-2.2 with commit 9211999 <http://gitorious.org/webkit/qtwebkit/commit/9211999> 339175 15 ademar 2011-01-24 12:16:57 -0800 Revision r68682 cherry-picked into qtwebkit-2.2 with commit 9211999 <http://gitorious.org/webkit/qtwebkit/commit/9211999> 339210 16 ademar 2011-01-24 13:07:56 -0800 Revision r69514 cherry-picked into qtwebkit-2.2 with commit 3a5ef1b <http://gitorious.org/webkit/qtwebkit/commit/3a5ef1b> 69155 2010-09-28 19:45:33 -0700 2010-09-29 13:36:44 -0700 [PATCH] Clear Load State in userCancelledLoad video.patch text/plain 1477 joepeck ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg N2EzMDQ5Yy4uYmViYmFhZCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wOS0yOCAgSm9zZXBoIFBlY29y YXJvICA8am9lcGVja0B3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIENSQVNIIGF0IFdlYkNvcmU6OkhUTUxTb3VyY2VFbGVtZW50Ojpz Y2hlZHVsZUVycm9yRXZlbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTQ2Nzc3CisKKyAgICAgICAgSXQgc2hvdWxkIG5vdCBiZSBwb3NzaWJsZSB0byBn ZXQgdG8gYmUgaW4gdGhlIExvYWRpbmdGcm9tU291cmNlRWxlbWVudAorICAgICAgICBzdGF0ZSB3 aXRoIGEgbnVsbCBjdXJyZW50IHNvdXJjZSBub2RlLiBUaGlzIHdhcyBhIHBvc3NpYmxlIHNjZW5h cmlvCisgICAgICAgIHdoZXJlIHRoZSBjdXJyZW50IHNvdXJjZSBub2RlIHdhcyBjbGVhcmVkIGJ1 dCB0aGUgbG9hZCBzdGF0ZSB3YXMgbm90CisgICAgICAgIGNsZWFyZWQgYXMgd2VsbC4gVHJpZWQg YW5kIGNvdWxkIG5vdCBjcmVhdGUgYSB0ZXN0IGNhc2UgZm9yIHRoaXMuCisKKyAgICAgICAgKiBo dG1sL0hUTUxNZWRpYUVsZW1lbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SFRNTE1lZGlhRWxl bWVudDo6dXNlckNhbmNlbGxlZExvYWQpOiBjbGVhciB0aGUgbG9hZCBzdGF0ZSBiZWNhdXNlIHdl IGFsc28gY2xlYXIgdGhlIGN1cnJlbnQgc291cmNlIG5vZGUuCisKIDIwMTAtMDktMjggIFhpYW9t ZWkgSmkgIDx4amlAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo cmV2ZXJ0IHVuc3F1YXNoZWQgZ2l0IGNvbW1pdHMpCmRpZmYgLS1naXQgYS9XZWJDb3JlL2h0bWwv SFRNTE1lZGlhRWxlbWVudC5jcHAgYi9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAK aW5kZXggYmM4Njc3OC4uMDBmNDQ4YyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL0hUTUxNZWRp YUVsZW1lbnQuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcApAQCAt MTk5Nyw2ICsxOTk3LDcgQEAgdm9pZCBIVE1MTWVkaWFFbGVtZW50Ojp1c2VyQ2FuY2VsbGVkTG9h ZCgpCiAgICAgbV9wbGF5ZXIuY2xlYXIoKTsKICNlbmRpZgogICAgIHN0b3BQZXJpb2RpY1RpbWVy cygpOworICAgIG1fbG9hZFN0YXRlID0gV2FpdGluZ0ZvclNvdXJjZTsKIAogICAgIC8vIDIgLSBT ZXQgdGhlIGVycm9yIGF0dHJpYnV0ZSB0byBhIG5ldyBNZWRpYUVycm9yIG9iamVjdCB3aG9zZSBj b2RlIGF0dHJpYnV0ZSBpcyBzZXQgdG8gTUVESUFfRVJSX0FCT1JURUQuCiAgICAgbV9lcnJvciA9 IE1lZGlhRXJyb3I6OmNyZWF0ZShNZWRpYUVycm9yOjpNRURJQV9FUlJfQUJPUlRFRCk7Cg== 70455 2010-10-11 11:10:01 -0700 2010-10-11 11:41:45 -0700 [PATCH] Another State and ASSERT / Early Return to stop Crashes loading-state.patch text/plain 1975 joepeck ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NTliOGQ0MC4uNmM2ZDc4NiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMC0xMC0xMSAgSm9zZXBoIFBlY29y YXJvICA8am9lcGVja0B3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIENSQVNIIGF0IFdlYkNvcmU6OkhUTUxTb3VyY2VFbGVtZW50Ojpz Y2hlZHVsZUVycm9yRXZlbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTQ2Nzc3CisKKyAgICAgICAgQW5vdGhlciBjYXNlIHRoYXQgY291bGQgbGVhZCB0 byBhbiBpbXByb3BlciBzZXQgb2Ygc3RhdGVzIGJldHdlZW4KKyAgICAgICAgbV9jdXJyZW50Tm9k ZSBhbmQgbV9sb2FkaW5nU3RhdGUuIEFsc28gYWRkZWQgYW4gQVNTRVJUIGFuZCBlYXJseQorICAg ICAgICByZXR1cm4gdG8gcHJldmVudCBjcmFzaGVzIGlmIHRoaXMgY2FzZSBjYW4gc3RpbGwgaGFw cGVuIGluIG90aGVyIHdheXMuCisKKyAgICAgICAgKiBodG1sL0hUTUxNZWRpYUVsZW1lbnQuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6SFRNTE1lZGlhRWxlbWVudDo6cHJlcGFyZUZvckxvYWQpOgor CiAyMDEwLTEwLTA5ICBKb3NlcGggUGVjb3Jhcm8gIDxqb2VwZWNrQHdlYmtpdC5vcmc+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCmRpZmYgLS1naXQgYS9XZWJDb3JlL2h0 bWwvSFRNTE1lZGlhRWxlbWVudC5jcHAgYi9XZWJDb3JlL2h0bWwvSFRNTE1lZGlhRWxlbWVudC5j cHAKaW5kZXggM2Q5MTQ5ZS4uN2MyMTM4ZiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL0hUTUxN ZWRpYUVsZW1lbnQuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcApA QCAtNTMwLDYgKzUzMCw3IEBAIHZvaWQgSFRNTE1lZGlhRWxlbWVudDo6cHJlcGFyZUZvckxvYWQo KQogICAgIG1fZGlzcGxheU1vZGUgPSBVbmtub3duOwogCiAgICAgLy8gMSAtIEFib3J0IGFueSBh bHJlYWR5LXJ1bm5pbmcgaW5zdGFuY2Ugb2YgdGhlIHJlc291cmNlIHNlbGVjdGlvbiBhbGdvcml0 aG0gZm9yIHRoaXMgZWxlbWVudC4KKyAgICBtX2xvYWRTdGF0ZSA9IFdhaXRpbmdGb3JTb3VyY2U7 CiAgICAgbV9jdXJyZW50U291cmNlTm9kZSA9IDA7CiAKICAgICAvLyAyIC0gSWYgdGhlcmUgYXJl IGFueSB0YXNrcyBmcm9tIHRoZSBtZWRpYSBlbGVtZW50J3MgbWVkaWEgZWxlbWVudCBldmVudCB0 YXNrIHNvdXJjZSBpbiAKQEAgLTg3Myw2ICs4NzQsMTAgQEAgdm9pZCBIVE1MTWVkaWFFbGVtZW50 OjpzZXROZXR3b3JrU3RhdGUoTWVkaWFQbGF5ZXI6Ok5ldHdvcmtTdGF0ZSBzdGF0ZSkKICAgICAg ICAgLy8gSWYgd2UgZmFpbGVkIHdoaWxlIHRyeWluZyB0byBsb2FkIGEgPHNvdXJjZT4gZWxlbWVu dCwgdGhlIG1vdmllIHdhcyBuZXZlciBwYXJzZWQsIGFuZCB0aGVyZSBhcmUgbW9yZQogICAgICAg ICAvLyA8c291cmNlPiBjaGlsZHJlbiwgc2NoZWR1bGUgdGhlIG5leHQgb25lCiAgICAgICAgIGlm IChtX3JlYWR5U3RhdGUgPCBIQVZFX01FVEFEQVRBICYmIG1fbG9hZFN0YXRlID09IExvYWRpbmdG cm9tU291cmNlRWxlbWVudCkgeworICAgICAgICAgICAgQVNTRVJUKG1fY3VycmVudFNvdXJjZU5v ZGUpOworICAgICAgICAgICAgaWYgKCFtX2N1cnJlbnRTb3VyY2VOb2RlKQorICAgICAgICAgICAg ICAgIHJldHVybjsKKwogICAgICAgICAgICAgbV9jdXJyZW50U291cmNlTm9kZS0+c2NoZWR1bGVF cnJvckV2ZW50KCk7CiAgICAgICAgICAgICBpZiAoaGF2ZVBvdGVudGlhbFNvdXJjZUNoaWxkKCkp IHsKICAgICAgICAgICAgICAgICBMT0coTWVkaWEsICJIVE1MTWVkaWFFbGVtZW50OjpzZXROZXR3 b3JrU3RhdGUgc2NoZWR1bGluZyBuZXh0IDxzb3VyY2U+Iik7Cg==