Bug 1287007 - Make window.close in extension pages async r=billm
authorRob Wu <rob@robwu.nl>
Tue, 13 Sep 2016 20:26:18 -0700
changeset 349179 437ef72ca3543d70dd1e106830ec5fee02e03d97
parent 349178 919c8b9e2ab4ef7aa51f7cf91c6c738a77e592e2
child 349180 2b6e6f6614091834bcadaca619909b2a2033fa5c
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)
reviewersbillm
bugs1287007
milestone52.0a1
Bug 1287007 - Make window.close in extension pages async r=billm Test coverage by tabs.onRemoved + window.close() in: toolkit/components/extensions/test/mochitest/test_ext_tab_teardown.html MozReview-Commit-ID: 7asg2XGrTaQ
browser/components/extensions/ext-tabs.js
toolkit/components/extensions/ExtensionChild.jsm
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -108,23 +108,16 @@ let tabListener = {
     WindowListManager.addOpenListener(this.handleWindowOpen);
     WindowListManager.addCloseListener(this.handleWindowClose);
 
     EventEmitter.decorate(this);
 
     this.initialized = true;
   },
 
-  destroy() {
-    AllWindowEvents.removeListener("TabClose", this);
-    AllWindowEvents.removeListener("TabOpen", this);
-    WindowListManager.removeOpenListener(this.handleWindowOpen);
-    WindowListManager.removeCloseListener(this.handleWindowClose);
-  },
-
   handleEvent(event) {
     switch (event.type) {
       case "TabOpen":
         if (event.detail.adoptedTab) {
           this.adoptedTabs.set(event.detail.adoptedTab, event.target);
         }
 
         // We need to delay sending this event until the next tick, since the
@@ -211,17 +204,27 @@ let tabListener = {
   emitCreated(tab) {
     this.emit("tab-created", {tab});
   },
 
   emitRemoved(tab, isWindowClosing) {
     let windowId = WindowManager.getId(tab.ownerGlobal);
     let tabId = TabManager.getId(tab);
 
-    this.emit("tab-removed", {tab, tabId, windowId, isWindowClosing});
+    // When addons run in-process, `window.close()` is synchronous. Most other
+    // addon-invoked calls are asynchronous since they go through a proxy
+    // context via the message manager. This includes event registrations such
+    // as `tabs.onRemoved.addListener`.
+    // So, even if `window.close()` were to be called (in-process) after calling
+    // `tabs.onRemoved.addListener`, then the tab would be closed before the
+    // event listener is registered. To make sure that the event listener is
+    // notified, we dispatch `tabs.onRemoved` asynchronously.
+    Services.tm.mainThread.dispatch(() => {
+      this.emit("tab-removed", {tab, tabId, windowId, isWindowClosing});
+    }, Ci.nsIThread.DISPATCH_NORMAL);
   },
 
   tabReadyInitialized: false,
   tabReadyPromises: new WeakMap(),
   initializingTabs: new WeakSet(),
 
   initTabReady() {
     if (!this.tabReadyInitialized) {
@@ -265,33 +268,38 @@ let tabListener = {
       } else {
         this.tabReadyPromises.set(tab, deferred);
       }
     }
     return deferred.promise;
   },
 };
 
+/* eslint-disable mozilla/balanced-listeners */
+extensions.on("startup", () => {
+  tabListener.init();
+});
+/* eslint-enable mozilla/balanced-listeners */
+
 extensions.registerSchemaAPI("tabs", "addon_parent", context => {
   let {extension} = context;
   let self = {
     tabs: {
       onActivated: new WindowEventManager(context, "tabs.onActivated", "TabSelect", (fire, event) => {
         let tab = event.originalTarget;
         let tabId = TabManager.getId(tab);
         let windowId = WindowManager.getId(tab.ownerGlobal);
         fire({tabId, windowId});
       }).api(),
 
       onCreated: new EventManager(context, "tabs.onCreated", fire => {
         let listener = (eventName, event) => {
           fire(TabManager.convert(extension, event.tab));
         };
 
-        tabListener.init();
         tabListener.on("tab-created", listener);
         return () => {
           tabListener.off("tab-created", listener);
         };
       }).api(),
 
       /**
        * Since multiple tabs currently can't be highlighted, onHighlighted
@@ -306,41 +314,38 @@ extensions.registerSchemaAPI("tabs", "ad
         fire({tabIds, windowId});
       }).api(),
 
       onAttached: new EventManager(context, "tabs.onAttached", fire => {
         let listener = (eventName, event) => {
           fire(event.tabId, {newWindowId: event.newWindowId, newPosition: event.newPosition});
         };
 
-        tabListener.init();
         tabListener.on("tab-attached", listener);
         return () => {
           tabListener.off("tab-attached", listener);
         };
       }).api(),
 
       onDetached: new EventManager(context, "tabs.onDetached", fire => {
         let listener = (eventName, event) => {
           fire(event.tabId, {oldWindowId: event.oldWindowId, oldPosition: event.oldPosition});
         };
 
-        tabListener.init();
         tabListener.on("tab-detached", listener);
         return () => {
           tabListener.off("tab-detached", listener);
         };
       }).api(),
 
       onRemoved: new EventManager(context, "tabs.onRemoved", fire => {
         let listener = (eventName, event) => {
           fire(event.tabId, {windowId: event.windowId, isWindowClosing: event.isWindowClosing});
         };
 
-        tabListener.init();
         tabListener.on("tab-removed", listener);
         return () => {
           tabListener.off("tab-removed", listener);
         };
       }).api(),
 
       onReplaced: ignoreEvent(context, "tabs.onReplaced"),
 
@@ -1032,17 +1037,16 @@ extensions.registerSchemaAPI("tabs", "ad
               tabId,
               oldZoomFactor,
               newZoomFactor,
               zoomSettings: self.tabs._getZoomSettings(tabId),
             });
           }
         };
 
-        tabListener.init();
         tabListener.on("tab-attached", tabCreated);
         tabListener.on("tab-created", tabCreated);
 
         AllWindowEvents.addListener("FullZoomChange", zoomListener);
         AllWindowEvents.addListener("TextZoomChange", zoomListener);
         return () => {
           tabListener.off("tab-attached", tabCreated);
           tabListener.off("tab-created", tabCreated);
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -108,17 +108,16 @@ class WannabeChildAPIManager extends Chi
   }
 
   getFallbackImplementation(namespace, name) {
     // This is gross and should be removed ASAP.
     let shouldSynchronouslyUseParentAPI = false;
     // Incompatible APIs are listed here.
     if (namespace == "runtime" && name == "connectNative" || // Returns a custom Port.
         namespace == "runtime" && name == "sendNativeMessage" || // Fix together with connectNative.
-        namespace == "tabs" && name == "onRemoved" || // bugzil.la/1300234
         namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
         namespace == "webRequest") { // Incompatible by design (synchronous).
       shouldSynchronouslyUseParentAPI = true;
     }
     if (shouldSynchronouslyUseParentAPI) {
       let proxyContext = ParentAPIManager.proxyContexts.get(this.id);
       let apiObj = findPathInObject(proxyContext.apiObj, namespace, false);
       if (apiObj && name in apiObj) {
@@ -332,17 +331,16 @@ class ContentGlobal {
         return;
       }
       this.global.removeEventListener("DOMContentLoaded", this);
       this.global.sendAsyncMessage("Extension:ExtensionViewLoaded");
     }
   }
 }
 
-
 this.ExtensionChild = {
   // Map<nsIContentFrameMessageManager, ContentGlobal>
   contentGlobals: new Map(),
 
   // Map<innerWindowId, ExtensionContext>
   extensionContexts: new Map(),
 
   initOnce() {