Bug 111293 - MediaStream.ended must return true when it is created with ended tracks.
Summary: MediaStream.ended must return true when it is created with ended tracks.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Li Yin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-04 01:11 PST by Li Yin
Modified: 2013-03-04 05:56 PST (History)
7 users (show)

See Also:


Attachments
Patch (8.23 KB, patch)
2013-03-04 01:37 PST, Li Yin
no flags Details | Formatted Diff | Diff
Patch (9.12 KB, patch)
2013-03-04 04:21 PST, Li Yin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Li Yin 2013-03-04 01:11:58 PST
Spec: http://dev.w3.org/2011/webrtc/editor/getusermedia.html#MediaStream-ended
When a MediaStream object is created, its ended attribute must be set to false, unless it is being created using the MediaStream() constructor whose arguments are lists of MediaStreamTrack objects that are all ended, in which case the MediaStream object must be created with its ended attribute set to true.
Comment 1 Li Yin 2013-03-04 01:37:43 PST
Created attachment 191175 [details]
Patch
Comment 2 Tommy Widenflycht 2013-03-04 01:59:43 PST
Comment on attachment 191175 [details]
Patch

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

Unofficial R+ from me.

> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114
> +            m_ended = true;

To the actual reviewer:
The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored.

> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88
> +    var ended = arguments[3]?arguments[3]:false;

Nit: I would prefer ended to be added as a proper argument.  Also spaces around the operators is the norm.
Comment 3 Kentaro Hara 2013-03-04 02:29:36 PST
Comment on attachment 191175 [details]
Patch

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

>> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114
>> +            m_ended = true;
> 
> To the actual reviewer:
> The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored.

Can we add some ASSERT() about this?

>> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88
>> +    var ended = arguments[3]?arguments[3]:false;
> 
> Nit: I would prefer ended to be added as a proper argument.  Also spaces around the operators is the norm.

Agreed.
Comment 4 Li Yin 2013-03-04 04:13:15 PST
(In reply to comment #3)
> (From update of attachment 191175 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191175&action=review
> 
> >> Source/WebCore/platform/mediastream/MediaStreamDescriptor.h:114
> >> +            m_ended = true;
> > 
> > To the actual reviewer:
> > The sources are filtered in Source/WebCore/Modules/mediastream/MediaStream.cpp in the function processTrack so when the execution comes to this stage all ended tracks have been ignored.
> 
> Can we add some ASSERT() about this?

The ended status is not fatal to MediaStreamDescriptor object, so maybe it is not necessary to do that.

> 
> >> LayoutTests/fast/mediastream/MediaStreamConstructor.html:88
> >> +    var ended = arguments[3]?arguments[3]:false;
> > 
> > Nit: I would prefer ended to be added as a proper argument.  Also spaces around the operators is the norm.
> 
> Agreed.

I just wanted to express the default value of ended attribute. Anyway, I will fix it.
Comment 5 Li Yin 2013-03-04 04:21:16 PST
Created attachment 191202 [details]
Patch
Comment 6 Kentaro Hara 2013-03-04 04:37:17 PST
Comment on attachment 191202 [details]
Patch

OK, thank you and tommy!
Comment 7 WebKit Review Bot 2013-03-04 05:56:56 PST
Comment on attachment 191202 [details]
Patch

Clearing flags on attachment: 191202

Committed r144623: <http://trac.webkit.org/changeset/144623>
Comment 8 WebKit Review Bot 2013-03-04 05:56:59 PST
All reviewed patches have been landed.  Closing bug.