1465514 - Extension port disconnected when one of the recipients is unloaded

Status: NEW

(WebExtensions :: General, defect, P3)

Description

[Rob Wu [:robwu]]

Attachment: port-multi-navigate.zip

An extension port is a channel between 1 sender and >=0 recipients:
- runtime.connect: All extension pages/frames in the extension process are recipients.
- tabs.connect: Unless frameId is set, all frames in a tab are recipients.

Currently, a port is disconnected if:
- either side calls port.disconnect().
- either side unloads.

The last one should be changed, to:
- the sender unloads, or *all* recipients unload.

The exact expectations for the port lifetime are documented at https://developer.chrome.com/extensions/messaging#port-lifetime

STR:
1. Load the attached extension.
   The extension shows a tab and a frame (=two recipients in the extension process).
2. Open example.com in a new tab.
3. Click on the extension button (which calls runtime.connect)
   (The frame will unload upon onConnect)

Expected result:
- The port should still be alive, which is shown by a response.
- The page should show:
runtime.connect() ... waiting for response (expecting response before disconnect)
Received message: Message from main page (the reloaded frame did not close the channel, or the frame was removed)

Actual result:
- The port is disconnected when the frame was unloaded.
- The page shows:
runtime.connect() ... waiting for response (expecting response before disconnect) port.onDisconnect; error= null

Comment 1

[Shane Caraveo (:mixedpuppy)]

Year-old assigned bug, can you re-triage?

Comment 2

[Rob Wu [:robwu]]

This should be fixed, but to avoid duplicate work it is probably best when we work on it during or after the refactor of the messaging implementation (MessageChannel and such).

I'll keep it P2, but won't be surprised if this stays untouched for a couple more months.

Comment 3

[Tomislav Jovanovic :zombie]

(In reply to Rob Wu [:robwu] from comment #0)

The last one should be changed, to:

• the sender unloads, or all recipients unload.

The exact expectations for the port lifetime are documented at
https://developer.chrome.com/extensions/messaging#port-lifetime

I'm not sure we want to change this, if the author expects all of the listeners which ever responded on the port are still listening until it gets a onDisconnect, there might be cleanup bugs.

Anyway, I'm currently rewriting the Ports implementation in bug 1583484, and I'll see if changes here might be easier with the new design.

Comment 4

[Tomislav Jovanovic :zombie]

This is just so stupid:

You may want to find out when a connection is closed, for example if you are maintaining separate state for each open port. For this you can listen to the runtime.Port.onDisconnect event. This event is fired when there are no valid ports at the other side of the channel. This happens in the following situations:

• There are no listeners for runtime.onConnect at the other end.
• The tab containing the port is unloaded (e.g. if the tab is navigated).
• The frame from where connect was called has unloaded.
• All frames that received the port (via runtime.onConnect) have unloaded.
• runtime.Port.disconnect is called by the other end. Note that if a connect call results in multiple ports at the receiver's end, and disconnect() is called on any of these ports, then the onDisconnect event is only fired at the port of the sender, and not at the other ports.

Comment 5

[Rob Wu [:robwu]]

The issue came up again in https://github.com/mdn/content/pull/22257

This bug was not resolved by the refactor from MessageChannel to conduits (bug 1583484). BroadcastConduit acts as a relay of messages, and closes the sender when any of the recipients closes: https://searchfox.org/mozilla-central/rev/aa329cf7506ddd966542e642ec00223fd7461599/toolkit/components/extensions/ConduitsParent.jsm#211-225

The .disconnect() API also relies on that mechanism: https://searchfox.org/mozilla-central/rev/aa329cf7506ddd966542e642ec00223fd7461599/toolkit/components/extensions/ExtensionChild.jsm#292

In fixing the behavior for closing conduits (due to unload), we should make sure that the close-port-immediately aspect of disconnect() is also preserved.

Comment 6

[Evangelos]

can this also affect the connectNative function?

I'm trying to run this web-extension example: https://github.com/mdn/webextensions-examples/tree/main/native-messaging on Linux (latest firefox) and macOs (latest firefox dev) and on calling browser.runtime.connectNative('ping_pong'); I get a disconnect event with a null error object.

I think regardless if this is a bug or expected behavior,
it should be:

• documented (mdn? jsdocs?)
• the disconnect object should contain a human-readable error, if this is an error, or a message if its not