Event listener lifetime is incorrect for MessagePort and BroadcastChannel
Reported by
k...@luminance.org,
Nov 23
|
|||||||
Issue description
UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0
Steps to reproduce the problem:
--- from an extension content script ---
1. Create an iframe pointing at about:blank. Load a simple script in it, which I'll refer to as 'sandbox' from here on
--- from the sandbox ---
2. register a 'message' event listener on its parent window
--- from the content script ---
3. Create a MessageChannel, store it. Port1 of this channel will be referred to as 'contentToPage' and port2 will be referred to as 'pageToContent'
4. Add a message event listener to port1 aka contentToPage that console.logs the event data.
5. Transfer port2 aka pageToContent to the sandbox via window.postMessage.
6. Perform a window.setTimeout to schedule an operation I'll refer to as 'detach', defined further below. Give it a moderate delay, like 1000ms
7. Perform a window.setTimeout to schedule an operation I'll refer to as 'chat', defined further below. Give it a long delay, like 5000ms
--- from the sandbox ---
8. Receive pageToContent from the content script. Store it.
9. Unregister your message event listener from the parent window.
10. Add an event listener to pageToContent that console.logs the event data.
11. postMessage "hello world" to the content script through pageToContent.
--- from the content script ---
12. the event listener should log "hello world" because it received it from the sandbox.
13. 'detach' should be triggered by setTimeout. It removes the iframe from the page. This should have no observable effect other than the removal of the iframe.
14. 'chat' should be triggered by setTimeout. it postMessages "hello sandbox" to the sandbox, via contentToPage.
--- from the sandbox ---
15. the sandbox's event listener should log "hello sandbox" because it received it from the content script.
What is the expected behavior?
Event listeners on both sides of the content/page boundary should remain functioning as long as the channel's ports are open
What went wrong?
In every version after 70, the event listener from the sandbox is silently detached from the port and never receives events again.
Both ports are still open and you can postMessage through them. Nothing has been garbage collected.
Did this work before? Yes Chrome v70
Does this work in other browsers? Yes
Chrome version: Version 72.0.3619.0 (Official Build) canary (64-bit) Channel: canary
OS Version: 10.0
Flash Version:
It's intended and well documented that unloading an iframe will basically detach its Window object, disabling most of its APIs. That's fine. The parent window object remains functioning, as do its APIs, and that's sufficient. All of the functions and objects created in the iframe remain valid as long as they are retained by the parent window (in a local variable, for example). This works in both 70 and later versions.
The issue specifically appears to be event listener lifetime. The ports are not erroneously being closed or disentangled, the event listeners on them just go away. BroadcastChannel has the same problem. Event listeners created by JS in the parent window remain registered and functioning.
I'm going to try to create a reduced test case here, but naturally, it's very complex to reproduce. This breaks my extension (w/ ~70k active weekly users) presently though I've come up with an elaborate workaround for the GC being broken.
The workaround I've identified that clearly demonstrates the problem is essentially to change this:
port.addEventListener("message", myEventHandler);
to this:
parentWindow.port = port;
parentWindow.myEventHandler = myEventHandler;
parentWindow.eval("port.addEventListener('message', function (evt) { myEventHandler(evt); }");
,
Nov 23
My guess was that it could be trying to schedule the events on the iframe, but that would be wrong for it to do since the message port is owned by the parent window, not the iframe (it was transferred there). On v70, as expected, a message port transferred to the iframe will stop working once the iframe is removed from the page.
,
Nov 23
The bug can be observed using this test case. Load the html file, click 'send to sandbox' and 'send from sandbox' once, then click detach and click them again. Chrome 70 output: sandbox got: message from page page got: message from sandbox sandbox got: message from page page got: message from sandbox Chrome 72 output: sandbox got: message from page page got: message from sandbox page got: message from sandbox
,
Nov 25
,
Nov 26
Able to reproduce the issue on reported chrome version 72.0.3619.0 using Windows-10, Mac 10.12.6 & Ubuntu 14.04 hence providing Bisect Info Bisect Info: ================ Good build: 71.0.3543.0 Bad build: 71.0.3544.0 Note: On running per-revision bisect facing error, hence providing below change-log from chromium bisect You are probably looking for a change made after 588746 (known good), but no later than 588763 (first known bad). Change-Log: https://chromium.googlesource.com/chromium/src/+log/67a71bb8f222f16b93fd8bdf478cc7a10de52f35..d04bcae6a5618e984746ab4dd08549a3e89562db Suspecting: https://chromium.googlesource.com/chromium/src/+/da27378f03ea3dc4bdba82e3cb1d13ad2fef7a7c from above change log Change-Id: I4bf7fe2f4fd51287f873c2377edfd57e4bb2a54a Reviewed-on: https://chromium-review.googlesource.com/1172234 Note: As the author(Yuki Yamada) of the CL last visit is more than 30 days, hence assigning it to reviewer(Kentaro Hara) @Kentaro Hara: Please confirm the issue and help in re-assigning if it is not related to the change. Adding ReleaseBlock-Stable for M-71, feel free to remove it if not applicable. Thanks!
,
Nov 26
yukishiino: Would you mind taking a look at this?
,
Nov 27
This is an intended behavior. Blink is now checking the registered function's realm (= window in this case). The unnamed function created in iframe (line 18 in messageport-test.html) will not be invoked once the iframe gets detached. This restriction applies not only to event listeners but also all IDL callback functions and IDL callback interfaces.
,
Nov 27
If this is intended it should be in the relevant specs. I didn't see it in them when I read through. Was that my oversight? Is it only in drafts?
,
Nov 27
I can confirm also from testing that setTimeout, setInterval and requestAnimationFrame silently drop handlers from the iframe in version 70, so only messageport and broadcastchannel were changed. It makes sense to make these consistent with each other. However, this is fully inconsistent with both Firefox and Edge. In firefox all of them except requestAnimationFrame work, and in Edge all the event handlers work including rAF. From my reading of the spec, Edge seems correct and Chrome is entirely incorrect. Given this, it seems like either Chrome is intentionally broken now, or both FF and Edge implement the spec incorrectly and I've misread it. Is your argument that the latter is true and I should file bugs against FF and Edge?
,
Nov 28
It's a long term known issue that Blink doesn't work with full features on detached frames, and none of user agents nor standards have good agreements. In your term, "intentionally broken" might be close to the situation. Regardless of whether Blink invokes event listeners or not, Blink does not work with full features on detached frames due to technical internal issues. I'm not sure which is better whether to invoke IDL callback and something sometimes fails (this may be harder to debug) or to not invoke IDL callback and it always fails (this may be easier to debug).
,
Dec 17
Issue 915355 has been merged into this issue. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by mek@google.com
, Nov 23