Bug 1317154: Correctly support multiple concurrent listeners for the same event. r=aswan a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Fri, 18 Nov 2016 12:20:22 -0800
changeset 352558 d10dd0e435a92cdf940fbc5e5ab6529806380acf
parent 352557 16b857fbf1ab78641be7ea6c8f4e2cad163193cc
child 352559 c684ebca8623a319e2ebd93a8140de680bf88456
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan, jcristau
bugs1317154
milestone52.0a2
Bug 1317154: Correctly support multiple concurrent listeners for the same event. r=aswan a=jcristau 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.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
@@ -511,25 +511,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) {
       let error = new Error("WebExtension context not found!");
       Cu.reportError(error);
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -85,16 +85,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_suspend.html]
 skip-if = os == 'android' # webrequest api unsupported (bug 1258975).
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>