Bug 1448674 - avoid closing extension ports while detaching tabs r?aswan draft
authorRob Wu <rob@robwu.nl>
Fri, 06 Apr 2018 13:34:30 +0100
changeset 779935 a840af71ad6f40967cdbd2ed0b63a548b37810d3
parent 777163 ff0efa4132f0efd78af0910762aec7dcc1a8de66
push id105913
push userbmo:rob@robwu.nl
push dateTue, 10 Apr 2018 20:48:36 +0000
reviewersaswan
bugs1448674, 1445537
milestone61.0a1
Bug 1448674 - avoid closing extension ports while detaching tabs r?aswan Extension ports are automatically closed when the message manager of the source is destroyed. When a tab is detached from a window, its frameloader is moved to the new window and the original message manager is destroyed. Bug 1445537 started listening for SwapDocShells events, but that only works for the first swap (e.g. detaching a tab once). To avoid early disconnection of the port, we should continue to subscribe to SwapDocShells events. MozReview-Commit-ID: G2ZYAhNyHIL
browser/components/extensions/test/browser/browser-common.ini
browser/components/extensions/test/browser/browser_ext_connect_and_move_tabs.js
toolkit/components/extensions/ExtensionParent.jsm
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -65,16 +65,17 @@ skip-if = (os == 'win' && !debug) # bug 
 [browser_ext_browsingData_serviceWorkers.js]
 [browser_ext_chrome_settings_overrides_home.js]
 [browser_ext_commands_execute_browser_action.js]
 [browser_ext_commands_execute_page_action.js]
 [browser_ext_commands_execute_sidebar_action.js]
 [browser_ext_commands_getAll.js]
 [browser_ext_commands_onCommand.js]
 [browser_ext_commands_update.js]
+[browser_ext_connect_and_move_tabs.js]
 [browser_ext_contentscript_connect.js]
 [browser_ext_contextMenus.js]
 [browser_ext_contextMenus_checkboxes.js]
 [browser_ext_contextMenus_commands.js]
 [browser_ext_contextMenus_icons.js]
 [browser_ext_contextMenus_onclick.js]
 [browser_ext_contextMenus_radioGroups.js]
 [browser_ext_contextMenus_uninstall.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_connect_and_move_tabs.js
@@ -0,0 +1,95 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+// Tests that the Port object created by browser.runtime.connect is not
+// prematurely disconnected as the underlying message managers change when a
+// tab is moved between windows.
+
+function loadExtension() {
+  return ExtensionTestUtils.loadExtension({
+    manifest: {
+      "content_scripts": [{
+        "js": ["script.js"],
+        "matches": ["http://mochi.test/"],
+      }],
+    },
+    background() {
+      browser.runtime.onConnect.addListener((port) => {
+        port.onDisconnect.addListener(() => {
+          browser.test.fail("onDisconnect should not fire because the port is to be closed from this side");
+          browser.test.sendMessage("port_disconnected");
+        });
+        port.onMessage.addListener(async msg => {
+          browser.test.assertEq("connect_from_contentscript", msg, "expected message");
+          // Move a tab to a new window and back. Regression test for bugzil.la/1448674
+          let {windowId, id: tabId, index} = port.sender.tab;
+          await browser.windows.create({tabId});
+          await browser.tabs.move(tabId, {index, windowId});
+          await browser.windows.create({tabId});
+          await browser.tabs.move(tabId, {index, windowId});
+          try {
+            // When the port is unexpectedly disconnected, postMessage will throw an error.
+            port.postMessage("ping");
+          } catch (e) {
+            browser.test.fail(`Error: ${e} :: ${e.stack}`);
+            browser.test.sendMessage("port_ping_ponged_before_disconnect");
+          }
+        });
+
+        browser.runtime.onMessage.addListener((msg, sender) => {
+          browser.test.assertEq("disconnect-me", msg, "expected message");
+          port.disconnect();
+          // Now port.onDisconnect should fire in the content script.
+        });
+      });
+
+      browser.test.onMessage.addListener(msg => {
+        browser.test.assertEq("open_extension_tab", msg, "expected message");
+        browser.tabs.create({url: "tab.html"});
+      });
+    },
+
+    files: {
+      "tab.html": `
+        <!DOCTYPE html><meta charset="utf-8">
+        <script src="script.js"></script>
+      `,
+      "script.js": function() {
+        let port = browser.runtime.connect();
+        port.onMessage.addListener(msg => {
+          browser.test.assertEq("ping", msg, "expected message");
+          browser.test.sendMessage("port_ping_ponged_before_disconnect");
+          port.onDisconnect.addListener(() => {
+            browser.test.sendMessage("port_disconnected");
+          });
+          browser.runtime.sendMessage("disconnect-me");
+        });
+        port.postMessage("connect_from_contentscript");
+      },
+    },
+  });
+}
+
+add_task(async function contentscript_connect_and_move_tabs() {
+  let extension = loadExtension();
+  await extension.startup();
+  await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://mochi.test:8888/");
+  await extension.awaitMessage("port_ping_ponged_before_disconnect");
+  await extension.awaitMessage("port_disconnected");
+  // Must use gBrowser.selectedTab instead of the return value of
+  // BrowserTestUtils.openNewForegroundTab because the latter does not refer to
+  // the tab because the tab is moved between windows during the test.
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  await extension.unload();
+});
+
+add_task(async function extension_tab_connect_and_move_tabs() {
+  let extension = loadExtension();
+  await extension.startup();
+  extension.sendMessage("open_extension_tab");
+  await extension.awaitMessage("port_ping_ponged_before_disconnect");
+  await extension.awaitMessage("port_disconnected");
+  // Upon unloading the extension, the extension tab is automatically removed.
+  await extension.unload();
+});
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -227,16 +227,17 @@ ProxyMessenger = {
   },
 
   handleEvent(event) {
     if (event.type === "SwapDocShells") {
       let {messageManager} = event.originalTarget;
       if (this.ports.has(messageManager)) {
         this.ports.set(event.detail.messageManager, this.ports.get(messageManager));
         this.ports.delete(messageManager);
+        event.detail.addEventListener("SwapDocShells", this, {once: true});
       }
     }
   },
 
   async receiveMessage({target, messageName, channelId, sender, recipient, data, responseType}) {
     if (recipient.toNativeApp) {
       let {childId, toNativeApp} = recipient;
       if (messageName == "Extension:Message") {