Should BroadcastChannel check for closed flag before dispatching events?

[mkruisselbrink]

Currently the way BroadcastChannel.postMessage is specified, the postMessage method synchronously collects all destination channels (that match and aren't closed), and then queues tasks for all of these channels to dispatch a message event. This seems counterintuitive in code like this:

let c1 = new BroadcastChannel('foo');
let c2 = new BroadcastChannel('foo');
c2.onmessage = e => { console.log(e.data); c2.close(); };
c1.postMessage('first');
c1.postMessage('second');

According to the spec the expected result of this is for both 'first' and 'second' to be logged to the console. Because despite c2 closing itself after receiving the first event, the second event has already been queued and should still be delivered. This behavior doesn't seem to match the firefox implementation, which won't deliver the second event to c2 if it closed itself already in an earlier message event. So should the spec be modified to have a check inside the queued task to make sure the BroadcastChannel object still is not closed?

But on the other hand the behavior as specced is consistent with the equivalent code using MessageChannel/MessagePort (https://jsfiddle.net/c9mbo2c3/2/), where at least firefox and chrome seem to actually match the spec.

So should the spec change for BroadcastChannel, or should firefox fix their implementation? (And if the spec is changed, should a similar change be made for MessagePort? Even though the implementations I tried there curently match the spec)

[mkruisselbrink]

The behavior as specified seems particularly weird when c1 and c2 live in different threads/event loops. As in that case it seems perfectly legal from a spec point of view for the task queued by the first postMessage to execute before the second postMessage is called, which means that in that case it is undefined if the second message gets delivered or not.

[domenic]

@bakulf, would you be able to weigh in with Gecko's perspective?

My weak opinion is that being consistent with MessageChannel and MessagePort is more important, and Firefox should probably fix their implementation.

[bakulf]

Gecko collects all destination channels asynchronously. I can change it. Bug 1278340.

[smaug----]

My suggestion was to process close() asynchronously for receiving but synchronously for posting message.

[bakulf]

Right, This is actually what we do. In this way, if there are messages in the process to be received, they will be processed before 'close()'. Of course, close() synchronously, sets a flag so that any postMessage() will be ignored.

[gterzian]

I think we can add to this:

• Check for the worker not closing
• Check for the document being fully-active.

Basically, all the checks of step 7, except for the channel name, which isn't going to change.

So I'll open a PR and move them to Step 10, inside the task.

[gterzian]

Ah no that's incorrect, since the worker closing, or the document not being fully-active, aren't things you can check from inside a task if they are positive and negative respectively(the task will by definition not run?).

I'll focus on the close flag then. There is already a test in the suite that links to this issue BTW.