MediaRecorder: make EventListener and stop if receiving on{add,remove}track from WebMediaStream

MediaRecorder: make EventListener and stop if receiving on{add,remove}track from WebMediaStream

MediaRecorder is supposed to stop if the amount of tracks of the
associated MediaStream varies. This CL adds an EventListener to
MR to listen to Blink MediaStream.{add,remove}track and simply
signals an error to MR in that case. Added a LayoutTest as well.

Note that MediaStream did _not_ send an {add,remove}track if the
added/removed track was local, it did only for remote, so this
CL changes that (yes, needs review from MS folks), from the 
spec [1]:
> onaddtrack of type EventHandler,
> This event handler, of type addtrack, is executed when a MediaStreamTrack is added to the MediaStream.
[...]
> onremovetrack of type EventHandler,
> This event handler, of type removetrack, is executed when a MediaStreamTrack is removed from the MediaStream.


BUG=545156
[1] http://www.w3.org/TR/mediacapture-streams/#attributes

[mcasas, 2016-01-09 02:53:03]

Description was changed from

==========
MediaRecorder: listen and stop if receiving on{add,remove}track Events from
WebMediaStream

MediaRecorder is supposed to stop if the amount of tracks of the
associated MediaStream varies. This CL adds an EventListener to
MR to listen to Blink MediaStream.{add,remove}track and simply
signals an error to MR in that case. Added a LayoutTest as well.

Note that MediaStream did _not_ send an {add,remove}track if the
added/removed track was local, it did only for remote, so this
CL changes that (yes, needs review from MS folks).

BUG=545156
==========

to

==========
MediaRecorder: listen and stop if receiving on{add,remove}track Events from
WebMediaStream

MediaRecorder is supposed to stop if the amount of tracks of the
associated MediaStream varies. This CL adds an EventListener to
MR to listen to Blink MediaStream.{add,remove}track and simply
signals an error to MR in that case. Added a LayoutTest as well.

Note that MediaStream did _not_ send an {add,remove}track if the
added/removed track was local, it did only for remote, so this
CL changes that (yes, needs review from MS folks), from the 
spec [1]:
> onaddtrack of type EventHandler,
> This event handler, of type addtrack, is executed when a MediaStreamTrack is
added to the MediaStream.
[...]
> onremovetrack of type EventHandler,
> This event handler, of type removetrack, is executed when a MediaStreamTrack
is removed from the MediaStream.


BUG=545156

[1] http://www.w3.org/TR/mediacapture-streams/#attributes
==========

[mcasas, 2016-01-09 02:58:12]

Description was changed from

==========
MediaRecorder: listen and stop if receiving on{add,remove}track Events from
WebMediaStream

MediaRecorder is supposed to stop if the amount of tracks of the
associated MediaStream varies. This CL adds an EventListener to
MR to listen to Blink MediaStream.{add,remove}track and simply
signals an error to MR in that case. Added a LayoutTest as well.

Note that MediaStream did _not_ send an {add,remove}track if the
added/removed track was local, it did only for remote, so this
CL changes that (yes, needs review from MS folks), from the 
spec [1]:
> onaddtrack of type EventHandler,
> This event handler, of type addtrack, is executed when a MediaStreamTrack is
added to the MediaStream.
[...]
> onremovetrack of type EventHandler,
> This event handler, of type removetrack, is executed when a MediaStreamTrack
is removed from the MediaStream.


BUG=545156

[1] http://www.w3.org/TR/mediacapture-streams/#attributes
==========

to

==========
MediaRecorder: make EventListener and stop if receiving on{add,remove}track from
WebMediaStream

MediaRecorder is supposed to stop if the amount of tracks of the
associated MediaStream varies. This CL adds an EventListener to
MR to listen to Blink MediaStream.{add,remove}track and simply
signals an error to MR in that case. Added a LayoutTest as well.

Note that MediaStream did _not_ send an {add,remove}track if the
added/removed track was local, it did only for remote, so this
CL changes that (yes, needs review from MS folks), from the 
spec [1]:
> onaddtrack of type EventHandler,
> This event handler, of type addtrack, is executed when a MediaStreamTrack is
added to the MediaStream.
[...]
> onremovetrack of type EventHandler,
> This event handler, of type removetrack, is executed when a MediaStreamTrack
is removed from the MediaStream.


BUG=545156

[1] http://www.w3.org/TR/mediacapture-streams/#attributes
==========

[mcasas, 2016-01-09 03:18:22]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-01-09 03:18:30]

Patchset #1 (id:20001) has been deleted

[mcasas, 2016-01-09 04:24:50]

mcasas@chromium.org changed reviewers:
+ tommi@chromium.org

[mcasas, 2016-01-09 04:24:51]

tommi@ PTAL (or send to perkj@)

[perkj_chrome, 2016-01-11 08:21:20]

perkj@chromium.org changed reviewers:
+ perkj@chromium.org

[perkj_chrome, 2016-01-11 08:21:20]

Hej- I am afraid this is against the spec.

From the spec:
http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack 

A new MediaStreamTrack has been added to this stream. Note that this event is
not fired when the script directly modifies the tracks of a MediaStream.

https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html
(right):

https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:78:
stream1.onaddtrack = function() {};
This does not seem right according to spec. (it was correct from the beginning)
http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack

[mcasas, 2016-01-11 15:31:34]

On 2016/01/11 08:21:20, perkj_chrome wrote:
> Hej- I am afraid this is against the spec.
> 
> From the spec:
> http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack 
> 
> A new MediaStreamTrack has been added to this stream. Note that this event is
> not fired when the script directly modifies the tracks of a MediaStream.
> 
>
https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html
> (right):
> 
>
https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:78:
> stream1.onaddtrack = function() {};
> This does not seem right according to spec. (it was correct from the
beginning)
> http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack

Per, the Section you're quoting is non-normative and. IMHO,
it makes sense that onaddtrack is removed after any addtrack,
regardless of the nature of the track being added. Filed a bug
against the Spec for this inconsistency:
https://github.com/w3c/mediacapture-main/issues/302

IMHO, any {add,remove}track should fire and event named
on{add,remove}track, so please comment in the bug why it
shouldn't be like that (or rename the event).

[mcasas, 2016-01-11 22:07:43]

On 2016/01/11 15:31:34, mcasas wrote:
> On 2016/01/11 08:21:20, perkj_chrome wrote:
> > Hej- I am afraid this is against the spec.
> > 
> > From the spec:
> > http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack 
> > 
> > A new MediaStreamTrack has been added to this stream. Note that this event
is
> > not fired when the script directly modifies the tracks of a MediaStream.
> > 
> >
>
https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
> > File
> >
>
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/1570263002/diff/40001/third_party/WebKit/Layo...
> >
>
third_party/WebKit/LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:78:
> > stream1.onaddtrack = function() {};
> > This does not seem right according to spec. (it was correct from the
> beginning)
> > http://www.w3.org/TR/mediacapture-streams/#event-mediastream-addtrack
> 
> Per, the Section you're quoting is non-normative and. IMHO,
> it makes sense that onaddtrack is removed after any addtrack,
> regardless of the nature of the track being added. Filed a bug
> against the Spec for this inconsistency:
> https://github.com/w3c/mediacapture-main/issues/302
> 
> IMHO, any {add,remove}track should fire and event named
> on{add,remove}track, so please comment in the bug why it
> shouldn't be like that (or rename the event).

After talking to hta@ offline, and seeing that the consensus 
is that on{add,remove}track is only fired when calling 
{add,remove}track _with remote tracks_, I guess I'll close this
CL and try a third time :P
Thanks perkj@ for your time!