Refactor event manager to remove memory leaks

src/ol/control/control.js

@@ -60,7 +60,7 @@ ol.control.Control = function(options) {
 
   /**
    * @protected
-   * @type {!Array.<?number>}
+   * @type {!Array.<ol.events.Key>}
    */
   this.listenerKeys = [];
 
@@ -108,7 +108,7 @@ ol.control.Control.prototype.setMap = function(map) {
     goog.dom.removeNode(this.element);
   }
   if (this.listenerKeys.length > 0) {
-    this.listenerKeys.forEach(/** @type {Function} */ (ol.events.unlistenByKey));
+    ol.events.unlistenByKey(this.listenerKeys);
     this.listenerKeys.length = 0;
   }
   this.map_ = map;

src/ol/events.js

@@ -2,6 +2,9 @@ goog.provide('ol.events');
 goog.provide('ol.events.EventType');
 goog.provide('ol.events.KeyCode');
 
+goog.require('goog.asserts');
+goog.require('goog.object');
+
 
 /**
  * @enum {string}
@@ -50,25 +53,36 @@ ol.events.KeyCode = {
 };
 
 
+// Event manager inspired by
+// https://google.github.io/closure-library/api/source/closure/goog/events/events.js.src.html
+
+
+/**
+ * Property name on an event target for the listener map associated with the
+ * event target.
+ * @const {string}
+ * @private
+ */
+ol.events.LISTENER_MAP_PROP_ = 'olm_' + ((Math.random() * 1e6) | 0);
+
+
+/**
+ * @typedef {EventTarget|ol.events.EventTarget|
+ *     {addEventListener: function(string, Function, boolean=),
+ *     removeEventListener: function(string, Function, boolean=)}}
+ */
+ol.events.EventTargetLike;
+
+
 /**
  * Key to use with {@link ol.Observable#unByKey}.
  *
- * @typedef {string|Array.<string>}
+ * @typedef {ol.events.ListenerObjType|Array.<ol.events.ListenerObjType>}
  * @api
  */
 ol.events.Key;
 
 
-/**
- * @typedef {{listener: ol.events.ListenerFunctionType,
- *     target: (EventTarget|ol.events.EventTarget),
- *     thisArg: (Object|undefined),
- *     type: (ol.events.EventType|Array.<ol.events.EventType>),
- *     useCapture: boolean}}
- */
-ol.events.ListenerData;
-
-
 /**
  * Listener function. This function is called with an event object as argument.
  * When the function returns `false`, event propagation will stop.
@@ -80,45 +94,68 @@ ol.events.ListenerFunctionType;
 
 
 /**
- * @private
+ * @typedef {{bindTo: (Object|undefined),
- * @type {Object.<ol.events.Key, ol.events.ListenerData>}
+ *     boundListener: (ol.events.ListenerFunctionType|undefined),
+ *     callOnce: boolean,
+ *     listener: ol.events.ListenerFunctionType,
+ *     target: (EventTarget|ol.events.EventTarget),
+ *     type: (ol.events.EventType|string),
+ *     useCapture: boolean}}
  */
-ol.events.listenersByKey_ = {};
+ol.events.ListenerObjType;
 
 
 /**
- * @private
- * @param {EventTarget|ol.events.EventTarget|
- *     {removeEventListener: function(string, Function, boolean=)}} target Event target.
- * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
- *     Event type.
  * @param {ol.events.ListenerFunctionType} listener Listener.
- * @param {boolean=} opt_useCapture Use capture. For listeners on an
+ * @param {ol.events.ListenerObjType} listenerObj Listener object.
- *     {@link ol.events.EventTarget}, `true` simply means that the listener will
+ * @return {ol.events.ListenerFunctionType} Bound listener.
- *     be called before already registered listeners. Default is false.
- * @param {Object=} opt_this Object referenced by the `this` keyword in the
- *     listener. Default is the `target`.
- * @return {ol.events.ListenerFunctionType} Listener that unregisters itself.
  */
-ol.events.createListenOnce_ = function(target, type, listener, opt_useCapture, opt_this) {
+ol.events.bindListener_ = function(listener, listenerObj) {
-  var count = Array.isArray(type) ? type.length : 1;
+  return function(evt) {
-  var key = ol.events.getKey.apply(undefined, arguments);
+    var rv = listenerObj.listener.call(listenerObj.bindTo, evt);
-  return function listenOnce(evt) {
+    if (listenerObj.callOnce) {
-    listener.call(opt_this || this, evt);
+      ol.events.unlistenByKey(listenerObj);
-    target.removeEventListener(evt.type, listenOnce, !!opt_useCapture);
+    }
-    --count;
+    return rv;
-    if (count === 0) {
+  }
-      delete ol.events.listenersByKey_[key];
+};
+
+
+/**
+ * Finds the matching {@link ol.events.ListenerObjType} in the given listener
+ * array.
+ * @param {!Array<!ol.events.ListenerObjType>} listenerArray Array of listeners.
+ * @param {!Function} listener The listener function.
+ * @param {boolean} useCapture The capture flag for the listener.
+ * @param {Object=} opt_this The `this` value inside the listener.
+ * @param {boolean=} opt_remove Remove the found listener from the array.
+ * @return {ol.events.ListenerObjType|undefined} The matching listener.
+ * @private
+ */
+ol.events.findListener_ = function(
+    listenerArray, listener, useCapture, opt_this, opt_remove) {
+  var listenerObj;
+  for (var i = 0, ii = listenerArray.length; i < ii; ++i) {
+    listenerObj = listenerArray[i];
+    if (listenerObj.listener === listener &&
+        listenerObj.useCapture == useCapture &&
+        listenerObj.bindTo === opt_this) {
+      if (opt_remove) {
+        listenerArray.splice(i, 1);
+      }
+      return listenerObj;
     }
   }
+  return undefined;
 };
 
 
 /**
  * @param {EventTarget|ol.events.EventTarget} target Event target.
  * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
- *     Event type.
+ *     Event t@param {Event|ol.events.Event} event Event to dispatch on the
- * @param {Event|ol.events.Event} event Event to dispatch on the `target`.
+ *     `target`.
+ * @param {Event|ol.events.Event} event Event.
  */
 ol.events.fireListeners = function(target, type, event) {
   event.type = type;
@@ -127,99 +164,19 @@ ol.events.fireListeners = function(target, type, event) {
 
 
 /**
- * @param {EventTarget|ol.events.EventTarget} target Event target.
+ * @param {ol.events.EventTargetLike} target Target.
- * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
- *     Event type.
- * @param {ol.events.ListenerFunctionType} listener Listener.
- * @param {boolean=} opt_useCapture Use capture. For listeners on an
- *     {@link ol.events.EventTarget}, `true` simply means that the listener will
- *     be called before already registered listeners. Default is false.
- * @param {Object=} opt_this Object referenced by the `this` keyword in the
- *     listener. Default is the `target`.
- * @return {!ol.events.Key} Key for unlistenByKey.
- */
-ol.events.getKey = function(target, type, listener, opt_useCapture, opt_this) {
-  return [
-    goog.getUid(target), type.toString(),goog.getUid(listener),
-    Number(!!opt_useCapture), (opt_this ? goog.getUid(opt_this) : '')
-  ].toString();
-};
-
-
-/**
- * @param {ol.events.EventTarget} target Target.
  * @param {ol.events.EventType|string} type Type.
- * @return {Array.<ol.events.ListenerFunctionType>} Listeners.
+ * @return {Array.<ol.events.ListenerObjType>|undefined} Listeners.
  */
 ol.events.getListeners = function(target, type) {
-  return target.getListeners(type);
+  var listenerMap = target[ol.events.LISTENER_MAP_PROP_];
+  return listenerMap ? listenerMap[type] : undefined;
 };
 
 
 /**
  * @param {EventTarget|ol.events.EventTarget|
- *     {addEventListener: function(string, Function, boolean=)}} target Event target.
+ *     {removeEventListener: function(string, Function, boolean=)}} target
- * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
- *     Event type.
- * @param {ol.events.ListenerFunctionType} listener Listener.
- * @param {boolean=} opt_useCapture Use capture. For listeners on an
- *     {@link ol.events.EventTarget}, `true` simply means that the listener will
- *     be called before already registered listeners. Default is false.
- * @param {Object=} opt_this Object referenced by the `this` keyword in the
- *     listener. Default is the `target`.
- * @return {ol.events.Key} Key for unlistenByKey.
- */
-ol.events.listen = function(target, type, listener, opt_useCapture, opt_this) {
-  var targetListener = opt_this ? listener.bind(opt_this) : listener;
-  //TODO remove:
-  targetListener.handler = opt_this;
-  var types = Array.isArray(type) ? type : [type];
-  var key = ol.events.getKey.apply(undefined, arguments);
-  if (!ol.events.listenersByKey_[key]) {
-    for (var i = 0, ii = types.length; i < ii; ++i) {
-      target.addEventListener(types[i], targetListener, !!opt_useCapture);
-    }
-    ol.events.listenersByKey_[key] = /** @type {ol.events.ListenerData} */ ({
-      listener: targetListener,
-      target: target,
-      thisArg: opt_this,
-      type: type,
-      useCapture: opt_useCapture
-    });
-  }
-  return key;
-};
-
-
-/**
- * @param {EventTarget|ol.events.EventTarget|
- *     {addEventListener: function(string, Function, boolean=),
- *     removeEventListener: function(string, Function, boolean=)}} target Event target.
- * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
- *     Event type.
- * @param {ol.events.ListenerFunctionType} listener Listener.
- * @param {boolean=} opt_useCapture Use capture. For listeners on an
- *     {@link ol.events.EventTarget}, `true` simply means that the listener will
- *     be called before already registered listeners. Default is false.
- * @param {Object=} opt_this Object referenced by the `this` keyword in the
- *     listener. Default is the `target`.
- * @return {ol.events.Key} Key for unlistenByKey.
- */
-ol.events.listenOnce = function(target, type, listener, opt_useCapture, opt_this) {
-  var key = ol.events.getKey.apply(undefined, arguments);
-  if (!ol.events.listenersByKey_[key]) {
-    var targetListener = ol.events.createListenOnce_(target, type, listener,
-        opt_useCapture, opt_this);
-    var onceKey = ol.events.listen(target, type, targetListener, opt_useCapture);
-    ol.events.listenersByKey_[key] = ol.events.listenersByKey_[onceKey];
-    delete ol.events.listenersByKey_[onceKey];
-  }
-  return key;
-};
-
-
-/**
- * @param {EventTarget|ol.events.EventTarget|{removeEventListener: function(string, Function, boolean=)}} target
  *     Event target.
  * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
  *     Event type.
@@ -229,12 +186,101 @@ ol.events.listenOnce = function(target, type, listener, opt_useCapture, opt_this
  *     be called before already registered listeners. Default is false.
  * @param {Object=} opt_this Object referenced by the `this` keyword in the
  *     listener. Default is the `target`.
- * @return {ol.events.Key} Key that the listener was referenced with.
+ * @param {boolean=} opt_once If true, add the listener as one-off listener.
+ * @return {ol.events.Key} Unique key for the listener.
  */
-ol.events.unlisten = function(target, type, listener, opt_useCapture, opt_this) {
+ol.events.listen = function(
-  var key = ol.events.getKey.apply(undefined, arguments);
+    target, type, listener, opt_useCapture, opt_this, opt_once) {
-  ol.events.unlistenByKey(key);
+  if (Array.isArray(type)) {
-  return key;
+    var keys = [];
+    type.forEach(function(t) {
+      keys.push(ol.events.listen(target, t, listener, opt_useCapture, opt_this,
+          opt_once));
+    });
+    return keys;
+  }
+  goog.asserts.assertString(type);
+  var useCapture = !!opt_useCapture;
+  var listenerMap = target[ol.events.LISTENER_MAP_PROP_];
+  if (!listenerMap) {
+    target[ol.events.LISTENER_MAP_PROP_] = listenerMap = {};
+  }
+  var listenerArray = listenerMap[type];
+  if (!listenerArray) {
+    listenerArray = listenerMap[type] = [];
+  }
+  var listenerObj = ol.events.findListener_(listenerArray, listener, useCapture,
+      opt_this);
+  if (listenerObj) {
+    if (!opt_once) {
+      // Turn one-off listener into a permanent one.
+      listenerObj.callOnce = false;
+    }
+  } else {
+    listenerObj = /** @type {ol.events.ListenerObjType} */ ({
+      bindTo: opt_this,
+      callOnce: !!opt_once,
+      listener: listener,
+      target: target,
+      type: type,
+      useCapture: useCapture
+    });
+    listenerObj.boundListener = ol.events.bindListener_(listener, listenerObj);
+    target.addEventListener(type, listenerObj.boundListener, useCapture);
+    listenerArray.push(listenerObj);
+  }
+
+  return listenerObj;
+};
+
+
+/**
+ * @param {ol.events.EventTargetLike} target Event target.
+ * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
+ *     Event type.
+ * @param {ol.events.ListenerFunctionType} listener Listener.
+ * @param {boolean=} opt_useCapture Use capture. For listeners on an
+ *     {@link ol.events.EventTarget}, `true` simply means that the listener will
+ *     be called before already registered listeners. Default is false.
+ * @param {Object=} opt_this Object referenced by the `this` keyword in the
+ *     listener. Default is the `target`.
+ * @return {ol.events.Key} Key for unlistenByKey.
+ */
+ol.events.listenOnce = function(
+    target, type, listener, opt_useCapture, opt_this) {
+  return ol.events.listen(target, type, listener, opt_useCapture, opt_this,
+      true);
+};
+
+
+/**
+ * @param {ol.events.EventTargetLike} target Event target.
+ * @param {ol.events.EventType|string|Array.<(ol.events.EventType|string)>} type
+ *     Event type.
+ * @param {ol.events.ListenerFunctionType} listener Listener.
+ * @param {boolean=} opt_useCapture Use capture. For listeners on an
+ *     {@link ol.events.EventTarget}, `true` simply means that the listener will
+ *     be called before already registered listeners. Default is false.
+ * @param {Object=} opt_this Object referenced by the `this` keyword in the
+ *     listener. Default is the `target`.
+ */
+ol.events.unlisten = function(
+    target, type, listener, opt_useCapture, opt_this) {
+  if (Array.isArray(type)) {
+    type.forEach(function(t) {
+      ol.events.unlisten(target, t, listener, opt_useCapture, opt_this);
+    });
+    return;
+  }
+
+  var listenerArray = ol.events.getListeners(target, type);
+  if (listenerArray) {
+    var listenerObj = ol.events.findListener_(listenerArray, listener,
+        !!opt_useCapture, opt_this);
+    if (listenerObj) {
+      ol.events.unlistenByKey(listenerObj);
+    }
+  }
 };
 
 
@@ -242,15 +288,26 @@ ol.events.unlisten = function(target, type, listener, opt_useCapture, opt_this)
  * @param {ol.events.Key} key Key or keys.
  */
 ol.events.unlistenByKey = function(key) {
-  var listenerData = ol.events.listenersByKey_[key];
+  if (Array.isArray(key)) {
-  if (listenerData) {
+    key.forEach(ol.events.unlistenByKey);
-    var type = listenerData.type;
+    return;
-    var types = Array.isArray(type) ? type : [type];
+  }
-    for (var i = 0, ii = types.length; i < ii; ++i) {
+
-      listenerData.target.removeEventListener(types[i],
+  if (key && key.target) {
-          listenerData.listener, !!listenerData.useCapture);
+    key.target.removeEventListener(key.type, key.boundListener, key.useCapture);
+    var listenerArray = ol.events.getListeners(key.target, key.type);
+    if (listenerArray) {
+      ol.events.findListener_(listenerArray, key.listener,
+          key.useCapture, key.bindTo, true);
+      if (listenerArray.length === 0) {
+        var listenerMap = key.target[ol.events.LISTENER_MAP_PROP_];
+        delete listenerMap[key.type];
+        if (Object.keys(listenerMap).length === 0) {
+          delete key.target[ol.events.LISTENER_MAP_PROP_];
+        }
+      }
     }
-    delete ol.events.listenersByKey_[key];
+    goog.object.clear(key);
   }
 };
 
@@ -259,11 +316,10 @@ ol.events.unlistenByKey = function(key) {
  * @param {EventTarget|ol.events.EventTarget} target Target.
  */
 ol.events.unlistenAll = function(target) {
-  var listenerData;
+  var listenerMap = target[ol.events.LISTENER_MAP_PROP_];
-  for (var key in ol.events.listenersByKey_) {
+  if (listenerMap) {
-    listenerData = ol.events.listenersByKey_[key];
+    for (var type in listenerMap) {
-    if (listenerData.target === target || listenerData.thisArg === target) {
+      ol.events.unlistenByKey(listenerMap[type]);
-      ol.events.unlistenByKey(key);
     }
   }
 };

src/ol/events/eventtarget.js

@@ -57,7 +57,8 @@ ol.events.EventTarget.prototype.dispatchEvent = function(event) {
   var listeners = this.listeners_[type];
   if (listeners) {
     for (var i = listeners.length - 1; i >= 0; --i) {
-      if (listeners[i].call(this, evt) === false || evt.propagationStopped) {
+      if (listeners[i].call(this, evt) === false ||
+          evt.propagationStopped) {
         return false;
       }
     }

test/spec/ol/interaction/modifyinteraction.test.js

@@ -328,9 +328,9 @@ describe('ol.interaction.Modify', function() {
     beforeEach(function() {
       getListeners = function(feature, modify) {
         var listeners = ol.events.getListeners(
-            feature, ol.events.EventType.CHANGE);
+            feature, 'change');
         return listeners.filter(function(listener) {
-          return listener.handler == modify;
+          return listener.bindTo === modify;
         });
       };
     });
@@ -378,7 +378,6 @@ describe('ol.interaction.Modify', function() {
 
 goog.require('goog.dispose');
 goog.require('ol.events');
-goog.require('ol.events.EventType');
 goog.require('goog.style');
 goog.require('ol.Collection');
 goog.require('ol.Feature');

test/spec/ol/layer/layergroup.test.js

@@ -286,14 +286,14 @@ describe('ol.layer.Group', function() {
 
       var listeners = layerGroup.listenerKeys_[goog.getUid(layer)];
       expect(listeners.length).to.eql(2);
-      expect(typeof listeners[0]).to.be('string');
+      expect(typeof listeners[0]).to.be('object');
-      expect(typeof listeners[1]).to.be('string');
+      expect(typeof listeners[1]).to.be('object');
 
       // remove the layer from the group
       layers.pop();
       expect(goog.object.getCount(layerGroup.listenerKeys_)).to.eql(0);
-      expect(ol.events.listenersByKey_[listeners[0]]).to.be(undefined);
+      expect(listeners[0].listener).to.be(undefined);
-      expect(ol.events.listenersByKey_[listeners[1]]).to.be(undefined);
+      expect(listeners[1].listener).to.be(undefined);
     });
 
   });

test/spec/ol/observable.test.js

@@ -51,7 +51,7 @@ describe('ol.Observable', function() {
     it('returns a listener key', function() {
       var key = observable.on('foo', listener);
 
-      expect(typeof key).to.be('string');
+      expect(typeof key).to.be('object');
     });
 
   });
@@ -101,7 +101,7 @@ describe('ol.Observable', function() {
     it('returns a listener key', function() {
       var key = observable.once('foo', listener);
 
-      expect(typeof key).to.be('string');
+      expect(typeof key).to.be('object');
     });
 
   });