Bugzilla

(In reply to Andrea Giammarchi from comment #21)
> > Who is mutating the view though?
> 
> main receives `Int32Array` view with `SharedArrayBuffer` in it, and it does `i32v[0] = 1` on the main thread ... that's it, it notifies at index `0` something changes, that index `0` is then reset to have `0` as value right after, to avoid waiting next time on a different value.

A-ha!  I hadn't appreciated that the message might be sending a SharedArrayBuffer in it  We have checks about whether that's okay to deserialize which depend on both an [agent cluster check](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/ipc/SharedMessageBody.cpp#84-92) ([agent cluster spec link](https://html.spec.whatwg.org/multipage/webappapis.html#agents-and-agent-clusters)) and [a check on the global if we're allowed to use shared memory](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/ipc/SharedMessageBody.cpp#94-96) ([which hinges on COOP/COEP stuff](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cross-Origin-Embedder-Policy#features_that_depend_on_cross-origin_isolation) and [it looks like crossOriginIsolated may be how we surface that on globals to content](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer#security_requirements)) and where we'll [fail on deserialization](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/js/src/vm/StructuredClone.cpp#2900-2906) and then [fire a "messageerror" event](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/broadcastchannel/BroadcastChannel.cpp#516-520).  I think [browsers like Chrome have potentially doing things about the COEP requirements that I am not fully up on](https://developer.chrome.com/blog/shared-array-buffer-origin-trial-extension-124) and I think we may be more strict than other browsers in some cases, so it's possible we are deviating from other browsers on this.

This is something we definitely should have coverage for, but I'm not finding coverage in [our mochitests for BroadcastChannel that were added with the RefMessageBodyService that would have helped with this](https://searchfox.org/mozilla-central/source/dom/base/test/test_postMessages_broadcastChannel.html) or immediately in the web-platform-tests.  If you can easily confirm if you were experiencing a "messageerror" or receiving a SharedArrayBuffer that was broken (like we failed to actually share the memory), that's appreciated but it will definitely be an action item for my test stack to make sure we have in-tree test coverage for the SharedArrayBuffer case for BroadcastChannel.

(In reply to Andrea Giammarchi from comment #23)
> > For performance purposes, in general I think you would want to avoid BroadcastChannel since ...
> 
> OK, in advance, apologies that did hit a nerve (days if not weeks behind trying to find fixes around this issue) ... 

No worries, thank you for apologizing and my apologies that while I was aiming for terse in my phrasing (I tend to be overly verbose), upon re-reading what I wrote I think I came off brusque.  I very much appreciate how frustrating Firefox's technical debt can be and how when trying to workaround the technical debt, one can run into other technical debt that makes the workaround even harder!  I also very, very much appreciate that in a world where Firefox's market share is small, it's a major effort to try and support Firefox and that it takes a lot of effort to report these bugs and provide reproducible test cases as you've done in https://bugzilla.mozilla.org/show_bug.cgi?id=1956778#c6.  Thank you for caring about supporting Firefox and helping keep the web more than just two browser engines!

Expanding on what I made too terse and going into a little more detail on the Firefox implementation details for BroadcastChannel: all our messages end up being sent from whichever thread they're on in the content process via IPC to our parent process "IPDL Background" thread which is shared across all content processes and origins, so it can be subject to a fair amount of contention (all our storage APIs get routed through there too, for example).  That currently happens for our MessageChannel implementation too, but I will be fixing that in this bug to use our IPC mechanism to directly bind the source and target threads in the same process so "IPDL Background" will no longer be involved and I think this will bring us into line with Chrome/Blink.  For Worker.postMessage, we always just directly dispatch a runnable to the target thread.

I think other browsers will similarly have a need to perform rendezvous for BroadcastChannel in some central location, although other browsers may already do a better job of using different threads for different origins for that.  Which is to say, all things being equal, I think Worker.postMessage and transferred MessageChannel ports are going to have the most reliably low latency and I don't know that there's any documentation out there that helps make that clear.  BroadcastChannel should definitely work and it is a very useful API!  It's a real lifesaver for tests in particular where plumbing all the MessagePorts around can be a logistical nightmare.

(In reply to Andrea Giammarchi from comment #21)
> > Who is mutating the view though?
> 
> main receives `Int32Array` view with `SharedArrayBuffer` in it, and it does `i32v[0] = 1` on the main thread ... that's it, it notifies at index `0` something changes, that index `0` is then reset to have `0` as value right after, to avoid waiting next time on a different value.

A-ha!  I hadn't appreciated that the message might be sending a SharedArrayBuffer in it.  We have checks about whether that's okay to deserialize which depend on both an [agent cluster check](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/ipc/SharedMessageBody.cpp#84-92) ([agent cluster spec link](https://html.spec.whatwg.org/multipage/webappapis.html#agents-and-agent-clusters)) and [a check on the global if we're allowed to use shared memory](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/ipc/SharedMessageBody.cpp#94-96) ([which hinges on COOP/COEP stuff](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cross-Origin-Embedder-Policy#features_that_depend_on_cross-origin_isolation) and [it looks like crossOriginIsolated may be how we surface that on globals to content](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer#security_requirements)) and where we'll [fail on deserialization](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/js/src/vm/StructuredClone.cpp#2900-2906) and then [fire a "messageerror" event](https://searchfox.org/mozilla-central/rev/de87ab8fb7d046a068943d4728a2345568ff77c5/dom/broadcastchannel/BroadcastChannel.cpp#516-520).  I think [browsers like Chrome have potentially doing things about the COEP requirements that I am not fully up on](https://developer.chrome.com/blog/shared-array-buffer-origin-trial-extension-124) and I think we may be more strict than other browsers in some cases, so it's possible we are deviating from other browsers on this.

This is something we definitely should have coverage for, but I'm not finding coverage in [our mochitests for BroadcastChannel that were added with the RefMessageBodyService that would have helped with this](https://searchfox.org/mozilla-central/source/dom/base/test/test_postMessages_broadcastChannel.html) or immediately in the web-platform-tests.  If you can easily confirm if you were experiencing a "messageerror" or receiving a SharedArrayBuffer that was broken (like we failed to actually share the memory), that's appreciated but it will definitely be an action item for my test stack to make sure we have in-tree test coverage for the SharedArrayBuffer case for BroadcastChannel.

(In reply to Andrea Giammarchi from comment #23)
> > For performance purposes, in general I think you would want to avoid BroadcastChannel since ...
> 
> OK, in advance, apologies that did hit a nerve (days if not weeks behind trying to find fixes around this issue) ... 

No worries, thank you for apologizing and my apologies that while I was aiming for terse in my phrasing (I tend to be overly verbose), upon re-reading what I wrote I think I came off brusque.  I very much appreciate how frustrating Firefox's technical debt can be and how when trying to workaround the technical debt, one can run into other technical debt that makes the workaround even harder!  I also very, very much appreciate that in a world where Firefox's market share is small, it's a major effort to try and support Firefox and that it takes a lot of effort to report these bugs and provide reproducible test cases as you've done in https://bugzilla.mozilla.org/show_bug.cgi?id=1956778#c6.  Thank you for caring about supporting Firefox and helping keep the web more than just two browser engines!

Expanding on what I made too terse and going into a little more detail on the Firefox implementation details for BroadcastChannel: all our messages end up being sent from whichever thread they're on in the content process via IPC to our parent process "IPDL Background" thread which is shared across all content processes and origins, so it can be subject to a fair amount of contention (all our storage APIs get routed through there too, for example).  That currently happens for our MessageChannel implementation too, but I will be fixing that in this bug to use our IPC mechanism to directly bind the source and target threads in the same process so "IPDL Background" will no longer be involved and I think this will bring us into line with Chrome/Blink.  For Worker.postMessage, we always just directly dispatch a runnable to the target thread.

I think other browsers will similarly have a need to perform rendezvous for BroadcastChannel in some central location, although other browsers may already do a better job of using different threads for different origins for that.  Which is to say, all things being equal, I think Worker.postMessage and transferred MessageChannel ports are going to have the most reliably low latency and I don't know that there's any documentation out there that helps make that clear.  BroadcastChannel should definitely work and it is a very useful API!  It's a real lifesaver for tests in particular where plumbing all the MessagePorts around can be a logistical nightmare.