Bug 133510 - [MediaStream] MediaStream.addTrack should not check for tracks ended state.
Summary: [MediaStream] MediaStream.addTrack should not check for tracks ended state.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-04 06:01 PDT by Kiran
Modified: 2014-06-09 02:46 PDT (History)
9 users (show)

See Also:


Attachments
MediaStream.addTrack should not check for tracks ended state. (1.54 KB, patch)
2014-06-04 06:11 PDT, Kiran
no flags Details | Formatted Diff | Diff
MediaStream.addTrack should not check for tracks ended state. (5.77 KB, patch)
2014-06-04 07:06 PDT, Kiran
eric.carlson: review+
Details | Formatted Diff | Diff
MediaStream.addTrack should not check for tracks ended state. (5.84 KB, patch)
2014-06-04 20:38 PDT, Kiran
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kiran 2014-06-04 06:01:49 PDT
As per the updated spec, MediaStream.addTrack() should not check for tracks ended state.

https://www.w3.org/Bugs/Public/show_bug.cgi?id=25250
Comment 1 Kiran 2014-06-04 06:11:01 PDT
Created attachment 232475 [details]
MediaStream.addTrack should not check for tracks ended state.

MediaStream.addTrack should not check for tracks ended state.
Comment 2 Kiran 2014-06-04 07:06:24 PDT
Created attachment 232476 [details]
MediaStream.addTrack should not check for tracks ended state.

Added test files.
Comment 3 Eric Carlson 2014-06-04 09:19:04 PDT
Comment on attachment 232476 [details]
MediaStream.addTrack should not check for tracks ended state.

View in context: https://bugs.webkit.org/attachment.cgi?id=232476&action=review

If you address the comments, fill in the "Reviewed by" in each ChangeLog, and upload the new patch WITHOUT orphaning the old version, you don't need another review and any reviewer will mark it cq+.

> Source/WebCore/ChangeLog:14
> +        (WebCore::MediaStreamPrivate::addTrack):

Nit: I think it is helpful to have a comment about what changed.

> LayoutTests/fast/mediastream/MediaStream-add-ended-tracks.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">

Nit: you should use an HTML5 doctype here: "<!DOCTYPE html>"

> LayoutTests/fast/mediastream/MediaStream-add-ended-tracks.html:7
> +
> +

Nit: you have an extra blank line.

> LayoutTests/fast/mediastream/MediaStream-add-ended-tracks.html:10
> +

Ditto.

> LayoutTests/fast/mediastream/MediaStream-add-ended-tracks.html:41
> +

Ditto.

> LayoutTests/fast/mediastream/MediaStream-add-ended-tracks.html:50
> +

Ditto.
Comment 4 Kiran 2014-06-04 20:38:03 PDT
Created attachment 232519 [details]
MediaStream.addTrack should not check for tracks ended state.

Added comment line and removed blank spaces as per the review comments.
Comment 5 WebKit Commit Bot 2014-06-05 02:43:15 PDT
Comment on attachment 232519 [details]
MediaStream.addTrack should not check for tracks ended state.

Clearing flags on attachment: 232519

Committed r169611: <http://trac.webkit.org/changeset/169611>
Comment 6 Kiran 2014-06-09 02:45:38 PDT
Patches landed, so closing the bug.