Bug 1266134 - Fix toolbox destroy when destroy-host isn't able to reach chrome code. r=jryans draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Tue, 06 Sep 2016 08:19:35 -0700
changeset 417863 3581b3085e0544f56ee753da17e1169fbb8a24df
parent 417862 23504c315a20793e2941ce8fa984e4e25cd5cc02
child 417864 4a6ec59a319cdf794af2a9746fea0ad6c480214e
push id30520
push userbmo:poirot.alex@gmail.com
push dateTue, 27 Sep 2016 09:23:08 +0000
reviewersjryans
bugs1266134
milestone52.0a1
Bug 1266134 - Fix toolbox destroy when destroy-host isn't able to reach chrome code. r=jryans MozReview-Commit-ID: EQuDm5c7PVO
devtools/client/framework/toolbox-wrapper.js
--- a/devtools/client/framework/toolbox-wrapper.js
+++ b/devtools/client/framework/toolbox-wrapper.js
@@ -37,17 +37,17 @@ function ToolboxWrapper(target, hostType
 }
 
 ToolboxWrapper.prototype = {
   create(toolId) {
     return this.host.create()
       .then(() => {
         this.host.frame.setAttribute("aria-label", L10N.getStr("toolbox.label"));
         this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
-        this.host.frame.addEventListener("unload", this);
+        this.host.frame.addEventListener("unload", this, true);
 
         let toolbox = new Toolbox(this.target, toolId, this.host.type, this.host.frame.contentWindow, this.frameId);
 
         // Prevent reloading the toolbox when loading the tools in a tab (e.g. from about:debugging)
         if (!this.host.frame.contentWindow.location.href.startsWith("about:devtools-toolbox")) {
           this.host.frame.setAttribute("src", "about:devtools-toolbox");
         }
 
@@ -56,17 +56,21 @@ ToolboxWrapper.prototype = {
   },
 
   handleEvent(event) {
     switch(event.type) {
       case "message":
         this.onMessage(event);
         break;
       case "unload":
-        if (event.target.location.href == "about:blank") {
+        // On unload, host iframe already lost its contentWindow attribute, so
+        // we can only compare against locations. Here we filter two very
+        // different cases: preliminary about:blank document as well as iframes
+        // like tool iframes.
+        if (!event.target.location.href.startsWith("about:devtools-toolbox")) {
           break;
         }
         this.destroy();
         break;
     }
   },
 
   onMessage(event) {
@@ -158,17 +162,17 @@ ToolboxWrapper.prototype = {
       // toolbox.js to make sure focus actually happens.
       iframe.contentWindow.focus();
 
       this.destroyHost();
 
       this.host = newHost;
       this.host.setTitle(this.host.frame.contentWindow.document.title);
       this.host.frame.ownerDocument.defaultView.addEventListener("message", this);
-      this.host.frame.addEventListener("unload", this);
+      this.host.frame.addEventListener("unload", this, true);
 
       if (hostType != Toolbox.HostType.CUSTOM) {
         Services.prefs.setCharPref(LAST_HOST, hostType);
       }
 
       // Tell the toolbox the host changed
       this.postMessage({
         name: "switched-host",
@@ -179,16 +183,16 @@ ToolboxWrapper.prototype = {
 
   /**
    * Destroy the current host, and remove event listeners from its frame.
    *
    * @return {promise} to be resolved when the host is destroyed.
    */
   destroyHost() {
     this.host.frame.ownerDocument.defaultView.removeEventListener("message", this);
-    this.host.frame.removeEventListener("unload", this);
+    this.host.frame.removeEventListener("unload", this, true);
 
     this.host.off("minimized", this.onHostMinimized);
     this.host.off("maximized", this.onHostMaximized);
     return this.host.destroy();
   }
 };
 exports.ToolboxWrapper = ToolboxWrapper;