[Webkit-unassigned] [Bug 80978] [BlackBerry] MMRPlayer will hang webkit thread when retrieving media metadata

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 10 09:31:50 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=80978


Eric Carlson <eric.carlson at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #136391|review+, commit-queue?      |review-
               Flag|                            |




--- Comment #7 from Eric Carlson <eric.carlson at apple.com>  2012-04-10 09:31:50 PST ---
(From update of attachment 136391)
View in context: https://bugs.webkit.org/attachment.cgi?id=136391&action=review

Marking r- for the incorrect 'type' param in the cgi url.

> LayoutTests/http/tests/media/video-throttle-load-metadata-worker.js:2
> +postMessage("Message from worker thread");
>  \ No newline at end of file

Nit: please add a blank line at the end of the file.

> LayoutTests/http/tests/media/video-throttle-load-metadata.html:5
> +    <!--
> +    This test case is for https://bugs.webkit.org/show_bug.cgi?id=80978
> +    --!>

Nit: Personally, I think comments like this are very helpful to have in the markup of the test so someone looking at the test results, or running the test itself, can see it without having to look through the source.

> LayoutTests/http/tests/media/video-throttle-load-metadata.html:13
> +        function loadedmetadata(e) {
> +            logResult(true, "loaded metadata of media file");
> +            endTest();
> +        }

WebKit style is to have a function's opening brace on a new line.

> LayoutTests/http/tests/media/video-throttle-load-metadata.html:15
> +        function error(e) {

Ditto.

> LayoutTests/http/tests/media/video-throttle-load-metadata.html:20
> +        function start() {

Ditto.

> LayoutTests/http/tests/media/video-throttle-load-metadata.html:32
> +            video.src = "http://127.0.0.1:8000/media/video-throttled-load.cgi?name=" + movie + "&throttle=400&type=video/mp4";

"type=video/mp4" isn't necessarily correct. findMediaFile() will return a file that the current port can load, which may not be MPEG-4. You should use something like "mimeTypeForExtension(movie.split('.').pop())" instead.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list