[webkit-reviews] review granted: [Bug 61573] Update LayoutTests/media/broken-video to test for correct error codes : [Attachment 95064] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Scott Franklin
<scottfr at google.com>'s request for review:
Bug 61573: Update LayoutTests/media/broken-video to test for correct error
codes
https://bugs.webkit.org/show_bug.cgi?id=61573

Attachment 95064: Patch
https://bugs.webkit.org/attachment.cgi?id=95064&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=95064&action=review

r=me with the changes suggested.

> LayoutTests/media/broken-video.html:2
> +<p>Test that QuickTime file with broken headers and content generates a
SRC_NOT_SUPPORTED error.<p>

This comment is incorrect as the test will not open a QuickTime movie, the two
files that can be opened are garbage.mp4 and garbage.ogv. You also added a
check for networkState so you may as well mention that as well. Something like
"Tests that an invalid video file generates a MEDIA_ERR_SRC_NOT_SUPPORTED error
and sets networkState to NETWORK_NO_SOURCE" perhaps?

> LayoutTests/media/broken-video.html:11
> +	   testExpected("video.error.code", "4");
> +	   testExpected("video.networkState", "3");

I would prefer to have the named constants used here:

    testExpected("video.error.code", MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
    testExpected("video.networkState", HTMLMediaElement.NETWORK_NO_SOURCE);