Bug 111293 – MediaStream.ended must return true when it is created with ended tracks.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Li Yin]

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.

[Li Yin]

Created attachment 191175 [details]
Patch

[Tommy Widenflycht]

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.

[Kentaro Hara]

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.

[Li Yin]

(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.

[Li Yin]

Created attachment 191202 [details]
Patch

[Kentaro Hara]

Comment on attachment 191202 [details]
Patch

OK, thank you and tommy!

[WebKit Review Bot]

Comment on attachment 191202 [details]
Patch

Clearing flags on attachment: 191202

Committed r144623: <http://trac.webkit.org/changeset/144623>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.