Bug 1452514: Correctly handle a browser window closing before its popups do. r?mixedpuppy draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 19 Jul 2018 19:03:20 -0700
changeset 820668 3d02bc3fd105330109fa5077b486e89beda2a849
parent 820661 0fa34e8fd83d0dca0bc3a505b212854ab8735aad
push id116893
push usermaglione.k@gmail.com
push dateFri, 20 Jul 2018 02:07:06 +0000
reviewersmixedpuppy
bugs1452514
milestone63.0a1
Bug 1452514: Correctly handle a browser window closing before its popups do. r?mixedpuppy MozReview-Commit-ID: DOqfniMPf87
browser/components/extensions/ExtensionPopups.jsm
--- a/browser/components/extensions/ExtensionPopups.jsm
+++ b/browser/components/extensions/ExtensionPopups.jsm
@@ -73,16 +73,17 @@ class BasePopup {
     this.blockParser = blockParser;
 
     extension.callOnClose(this);
 
     this.contentReady = new Promise(resolve => {
       this._resolveContentReady = resolve;
     });
 
+    this.window.addEventListener("unload", this);
     this.viewNode.addEventListener(this.DESTROY_EVENT, this);
     this.panel.addEventListener("popuppositioned", this, {once: true, capture: true});
 
     this.browser = null;
     this.browserLoaded = new Promise((resolve, reject) => {
       this.browserLoadedDeferred = {resolve, reject};
     });
     this.browserReady = this.createBrowser(viewNode, popupURL);
@@ -96,16 +97,18 @@ class BasePopup {
 
   close() {
     this.closePopup();
   }
 
   destroy() {
     this.extension.forgetOnClose(this);
 
+    this.window.removeEventListener("unload", this);
+
     this.destroyed = true;
     this.browserLoadedDeferred.reject(new Error("Popup destroyed"));
     // Ignore unhandled rejections if the "attach" method is not called.
     this.browserLoaded.catch(() => {});
 
     BasePopup.instances.get(this.window).delete(this.extension);
 
     return this.browserReady.then(() => {
@@ -207,16 +210,17 @@ class BasePopup {
       case "Extension:DOMWindowClose":
         this.closePopup();
         break;
     }
   }
 
   handleEvent(event) {
     switch (event.type) {
+      case "unload":
       case this.DESTROY_EVENT:
         if (!this.destroyed) {
           this.destroy();
         }
         break;
       case "popuppositioned":
         if (!this.destroyed) {
           this.browserLoaded.then(() => {
@@ -476,16 +480,19 @@ class ViewPopup extends BasePopup {
    * @param {Element} viewNode
    *        The node to attach the browser to.
    * @returns {Promise<boolean>}
    *        Resolves when the browser is ready. Resolves to `false` if the
    *        browser was destroyed before it was fully loaded, and the popup
    *        should be closed, or `true` otherwise.
    */
   async attach(viewNode) {
+    this.viewNode.removeEventListener(this.DESTROY_EVENT, this);
+    this.panel.removeEventListener("popuppositioned", this, {once: true, capture: true});
+
     this.viewNode = viewNode;
     this.viewNode.addEventListener(this.DESTROY_EVENT, this);
     this.viewNode.setAttribute("closemenu", "none");
 
     this.panel.addEventListener("popuppositioned", this, {once: true, capture: true});
     if (this.extension.remote) {
       this.panel.setAttribute("remote", "true");
     }