From a07a4c309deb95919663e2189911fb0163684cea Mon Sep 17 00:00:00 2001 From: crschmidt Date: Wed, 28 Mar 2007 21:41:08 +0000 Subject: [PATCH] Cleanup to Events.js. By Erik, in #568. * code was only ever removing the first event listener from each element on unload cache (for loop instead of while) * code was frequently not removing observers because their associated elements have already been removed These two are fixed by this patch. Combined with the previous commits to setting events, this should fix the memory leaks demonstrated by the 'drip' tool for IE. git-svn-id: http://svn.openlayers.org/trunk/openlayers@2913 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf --- lib/OpenLayers/Events.js | 143 +++++++++++++++++++++++++-------------- 1 file changed, 91 insertions(+), 52 deletions(-) diff --git a/lib/OpenLayers/Events.js b/lib/OpenLayers/Events.js index 90426416f4..2b3c4a4884 100644 --- a/lib/OpenLayers/Events.js +++ b/lib/OpenLayers/Events.js @@ -69,7 +69,12 @@ OpenLayers.Event = { this.observers[element.id] = new Array(); } //add a new observer to this element's list - this.observers[element.id].push([name, observer, useCapture]); + this.observers[element.id].push({ + 'element': element, + 'name': name, + 'observer': observer, + 'useCapture': useCapture + }); //add the actual browser event listener if (element.addEventListener) { @@ -79,19 +84,40 @@ OpenLayers.Event = { } }, - unloadCache: function() { - if (!OpenLayers.Event.observers) return; - for (var elementId in OpenLayers.Event.observers) { + /** Given the id of an element to stop observing, cycle through the + * element's cached observers, calling stopObserving on each one, + * skipping those entries which can no longer be removed. + * + * @param {String} elementId + */ + stopObservingElement: function(elementId) { var elementObservers = OpenLayers.Event.observers[elementId]; if (elementObservers) { - for (var i = 0; i < elementObservers.length; i++) { - var args = new Array(elementId).concat(elementObservers[i]); - OpenLayers.Event.stopObserving.apply(this, args); + var i=0; + while(i < elementObservers.length) { + var entry = elementObservers[0]; + var args = new Array(entry.element, + entry.name, + entry.observer, + entry.useCapture); + var removed = OpenLayers.Event.stopObserving.apply(this, args); + if (!removed) { + i++; + } } } - } - OpenLayers.Event.observers = false; - }, + }, + + /** Cycle through all the element entries in the events cache and call + * stopObservingElement on each. + */ + unloadCache: function() { + if (!OpenLayers.Event.observers) return; + for (var elementId in OpenLayers.Event.observers) { + OpenLayers.Event.stopObservingElement.apply(this, [elementId]); + } + OpenLayers.Event.observers = false; + }, observe: function(elementParam, name, observer, useCapture) { var element = OpenLayers.Util.getElement(elementParam); @@ -105,54 +131,67 @@ OpenLayers.Event = { this._observeAndCache(element, name, observer, useCapture); }, - stopObserving: function(elementParam, name, observer, useCapture) { - var element = OpenLayers.Util.getElement(elementParam); - if (!element) return; - - useCapture = useCapture || false; - - if (name == 'keypress' && - (navigator.appVersion.match(/Konqueror|Safari|KHTML/) - || element.detachEvent)) - name = 'keydown'; - - // find element's entry in this.observers cache and remove it - var elementObservers = OpenLayers.Event.observers[element.id]; - if (elementObservers) { - var entry = [name, observer, useCapture]; - - // find the specific event type in the element's list - for (var i = 0; i < elementObservers.length; i++) { - var cacheEntry = elementObservers[i]; - - //compare all 3 elements of entry with observer - var sameEntry = true; - for (var j = 0; j < entry.length; j++) { - if (entry[j] != cacheEntry[j]) { - sameEntry = false; - break; + /** + * @param {DOMElement || String} elementParam + * @param {String} name + * @param {function} observer + * @param {Boolean} useCapture + * + * @returns Whether or not the event observer was removed + * @type Boolean + */ + stopObserving: function(elementParam, name, observer, useCapture) { + var foundEntry = false; + var element = OpenLayers.Util.getElement(elementParam); + if (element) { + + useCapture = useCapture || false; + + if (name == 'keypress') { + if ( navigator.appVersion.match(/Konqueror|Safari|KHTML/) || + element.detachEvent) { + name = 'keydown'; } } - - //if we've found it, remove it from the observers array - if (sameEntry) { - elementObservers.splice(i--, 1); - if (elementObservers.length == 0) { - OpenLayers.Event.observers[element.id] = null; + + // find element's entry in this.observers cache and remove it + var elementObservers = OpenLayers.Event.observers[element.id]; + if (elementObservers) { + + // find the specific event type in the element's list + var i=0; + while(!foundEntry && i < elementObservers.length) { + var cacheEntry = elementObservers[i]; + + if ((cacheEntry.name == name) && + (cacheEntry.observer == observer) && + (cacheEntry.useCapture == useCapture)) { + + elementObservers.splice(i, 1); + if (elementObservers.length == 0) { + OpenLayers.Event.observers[element.id] = null; + } + break; + foundEntry = true; + } + i++; } - break; + } + + //actually remove the event listener from browser + if (element.removeEventListener) { + element.removeEventListener(name, observer, useCapture); + } else if (element && element.detachEvent) { + element.detachEvent('on' + name, observer); } } - } - - //actually remove the event listener from browser - if (element.removeEventListener) { - element.removeEventListener(name, observer, useCapture); - } else if (element && element.detachEvent) { - element.detachEvent('on' + name, observer); - } - } + return foundEntry; + }, + + /** @final @type String */ + CLASS_NAME: "OpenLayers.Event" }; + /* prevent memory leaks in IE */ OpenLayers.Event.observe(window, 'unload', OpenLayers.Event.unloadCache, false);