Merge pull request #4991 from ahocevar/soft-remove-listeners

Do not remove listeners while dispatching event

src/ol/events/eventtarget.js

@@ -1,5 +1,6 @@
 goog.provide('ol.events.EventTarget');
 
+goog.require('goog.asserts');
 goog.require('ol.Disposable');
 goog.require('ol.events');
 goog.require('ol.events.Event');
@@ -26,6 +27,12 @@ ol.events.EventTarget = function() {
 
   goog.base(this);
 
+  /**
+   * @private
+   * @type {!Object.<string, number>}
+   */
+  this.pendingRemovals_ = {};
+
   /**
    * @private
    * @type {!Object.<string, Array.<ol.events.ListenerFunctionType>>}
@@ -46,7 +53,7 @@ ol.events.EventTarget.prototype.addEventListener = function(type, listener) {
     listeners = this.listeners_[type] = [];
   }
   if (listeners.indexOf(listener) === -1) {
-    listeners.unshift(listener);
+    listeners.push(listener);
   }
 };
 
@@ -63,13 +70,23 @@ ol.events.EventTarget.prototype.dispatchEvent = function(event) {
   var type = evt.type;
   evt.target = this;
   var listeners = this.listeners_[type];
+  var propagate;
   if (listeners) {
-    for (var i = listeners.length - 1; i >= 0; --i) {
+    if (!(type in this.pendingRemovals_)) {
-      if (listeners[i].call(this, evt) === false ||
+      this.pendingRemovals_[type] = 0;
-          evt.propagationStopped) {
+    }
-        return false;
+    for (var i = 0, ii = listeners.length; i < ii; ++i) {
+      if (listeners[i].call(this, evt) === false || evt.propagationStopped) {
+        propagate = false;
+        break;
       }
     }
+    var pendingRemovals = this.pendingRemovals_[type];
+    delete this.pendingRemovals_[type];
+    while (pendingRemovals--) {
+      this.removeEventListener(type, ol.nullFunction);
+    }
+    return propagate;
   }
 };
 
@@ -84,7 +101,7 @@ ol.events.EventTarget.prototype.disposeInternal = function() {
 
 /**
  * Get the listeners for a specified event type. Listeners are returned in the
- * opposite order that they will be called in.
+ * order that they will be called in.
  *
  * @param {string} type Type.
  * @return {Array.<ol.events.ListenerFunctionType>} Listeners.
@@ -114,9 +131,16 @@ ol.events.EventTarget.prototype.removeEventListener = function(type, listener) {
   var listeners = this.listeners_[type];
   if (listeners) {
     var index = listeners.indexOf(listener);
-    listeners.splice(index, 1);
+    goog.asserts.assert(index != -1, 'listener not found');
-    if (listeners.length === 0) {
+    if (type in this.pendingRemovals_) {
-      delete this.listeners_[type];
+      // make listener a no-op, and remove later in #dispatchEvent()
+      listeners[index] = ol.nullFunction;
+      ++this.pendingRemovals_[type];
+    } else {
+      listeners.splice(index, 1);
+      if (listeners.length === 0) {
+        delete this.listeners_[type];
+      }
     }
   }
 };

test/spec/ol/events.test.js

@@ -214,7 +214,7 @@ describe('ol.events', function() {
       var key2 = ol.events.listen(target, 'foo', listener, {});
       expect(key1.boundListener).to.not.equal(key2.boundListener);
       expect(target.getListeners('foo')).to.eql(
-          [key2.boundListener, key1.boundListener]);
+          [key1.boundListener, key2.boundListener]);
     });
   });
 

test/spec/ol/events/eventtarget.test.js

@@ -115,6 +115,38 @@ describe('ol.events.EventTarget', function() {
       expect(events[0]).to.equal(event);
       expect(events[0].target).to.equal(eventTarget);
     });
+    it('is safe to remove listeners in listeners', function() {
+      eventTarget.addEventListener('foo', spy3);
+      eventTarget.addEventListener('foo', function() {
+        eventTarget.removeEventListener('foo', spy1);
+        eventTarget.removeEventListener('foo', spy2);
+        eventTarget.removeEventListener('foo', spy3);
+      });
+      eventTarget.addEventListener('foo', spy1);
+      eventTarget.addEventListener('foo', spy2);
+      expect(function() {
+        eventTarget.dispatchEvent('foo');
+      }).not.to.throwException();
+      expect(called).to.eql([3]);
+      expect(eventTarget.getListeners('foo')).to.have.length(1);
+    });
+    it('is safe to do weird things in listeners', function() {
+      eventTarget.addEventListener('foo', spy2);
+      eventTarget.addEventListener('foo', function weird(evt) {
+        eventTarget.removeEventListener('foo', weird);
+        eventTarget.removeEventListener('foo', spy1);
+        eventTarget.dispatchEvent('foo');
+        eventTarget.removeEventListener('foo', spy2);
+        eventTarget.dispatchEvent('foo');
+        evt.preventDefault();
+      });
+      eventTarget.addEventListener('foo', spy1);
+      expect(function() {
+        eventTarget.dispatchEvent('foo');
+      }).not.to.throwException();
+      expect(called).to.eql([2, 2]);
+      expect(eventTarget.getListeners('foo')).to.be(undefined);
+    });
   });
 
   describe('#dispose()', function() {