58462 2011-04-13 11:33:50 -0700 Media engine fallback doesn't always work 2011-04-13 14:20:05 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) All All RESOLVED FIXED InRadar, PlatformOnly P2 Normal --- 1 eric.carlson eric.carlson ap webkit.review.bot oldest_to_newest 385044 0 eric.carlson 2011-04-13 11:33:50 -0700 When a media engine fails to open a url before readyState reaches HAVE_METADATA, MediaPlayer::networkStateChanged is supposed to try to open it with the next media engine. This doesn't work when the file doesn't have a MIME type and doesn't have an extension, eg. opening https://bug-52552-attachments.webkit.org/attachment.cgi?id=79249 in a media document. 385110 1 89424 eric.carlson 2011-04-13 12:00:00 -0700 Created attachment 89424 Proposed patch. 385111 2 eric.carlson 2011-04-13 12:00:44 -0700 <rdar://problem/9278261> 385112 3 webkit.review.bot 2011-04-13 12:02:02 -0700 Attachment 89424 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/MediaPlayer.cpp:791: Extra space after ( in if [whitespace/parens] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 385114 4 eric.carlson 2011-04-13 12:03:53 -0700 (In reply to comment #3) > Attachment 89424 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 > > Source/WebCore/platform/graphics/MediaPlayer.cpp:791: Extra space after ( in if [whitespace/parens] [5] Oh dear, I have been shamed by the pedantic-bot! 385148 5 89424 ap 2011-04-13 12:33:52 -0700 Comment on attachment 89424 Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=89424&action=review As discussed in person, it is wrong that we don't have a MIME type here - WebKit knew it when it decided to create a media document. Passing the MIME type down would be another way to fix this particular bug. There are other ways to reach this code without a known MIME type, so the fix is still useful. > Source/WebCore/ChangeLog:8 > + No new tests, this was verified manually with the url noted in the bug. This doesn't really explain why there is no regression test. 385221 6 eric.carlson 2011-04-13 14:20:05 -0700 http://trac.webkit.org/changeset/83763 89424 2011-04-13 12:00:00 -0700 2011-04-13 12:33:52 -0700 Proposed patch. patch_1.txt text/plain 1822 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzNzUwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDQtMTMgIEVyaWMgQ2Fy bHNvbiAgPGVyaWMuY2FybHNvbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgTWVkaWEgZW5naW5lIGZhbGxiYWNrIGRvZXNuJ3QgYWx3 YXlzIHdvcmsKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTU4NDYyCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLCB0aGlzIHdhcyB2ZXJpZmllZCBtYW51YWxs eSB3aXRoIHRoZSB1cmwgbm90ZWQgaW4gdGhlIGJ1Zy4KKworICAgICAgICAqIHBsYXRmb3JtL2dy YXBoaWNzL01lZGlhUGxheWVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhUGxheWVyOjpu ZXR3b3JrU3RhdGVDaGFuZ2VkKToKKwogMjAxMS0wNC0xMiAgUGF2ZWwgUG9kaXZpbG92ICA8cG9k aXZpbG92QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBQYXZlbCBGZWxkbWFu LgpJbmRleDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvTWVkaWFQbGF5ZXIuY3Bw Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL01lZGlhUGxh eWVyLmNwcAkocmV2aXNpb24gODM2NjcpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9NZWRpYVBsYXllci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTc4NiwxMSArNzg2LDEyIEBA IHZvaWQgTWVkaWFQbGF5ZXI6Om5ldHdvcmtTdGF0ZUNoYW5nZWQoKQogICAgIC8vIElmIG1vcmUg dGhhbiBvbmUgbWVkaWEgZW5naW5lIGlzIGluc3RhbGxlZCBhbmQgdGhpcyBvbmUgZmFpbGVkIGJl Zm9yZSBmaW5kaW5nIG1ldGFkYXRhLAogICAgIC8vIGxldCB0aGUgbmV4dCBlbmdpbmUgdHJ5Lgog ICAgIGlmIChtX3ByaXZhdGUtPm5ldHdvcmtTdGF0ZSgpID49IEZvcm1hdEVycm9yCi0gICAgICAg ICYmIG1fcHJpdmF0ZS0+cmVhZHlTdGF0ZSgpIDwgSGF2ZU1ldGFkYXRhIAotICAgICAgICAmJiBp bnN0YWxsZWRNZWRpYUVuZ2luZXMoKS5zaXplKCkgPiAxIAotICAgICAgICAmJiBiZXN0TWVkaWFF bmdpbmVGb3JUeXBlQW5kQ29kZWNzKG1fY29udGVudE1JTUVUeXBlLCBtX2NvbnRlbnRUeXBlQ29k ZWNzLCBtX2N1cnJlbnRNZWRpYUVuZ2luZSkpIHsKKyAgICAgICAgJiYgbV9wcml2YXRlLT5yZWFk eVN0YXRlKCkgPCBIYXZlTWV0YWRhdGEKKyAgICAgICAgJiYgaW5zdGFsbGVkTWVkaWFFbmdpbmVz KCkuc2l6ZSgpID4gMSkgeworICAgICAgICBpZiAoIG1fY29udGVudE1JTUVUeXBlLmlzRW1wdHko KSB8fCBiZXN0TWVkaWFFbmdpbmVGb3JUeXBlQW5kQ29kZWNzKG1fY29udGVudE1JTUVUeXBlLCBt X2NvbnRlbnRUeXBlQ29kZWNzLCBtX2N1cnJlbnRNZWRpYUVuZ2luZSkpIHsKICAgICAgICAgICAg IG1fcmVsb2FkVGltZXIuc3RhcnRPbmVTaG90KDApOwogICAgICAgICAgICAgcmV0dXJuOworICAg ICAgICB9CiAgICAgfQogICAgIGlmIChtX21lZGlhUGxheWVyQ2xpZW50KQogICAgICAgICBtX21l ZGlhUGxheWVyQ2xpZW50LT5tZWRpYVBsYXllck5ldHdvcmtTdGF0ZUNoYW5nZWQodGhpcyk7Cg==