1552112 - Native messaging: Only first onDisconnect listener is being called

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P2)

Description

[Dominik]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.131 Safari/537.36

Steps to reproduce:

1. Create a temporary Add-on:

manifest.js:

{
"manifest_version": 2,

"name": "Native Messaging onDisconnect Test",
"description": "Native Messaging onDisconnect Test",
"version": "1",

"applications": {
"gecko": {
"id": "[e-mail]",
"strict_min_version": "66.0.5"
}
},

"background": {
"scripts":["background.js"],
"persistent": true
},

"permissions": [
"nativeMessaging"
]
}

background.js:

function connect() {
console.log("connect to non-existing native messaging host ...");
var port = chrome.runtime.connectNative("testondisconnect.example.com");
if (port) {
port.onDisconnect.addListener(function() {
console.log("onDisconnect listener #1");
});
port.onDisconnect.addListener(function() {
console.log("onDisconnect listener #2");
});
}
}
connect();

2. Load the temporary Add-on and debug it

• about:addons
• Debug Add-ons
• Enable add-on debugging
• Load Temporary Add-on
• Debug / Allow connection

3. Test failing native messaging connect

• In console execute

connect()

or alternatively click "Reload" on the about:debugging#addons page.

Actual results:

Console output:

connect to non-existing native messaging host ...
onDisconnect listener #1

The second onDisconnect listener is not being called.

Expected results:

Expected output:

connect to non-existing native messaging host ...
onDisconnect listener #1
onDisconnect listener #2

All attached onDisconnect listeners should be called.

Comment 1

[Alex Cornestean]

The issue can be reproduced on the latest Release version (66.05 – 64 bit), Beta (67.0 – 64 bit) and Nightly (68.0a1 – 64 bit) by following the above STR under ‘Ubuntu16.04 LTS’, ‘Windows 10 Pro 64-bit’ and ‘macOS High Sierra 10.13.6’.

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Looking at the related code from ExtensionChild.jsm the onDisconnect event is definitely going to call the two listeners, but I think that the second one is never going to be reached because the holder object (which should contain the error related to the disconnection) has already been deserialized when the first listener has been called and the second time it is called a NS_ERROR_NOT_INITIALIZED error is going to be thrown:

• https://searchfox.org/mozilla-central/rev/a887c90ea9c19a0b5529a1f5fa351929944887ba/toolkit/components/extensions/ExtensionChild.jsm#189-190

And so this is very likely a regression we introduced in 66 when in Bug 1476032 a new keepData parameter (which defaults to false) has been introduced on the StructuredCloneHolder's deserialize method.

Comment 3

[Tomislav Jovanovic :zombie]

Doesn't reproduce after changes in bug 1602639.

Comment 4

[Alex Cornestean]

Verified the fix on the latest Nightly (76.0a1/20200324214641), Beta (75.0b8/20200325013053) and Release (74.0/20200309095159) under Windows 10 Pro 64-bit and Ubuntu 16.04 LTS.

Current results are the ones mentioned originally as the expected results i.e both onDisconnect listeners are called, confirming the fix.

Also tested the latest ESR (68.6.0esr/20200305175243). The issue is still present on this version.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 6

[Tomislav Jovanovic :zombie]

(In reply to Alex Cornestean from comment #4)

Also tested the latest ESR (68.6.0esr/20200305175243). The issue is still present on this version.

This was not uplifted to ESR, and I don't think it is worth doing so.