1705080 - MediaStreams are not compliant with EventTarget behaviour

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[michel.neumann]

Attachment: example.js

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_2_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.128 Safari/537.36

Steps to reproduce:

For a project using WebRTC, we are currently emitting "addtrack" and "removetrack" events explicitly on MediaStream instances to get notified for track changes to trigger rtp sender updates and re-negotiation on the peer connection managing that stream.

According to the MDN documentation and W3C, MediaStreams are derivatives of EventTargets, and therefore support the Pub/Sub pattern with "addEventListener" and "dispatchEvent". In the current Firefox, the event listeners are not executed as expected.

Please see the attached code examples.

Actual results:

In both examples, a media stream is created. Event listeners are added to the stream and several custom events are dispatched on the media stream. In the first example, none of the expected callbacks were executed. In the second example, only the "onaddtrack" callback is executed.

Expected results:

In the first example, two callbacks should be executed and printed to the console. In the second example, four callbacks should be executed and printed to the console. This example runs as expected in Chrome 89 and Safari 14, but not in Firefox 87, Dev Edition and Firefox Android 87.

Comment 1

[michel.neumann]

Comment on attachment 9215770 [details]
example.js

>(() => {
>    const stream = new MediaStream();
>    // Counter for successful executions
>    let counter = 0;
>
>    /**
>     * Test: Dispatch a custom event on a media stream which implements EventTarget.
>     */
>    stream.addEventListener("myEvent", () => {
>        // Expect this being logged; and it is not
>        console.log("addEventListener:myEvent");
>        counter++;
>    });
>
>    stream.addEventListener("myCustomEvent", () => {
>        // Expect this being logged; and it is not
>        console.log("addEventListener:myCustomEvent");
>        counter++;
>    });
>
>    stream.dispatchEvent(new Event("myEvent"));
>    stream.dispatchEvent(new CustomEvent("myCustomEvent"));
>
>    // Print result
>    if (counter !== 2) {
>        console.warn("Tests failed: Expected 2 executions, got %d", counter);
>    } else {
>        console.log("Tests succeeded");
>    }
>})();
>
>/**
> * Test with getUserMedia and MediaStreamTrackEvent
> */
>(async () => {
>    const stream = await navigator.mediaDevices.getUserMedia({ video: true });
>    // Request a webcam to get a media stream track
>    const track = stream.getVideoTracks()[0];
>    // Counter for successful executions
>    let counter = 0;
>
>    /**
>     * Test 1: Dispatch an event defined by the standards.
>     */
>    stream.addEventListener("addtrack", () => {
>        // Expect this being logged; but is not logged by Firefox
>        console.log("addEventListener:addtrack");
>        counter++;
>    });
>
>    stream.onaddtrack = () => {
>        // Expect this being logged by ALL browsers
>        console.log("onaddtrack");
>        counter++;
>    };
>
>    stream.dispatchEvent(new MediaStreamTrackEvent("addtrack", { track }));
>
>    /**
>     * Test 2: Dispatch a custom event on a media stream which implements EventTarget.
>     */
>    stream.addEventListener("myEvent", () => {
>        // Expect this being logged; but is not logged by Firefox
>        console.log("addEventListener:myEvent");
>        counter++;
>    });
>
>    stream.addEventListener("myCustomEvent", () => {
>        // Expect this being logged; but is not logged by Firefox
>        console.log("addEventListener:myCustomEvent");
>        counter++;
>    });
>
>    stream.dispatchEvent(new Event("myEvent"));
>    stream.dispatchEvent(new CustomEvent("myCustomEvent"));
>
>    // Print result
>    if (counter !== 4) {
>        console.warn("Tests failed: Expected 4 executions, got %d", counter);
>    } else {
>        console.log("Tests succeeded");
>    }
>})();

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::WebRTC: Audio/Video' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[Paul Adenot (:padenot)]

Jan-Ivar, please have a look at this.

Comment 4

[Felix Waterstraat]

It would be great, if someone from Mozilla could have a look at this issue!

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Confirmed. I see the same thing https://jsfiddle.net/jib1/1v0Lgw2y/

Firefox: Expected 4 executions, got 1
Chrome & Safari: Expected 4 executions, got 4

This is odd, since dispatchEvent should be inherited.

MediaStream is implemented by DOMMediaStream which inherits from DOMEventTargetHelper which has that method, which is ultimately the same one called internally.

Olli, is there some trick to this?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Hmm, it works for other event targets, like XMLHttpRequest.

MediaStream constructor doesn't seem to pass window to DOMEventTargetHelper
https://searchfox.org/mozilla-central/rev/e3c34d34cc7dce56df377f05183ad379ac1de123/dom/media/DOMMediaStream.cpp#120-121
vs
https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/dom/xhr/XMLHttpRequestEventTarget.h#21

Because of that difference, do we end up returning false from
https://searchfox.org/mozilla-central/rev/5359952d8b0be3e706e8c943c2bef2674723b8a9/dom/events/DOMEventTargetHelper.cpp#235 which would explain why trusted events get handled but untrusted don't.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1705080, make MediaStreams behave as normal EventTargets, r=jib

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

I think MediaStreamTrack is also broken.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10d327bb5893
make MediaStreams behave as normal EventTargets, r=jib

Comment 10

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/10d327bb5893

Comment 11

[michel.neumann]

Thanks for looking into this!