Bug 1317154: Correctly support multiple concurrent listeners for the same event. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 18 Nov 2016 12:20:22 -0800
changeset 441386 99aee3d711cd45f32c67eb1469e7d8994f42dde3
parent 441385 42a9728ad5f8b61596cbca5c3b7d83c8e84dc74c
child 537542 0d86be32f36569b22a7e48b86ae0b98a9b6b0341
push id36410
push usermaglione.k@gmail.com
push dateFri, 18 Nov 2016 20:21:09 +0000
reviewersaswan
bugs1317154
milestone53.0a1
Bug 1317154: Correctly support multiple concurrent listeners for the same event. r?aswan MozReview-Commit-ID: 4OgukI6Sc6v
browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/test/mochitest/mochitest-common.ini
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_listener_proxies.html
--- a/browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
+++ b/browser/components/extensions/test/browser/browser_ext_windows_create_tabId.js
@@ -99,18 +99,16 @@ add_task(function* testWindowCreate() {
       let readyPromise = Promise.all([
         // tabs.onUpdated can be invoked between the call of windows.create and
         // the invocation of its callback/promise, so set up the listeners
         // before creating the window.
         promiseTabUpdated("http://example.com/"),
         promiseTabUpdated("http://example.org/"),
       ]);
 
-      await new Promise(resolve => setTimeout(resolve, 0));
-
       window = await browser.windows.create({url: ["http://example.com/", "http://example.org/"]});
       await readyPromise;
 
       browser.test.assertEq(2, window.tabs.length, "2 tabs were opened in new window");
       browser.test.assertEq("about:blank", window.tabs[0].url, "about:blank, page not loaded yet");
       browser.test.assertEq("about:blank", window.tabs[1].url, "about:blank, page not loaded yet");
 
       window = await browser.windows.get(window.id, {populate: true});
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -520,25 +520,25 @@ ParentAPIManager = {
           path: data.path,
           args: listenerArgs,
         },
         {
           recipient: {childId},
         });
     }
 
-    context.listenerProxies.set(data.path, listener);
+    context.listenerProxies.set(data.listenerId, listener);
 
     let args = Cu.cloneInto(data.args, context.sandbox);
     findPathInObject(context.apiObj, data.path).addListener(listener, ...args);
   },
 
   removeListener(data) {
     let context = this.getContextById(data.childId);
-    let listener = context.listenerProxies.get(data.path);
+    let listener = context.listenerProxies.get(data.listenerId);
     findPathInObject(context.apiObj, data.path).removeListener(listener);
   },
 
   getContextById(childId) {
     let context = this.proxyContexts.get(childId);
     if (!context) {
       throw new Error("WebExtension context not found!");
     }
--- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini
@@ -84,16 +84,17 @@ skip-if = os == 'android' # Bug 1258975 
 [test_ext_background_api_injection.html]
 [test_ext_background_generated_url.html]
 [test_ext_background_teardown.html]
 [test_ext_tab_teardown.html]
 skip-if = (os == 'android') # Android does not support tabs API. Bug 1260250
 [test_ext_unload_frame.html]
 [test_ext_i18n.html]
 skip-if = (os == 'android') # Bug 1258975 on android.
+[test_ext_listener_proxies.html]
 [test_ext_web_accessible_resources.html]
 skip-if = (os == 'android') # Bug 1258975 on android.
 [test_ext_webrequest_background_events.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
 [test_ext_webrequest_basic.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
 [test_ext_webrequest_filter.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -1,5 +1,4 @@
 [DEFAULT]
 tags = webextensions in-process-webextensions
 
-
 [include:mochitest-common.ini]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_listener_proxies.html
@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for content script</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(function* test_listener_proxies() {
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "temporary",
+
+    manifest: {
+      "permissions": ["storage"],
+    },
+
+    async background() {
+      // Test that adding multiple listeners for the same event works as
+      // expected.
+
+      let awaitChanged = () => new Promise(resolve => {
+        browser.storage.onChanged.addListener(function listener() {
+          browser.storage.onChanged.removeListener(listener);
+          resolve();
+        });
+      });
+
+      let promises = [
+        awaitChanged(),
+        awaitChanged(),
+      ];
+
+      function removedListener() {}
+      browser.storage.onChanged.addListener(removedListener);
+      browser.storage.onChanged.removeListener(removedListener);
+
+      promises.push(awaitChanged(), awaitChanged());
+
+      browser.storage.local.set({foo: "bar"});
+
+      await Promise.all(promises);
+
+      browser.test.notifyPass("onchanged-listeners");
+    },
+  });
+
+  yield extension.startup();
+
+  yield extension.awaitFinish("onchanged-listeners");
+
+  yield extension.unload();
+});
+</script>
+
+</body>
+</html>