From a4dced69117a4fb9f0cedc782915fe3b34a9b0be Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Fri, 13 Nov 2009 16:12:50 -0500 Subject: [PATCH] Automatically track and remove long-lived event listeners. --- common/content/buffer.js | 63 ++++++++++++++++++--------------------- common/content/events.js | 45 +++++++++++++++++----------- common/content/marks.js | 9 +++--- common/content/modules.js | 3 ++ common/content/tabs.js | 41 ++++++++++++------------- 5 files changed, 84 insertions(+), 77 deletions(-) diff --git a/common/content/buffer.js b/common/content/buffer.js index 1f1a6207..2e8e0731 100644 --- a/common/content/buffer.js +++ b/common/content/buffer.js @@ -19,11 +19,9 @@ const Point = new Struct("x", "y"); * @instance buffer */ const Buffer = Module("buffer", { - requires: ["autocommands", "config"], + requires: ["config"], init: function () { - const self = this; - this.pageInfo = {}; this.addPageInfoSection("f", "Feeds", function (verbose) { @@ -144,32 +142,6 @@ const Buffer = Module("buffer", { return Array.map(metaNodes, function (node) [(node.name || node.httpEquiv), template.highlightURL(node.content)]) .sort(function (a, b) util.compareIgnoreCase(a[0], b[0])); }); - - window.XULBrowserWindow = this.progressListener; - window.QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIWebNavigation) - .QueryInterface(Ci.nsIDocShellTreeItem) - .treeOwner - .QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIXULWindow) - .XULBrowserWindow = this.progressListener; - - try { - getBrowser().addProgressListener(this.progressListener, Ci.nsIWebProgress.NOTIFY_ALL); - } - catch (e) {} // Why? --djk - - let appContent = document.getElementById("appcontent"); - appContent.addEventListener("DOMContentLoaded", this.closure.onDOMContentLoaded, true); - // this adds an event which is is called on each page load, even if the - // page is loaded in a background tab - appContent.addEventListener("load", this.closure.onPageLoad, true); - // called when the active document is scrolled - this._updateBufferPosition = function _updateBufferPosition() { - statusline.updateBufferPosition(); - modes.show(); - }; - appContent.addEventListener("scroll", self._updateBufferPosition, false); }, destroy: function () { @@ -177,11 +149,6 @@ const Buffer = Module("buffer", { getBrowser().removeProgressListener(this.progressListener); } catch (e) {} // Why? --djk - - let appContent = document.getElementById("appcontent"); - appContent.removeEventListener("DOMContentLoaded", this.closure.onDOMContentLoaded, true); - appContent.removeEventListener("load", this.closure.onPageLoad, true); - appContent.removeEventListener("scroll", buffer._updateBufferPosition, false); }, _triggerLoadAutocmd: function _triggerLoadAutocmd(name, doc) { @@ -198,6 +165,12 @@ const Buffer = Module("buffer", { autocommands.trigger(name, args); }, + // called when the active document is scrolled + _updateBufferPosition: function _updateBufferPosition() { + statusline.updateBufferPosition(); + modes.show(); + }, + onDOMContentLoaded: function onDOMContentLoaded(event) { let doc = event.originalTarget; if (doc instanceof HTMLDocument && !doc.defaultView.frameElement) @@ -205,6 +178,8 @@ const Buffer = Module("buffer", { }, // TODO: see what can be moved to onDOMContentLoaded() + // event listener which is is called on each page load, even if the + // page is loaded in a background tab onPageLoad: function onPageLoad(event) { if (event.originalTarget instanceof HTMLDocument) { let doc = event.originalTarget; @@ -1335,6 +1310,26 @@ const Buffer = Module("buffer", { }); }; }, + events: function () { + window.XULBrowserWindow = this.progressListener; + window.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIWebNavigation) + .QueryInterface(Ci.nsIDocShellTreeItem) + .treeOwner + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIXULWindow) + .XULBrowserWindow = this.progressListener; + + try { + getBrowser().addProgressListener(this.progressListener, Ci.nsIWebProgress.NOTIFY_ALL); + } + catch (e) {} // Why? --djk + + let appContent = document.getElementById("appcontent"); + events.addSessionListener(appContent, "DOMContentLoaded", this.closure.onDOMContentLoaded, true); + events.addSessionListener(appContent, "load", this.closure.onPageLoad, true); + events.addSessionListener(appContent, "scroll", this.closure._updateBufferPosition, false); + }, mappings: function () { var myModes = config.browserModes; diff --git a/common/content/events.js b/common/content/events.js index 8280c182..fdad17bf 100644 --- a/common/content/events.js +++ b/common/content/events.js @@ -22,6 +22,8 @@ const Events = Module("events", { this._currentMacro = ""; this._lastMacro = ""; + this.sessionListeners = []; + this._macros = storage.newMap("macros", true, { privateData: true }); // NOTE: the order of ["Esc", "Escape"] or ["Escape", "Esc"] @@ -119,30 +121,39 @@ const Events = Module("events", { this._wrappedOnKeyPress = wrapListener("onKeyPress"); this._wrappedOnKeyUpOrDown = wrapListener("onKeyUpOrDown"); - window.addEventListener("keypress", this.closure._wrappedOnKeyPress, true); - window.addEventListener("keydown", this.closure._wrappedOnKeyUpOrDown, true); - window.addEventListener("keyup", this.closure._wrappedOnKeyUpOrDown, true); + this.addSessionListener(window, "keypress", this.closure._wrappedOnKeyPress, true); + this.addSessionListener(window, "keydown", this.closure._wrappedOnKeyUpOrDown, true); + this.addSessionListener(window, "keyup", this.closure._wrappedOnKeyUpOrDown, true); this._activeMenubar = false; - window.addEventListener("popupshown", this.closure.onPopupShown, true); - window.addEventListener("popuphidden", this.closure.onPopupHidden, true); - window.addEventListener("DOMMenuBarActive", this.closure.onDOMMenuBarActive, true); - window.addEventListener("DOMMenuBarInactive", this.closure.onDOMMenuBarInactive, true); - window.addEventListener("resize", this.closure.onResize, true); + this.addSessionListener(window, "popupshown", this.closure.onPopupShown, true); + this.addSessionListener(window, "popuphidden", this.closure.onPopupHidden, true); + this.addSessionListener(window, "DOMMenuBarActive", this.closure.onDOMMenuBarActive, true); + this.addSessionListener(window, "DOMMenuBarInactive", this.closure.onDOMMenuBarInactive, true); + this.addSessionListener(window, "resize", this.closure.onResize, true); }, destroy: function () { - liberator.dump("TODO: remove all event listeners"); + liberator.dump("Removing all event listeners"); + for (let args in values(this.sessionListeners)) + args[0].removeEventListener.apply(args[0], args.slice(1)); + }, - window.removeEventListener("popupshown", this.closure.onPopupShown, true); - window.removeEventListener("popuphidden", this.closure.onPopupHidden, true); - window.removeEventListener("DOMMenuBarActive", this.closure.onDOMMenuBarActive, true); - window.removeEventListener("DOMMenuBarInactive", this.closure.onDOMMenuBarInactive, true); - window.removeEventListener("resize", this.closure.onResize, true); - window.removeEventListener("keypress", this.closure._wrappedOnKeyPress, true); - window.removeEventListener("keydown", this.closure._wrappedOnKeyUpOrDown, true); - window.removeEventListener("keyup", this.closure._wrappedOnKeyUpOrDown, true); + /** + * Adds an event listener for this session and removes it on + * liberator shutdown. + * + * @param {Element} target The element on which to listen. + * @param {string} event The event to listen for. + * @param {function} callback The function to call when the event is received. + * @param {boolean} capture When true, listen during the capture + * phase, otherwise during the bubbling phase. + */ + addSessionListener: function (target, event, callback, capture) { + let args = Array.slice(arguments, 0); + target.addEventListener.apply(target, args.slice(1)); + this.sessionListeners.push(args); }, /** diff --git a/common/content/marks.js b/common/content/marks.js index 1983aa1f..a4e71969 100644 --- a/common/content/marks.js +++ b/common/content/marks.js @@ -15,10 +15,6 @@ const Marks = Module("marks", { this._urlMarks = storage.newMap("url-marks", { store: true, privateData: true }); this._pendingJumps = []; - - var appContent = document.getElementById("appcontent"); - if (appContent) - appContent.addEventListener("load", this.closure._onPageLoad, true); }, /** @@ -240,6 +236,11 @@ const Marks = Module("marks", { isLocalMark: function isLocalMark(mark) /^['`a-z]$/.test(mark), isURLMark: function isURLMark(mark) /^[A-Z0-9]$/.test(mark), }, { + events: function () { + let appContent = document.getElementById("appcontent"); + if (appContent) + events.addSessionListener(appContent, "load", this.closure._onPageLoad, true); + }, mappings: function () { var myModes = config.browserModes; diff --git a/common/content/modules.js b/common/content/modules.js index d054d7af..5a5513da 100644 --- a/common/content/modules.js +++ b/common/content/modules.js @@ -67,6 +67,8 @@ Module.list = []; Module.constructors = {}; window.addEventListener("load", function () { + window.removeEventListener("load", arguments.callee, false); + function dump(str) window.dump(String.replace(str, /\n?$/, "\n").replace(/^/m, Config.prototype.name.toLowerCase() + ": ")); const start = Date.now(); const deferredInit = { load: [] }; @@ -124,6 +126,7 @@ window.addEventListener("load", function () { }, false); window.addEventListener("unload", function () { + window.removeEventListener("unload", arguments.callee, false); for (let [, mod] in iter(modules)) if (mod instanceof ModuleBase && "destroy" in mod) mod.destroy(); diff --git a/common/content/tabs.js b/common/content/tabs.js index c3288e0c..b8646373 100644 --- a/common/content/tabs.js +++ b/common/content/tabs.js @@ -26,34 +26,24 @@ const Tabs = Module("tabs", { this._lastBufferSwitchArgs = ""; this._lastBufferSwitchSpecial = true; - let tabContainer = this.getBrowser().mTabContainer; - this._updateTabCount = function () { statusline.updateTabCount(true); }; - ["TabMove", "TabOpen", "TabClose"].forEach(function (event) { - tabContainer.addEventListener(event, this._updateTabCount, false); - }, this); - this._onTabSelect = function () { - // TODO: is all of that necessary? - // I vote no. --Kris - modes.reset(); - statusline.updateTabCount(true); - this.updateSelectionHistory(); - if (options["focuscontent"]) - setTimeout(function () { liberator.focusContent(true); }, 10); // just make sure, that no widget has focus - }; - tabContainer.addEventListener("TabSelect", this.closure._onTabSelect, false); - // hide tabs initially to prevent flickering when 'stal' would hide them // on startup if (config.hasTabbrowser) this.getBrowser().mTabContainer.collapsed = true; // FIXME: see 'stal' comment }, - destroy: function () { - let tabContainer = this.getBrowser().mTabContainer; - ["TabMove", "TabOpen", "TabClose"].forEach(function (event) { - tabContainer.removeEventListener(event, this._updateTabCount, false); - }, this); - tabContainer.removeEventListener("TabSelect", this.closure._onTabSelect, false); + _updateTabCount: function () { + statusline.updateTabCount(true); + }, + + _onTabSelect: function () { + // TODO: is all of that necessary? + // I vote no. --Kris + modes.reset(); + statusline.updateTabCount(true); + this.updateSelectionHistory(); + if (options["focuscontent"]) + setTimeout(function () { liberator.focusContent(true); }, 10); // just make sure, that no widget has focus }, /** @@ -942,6 +932,13 @@ const Tabs = Module("tabs", { "Open tabs", completion.buffer); }, + events: function () { + let tabContainer = this.getBrowser().mTabContainer; + ["TabMove", "TabOpen", "TabClose"].forEach(function (event) { + events.addSessionListener(tabContainer, event, this.closure._updateTabCount, false); + }, this); + events.addSessionListener(tabContainer, "TabSelect", this.closure._onTabSelect, false); + }, mappings: function () { mappings.add([modes.NORMAL], ["g0", "g^"], "Go to the first tab",