Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BroadcastChannel: potential bug in "close" concept #5304

Closed
gterzian opened this issue Feb 19, 2020 · 3 comments
Closed

BroadcastChannel: potential bug in "close" concept #5304

gterzian opened this issue Feb 19, 2020 · 3 comments

Comments

@gterzian
Copy link
Member

gterzian commented Feb 19, 2020

Firstly, I think there might be a bug in the spec with regards to the use of close when broadcasting a message.

Currently, close is used as a filter to determine the channels to include in the "destinations" list, at Step 7 of https://html.spec.whatwg.org/multipage/#dom-broadcastchannel-postmessage

At that point, we're inside the task from where the message is "sent".

Step 10 then says to queue a task for each channel that was added to the "destination" list, in order to actually fire the MessageEvent.

So, would it not make more sense to check the close flag from inside the task queued at 10, to determine at the receiving end whether the destination channel has been closed already?

@gterzian gterzian changed the title BroadcastChannel: potential bug in "close" concept, question on destination list. BroadcastChannel: potential bug in "close" concept Feb 19, 2020
@gterzian
Copy link
Member Author

gterzian commented Feb 19, 2020

Ok so I've had a look at various UAs, the spec doesn't follow implementations, as they seem to be checking the close flag before firing the event from inside the task that is queued, not at step 7 when filtering for "destinations".

For example, the below test passes in FF and Chrome(my manual check), and should fail as per the current wording of the spec.

async_test(t => {
    let c1 = new BroadcastChannel('closed');
    let c2 = new BroadcastChannel('closed');

    c2.onmessage = t.unreached_func();
    
    c3.onmessage = t.step_func(() => t.done());
    // Note: not closed here. 
    // Per the spec the close flag is checked in the call to postMessage,
    // before queuing tasks to fire message events.
    c1.postMessage('test');
 
    // Note: closing after the tasks have already been queued.
    c2.close();
  }, 'messages aren\'t delivered to a closed port');

@gterzian
Copy link
Member Author

gterzian commented Feb 19, 2020

Dupe of #1371

Should we try to close both? That one is from 2016...

@annevk
Copy link
Member

annevk commented Feb 19, 2020

Ah, thanks for finding that. If you want to write a PR and tests, that'd be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants