Bug 1317154: Correctly support multiple concurrent listeners for the same event. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Fri, 18 Nov 2016 12:20:22 -0800
changeset 323503 aa98f9492ef7a1e9de2b21cf2d3b164b7a20955f
parent 323502 5d5b77bd538895c2cb305ccd4b0842cc754509ce
child 323504 d8dc1806bb827ab612513ea27918b361ae584d99
push id30978
push usercbook@mozilla.com
push dateMon, 21 Nov 2016 14:44:46 +0000
treeherdermozilla-central@0534254e9a40 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1317154
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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>