[webkit-reviews] review granted: [Bug 93052] Fire suspend event when loaded + add layout test : [Attachment 157040] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Andrew Scherkus
<scherkus at chromium.org>'s request for review:
Bug 93052: Fire suspend event when loaded + add layout test
https://bugs.webkit.org/show_bug.cgi?id=93052

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

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


> Source/WebCore/ChangeLog:12
> +	   Fire suspend event whenever network state is set to NETWORK_IDLE.
> +	   https://bugs.webkit.org/show_bug.cgi?id=93052
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   There was a regression in the Chromium port
(http://crbug.com/139511) that revealed we didn't
> +	   have a layout test for suspend events. Upon further investigation it
appeared we also had a bug
> +	   where we didn't fire the suspend event when a media engine reported
they had completely loaded
> +	   the media.
> +

You should mention the new test.

> LayoutTests/ChangeLog:15
> +	   * http/tests/media/video-load-suspend-expected.txt: Added.
> +	   * http/tests/media/video-load-suspend.html: Added.
> +

You forgot to log the changes to media/event-attributes-expected.txt.

> LayoutTests/http/tests/media/video-load-suspend.html:10
> +    function init() {
> +	   findMediaElement();
> +	   run("video.src = file");

Nit: A function's opening brace should be on a new line.