1642671 - Add test to verify that falsey return values from runtime.onMessage are not ignored

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Rob Wu [:robwu]]

This doesn't look right: https://hg.mozilla.org/mozilla-central/rev/ca3057f6cdc5#l3.50

A consequence of the value && value check is that falsey return values are ignored, and the caller will receive undefined due to the ultimate fallback.

I'll fix this and add some tests.

Comment 1

[Rob Wu [:robwu]]

I was mistaken about the meaning of that line.

zombie clarified: that's not a bug, that listener returns an array of response values, or undefined if there are no listeners.

This bug is only about writing tests then, if we don't have them already.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1642671 - Add tests for falsey result from runtime.onMessage

Comment 3

[Rob Wu [:robwu]]

I patched MessageEvent.emit by triggering a failure when runtime.onMessage's callback returns a falsey value other than undefined. Surprisingly out of all extension tests, only one test failed, because its onMessage handler coincidentally received a null value. The caller didn't check the returned result though. So I have added a test to make sure that we have sufficient coverage for the scenario of receiving a falsey result in onMessage.

This is the patch that I used to run the check:

diff --git a/toolkit/components/extensions/ExtensionChild.jsm b/toolkit/components/extensions/ExtensionChild.jsm
index a9e8a2c576576..14a1b2462f579 100644
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -181,6 +181,17 @@ class MessageEvent extends SimpleEventAPI {
       .map(fire => this.wrapResponse(fire, message, sender))
       .filter(x => x !== undefined);
 
+    const abort = (msg) => {
+      (this.context.chromeObj || this.context.cloneScope.wrappedJSObject.browser).test.fail(msg);
+    };
+    all.forEach(x => {
+      if (x && x instanceof Promise) {
+        x.then(v => {
+          v || v === undefined || abort("FALSEY??? " + v);
+        }, () => {});
+      }
+    });
+
     return all.length ? Promise.race(all).then(v => [v]) : [];
   }
 

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36f295b440e6
Add tests for falsey result from runtime.onMessage r=zombie,geckoview-reviewers,agi

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/36f295b440e6