[Webkit-unassigned] [Bug 132558] [MediaStream] MediaStream.addTrack Should not check for active state.

Thu May 8 07:19:31 PDT 2014

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

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

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

--- Comment #9 from Eric Carlson <eric.carlson at apple.com>  2014-05-08 07:19:50 PST ---
(From update of attachment 231057)
View in context: https://bugs.webkit.org/attachment.cgi?id=231057&action=review

r- for now because the test needs a little bit of work, but this is close. Thanks for responding to previous feedback.

> LayoutTests/fast/mediastream/MediaStream-add-tracks-to-inactive-stream.html:9
> +    <body>
> +        <p id="description"></p>
> +        <div id="console"></div>
> +        <script>

I know that you based this on an existing test, but I would really prefer to have this <script> element inside of <head> and call it from body.load or some such.

> LayoutTests/fast/mediastream/MediaStream-add-tracks-to-inactive-stream.html:17
> +            function error() {

Nit: WebKit coding style is to put a function's opening brace on a new line.

> LayoutTests/fast/mediastream/MediaStream-add-tracks-to-inactive-stream.html:40
> +            function shouldNotFire() {

This is not used. You need to add "onaddtrack" and "onremovetrack" handlers to stream2.

> LayoutTests/fast/mediastream/MediaStream-add-tracks-to-inactive-stream.html:51
> +            function shouldFireActive() {
> +                debug("Stream2 is active.");
> +                finishJSTest();
> +            }
> +
> +            function shouldFireInActive() {
> +                debug("Stream2 is inactive.");
> +                finishJSTest();

Neither of these is called, should they be? If not, each should call testFailed(...) instead of debug() to make it clear that there is an error.

> LayoutTests/fast/mediastream/MediaStream-add-tracks-to-inactive-stream.html:54
> +            function createStreamAndAddTRacks() {

Nit: "TRacks" should be "Tracks"

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