We have temporarily moved it below "if (!this._tools)" but we should fix this properly. Original Github PR: https://github.com/joewalker/devtools-window/pull/257

We should find out why the destructor is called twice (or before initialization). We could also trap exceptions if necessary.

I think we should tackle this alongside memory leak issues. I favour getting rid of all the "delete this.****" statements in the destroy functions, but we need to make sure we clear up after ourselves in order to do that.

The approach that I've taken here is: sometimes preventing double-delete is hard. A good clean-up function can clean-up without crippling itself, and there's no point in gratuitously causing problems by deleting things you don't need to delete. The change to Toolbox isn't directly connected, but it code that seems wrong to me. Comment added for feedback. Depending on what we think I could remove this section before commit.

Comment on attachment 699645 [details] [diff] [review] v1 >+ /* >+ // this._currentToolId is set at the end of selectTool so unless >+ // we're going to be calling open after selectTool (???) then it's >+ // always going to be undefined, so we can just remove this code >+ // especially given the new raise code ??? That was no-op? I did that :) >@@ -217,8 +217,13 @@ DevTools.prototype = { > destroy: function() { > Services.obs.removeObserver(this.destroy, "quit-application"); > >- delete this._trackedBrowserWindows; >- delete this._toolboxes; >+ for (let [key, tool] of this._tools) { >+ this.unregisterTool(key); Will this trigger the "tool-unregistered" event, and do all the related DOM modifications? We want to avoid that (performance).

https://tbpl.mozilla.org/?tree=Fx-Team&rev=fd6d84e5bf49 https://hg.mozilla.org/integration/fx-team/rev/346ee288166b