Bug 113133 – MediaStream should fire ended event when all tracks are ended

• Status: NEW
• Component: WebCore Misc.

[Li Yin]

Spec: http://dev.w3.org/2011/webrtc/editor/getusermedia.html#event-mediastream-ended
When all the tracks are ended, MediaStream should fire ended event.

[Li Yin]

Created attachment 194701 [details]
Patch

[Michael Catanzaro]

Comment on attachment 194701 [details]
Patch

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

Sorry you had no review for three years....

I'm not familiar with the media stream code, but I have a couple trivial code style comments. I presume the patch will need rebased after so long anyway, so I'll give this r- and expect a media stream developer will review this again if you update the patch.

> Source/WebCore/Modules/mediastream/MediaStream.cpp:335
> +    for (size_t i = 0; i < m_audioTracks.size(); ++i)

Use modern for loops: for (auto& audioTrack : m_audioTracks).

> Source/WebCore/Modules/mediastream/MediaStreamTrack.h:47
> +    static PassRefPtr<MediaStreamTrack> create(ScriptExecutionContext*, MediaStreamComponent*, MediaStream*);

Since this function is only ever called with 'this', the pointer is guaranteed to be non-null; therefore, it should be a reference rather than a pointer.  Maybe the other arguments don't follow this rule, but we should for new code.

> Source/WebCore/Modules/mediastream/MediaStreamTrack.h:91
> +    MediaStream* m_stream;

Same here: since you never null out m_stream and set it in the initializer list of the constructor, it should be a reference.

Either way, this is only right if the MediaStreamTrack can never outlive the MediaStream, but I presume that is true.