[webkit-reviews] review denied: [Bug 54888] Add regression test for clean shutdown during video playback. : [Attachment 83175] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has denied Ami Fischman
<fischman at chromium.org>'s request for review:
Bug 54888: Add regression test for clean shutdown during video playback.
https://bugs.webkit.org/show_bug.cgi?id=54888

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

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

Marking r- for the minor changes noted.

> LayoutTests/ChangeLog:6
> +	   Add regression test for clean shutdown during video playback.
> +	   This didn't always use to be the case (http://crbug.com/72730).

It would be good to have more information about why this test is necessary, I
didn't understand what "clean shutdown during video playback" meant until I
read the chromium bug.

> LayoutTests/media/video-plays-past-end-of-test.html:3
> +<!-- This test intentionally lets the video keep playing past endTest() to
> +	ensure that shutdown is clean. -->
> +

I really like to have information about what a test does in the markup so
someone opening the test manually can figure out what is happening without
having to view source. As above, "clean shutdown during video playback" didn't
explain it for me.