[Pkg-javascript-commits] [sockjs-client] 04/350: Fix handling of listeners being added and removed mid-dispatch

[tonnerre at ancient-solutions.com]

This is an automated email from the git hooks/post-receive script.

tonnerre-guest pushed a commit to branch upstream
in repository sockjs-client.

commit d2941069c5f1d5d6793ca65c9d4993b625171d44
Author: David Benjamin <davidben at mit.edu>
Date:   Tue Jul 2 01:42:55 2013 -0400

    Fix handling of listeners being added and removed mid-dispatch
    
    Before, having the last event listener remove itself (to implement
    EventEmitter.prototype.once-like semantics) threw an exception. Fix this by
    making a copy of the listener list on dispatch. While we're at it, mimic the
    semantics of the real DOM EventTarget interface. Event listeners added
    mid-dispatch do not run until the next one, however listeners removed, if they
    have not yet run, will not run in the current one.
    
    Also adds a unit test to test this behavior.
---
 lib/reventtarget.js             | 32 ++++++++++++++++++++++++++------
 tests/html/src/unittests.coffee | 23 ++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/lib/reventtarget.js b/lib/reventtarget.js
index 48a798d..22c63b9 100644
--- a/lib/reventtarget.js
+++ b/lib/reventtarget.js
@@ -9,6 +9,15 @@
 /* Simplified implementation of DOM2 EventTarget.
  *   http://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventTarget
  */
+
+var indexOfListener = function(list, listener) {
+    for (var i = 0; i < list.length; i++) {
+        if (list[i].listener === listener)
+            return i;
+    }
+    return -1;
+};
+
 var REventTarget = function() {};
 REventTarget.prototype.addEventListener = function (eventType, listener) {
     if(!this._listeners) {
@@ -18,8 +27,8 @@ REventTarget.prototype.addEventListener = function (eventType, listener) {
         this._listeners[eventType] = [];
     }
     var arr = this._listeners[eventType];
-    if(utils.arrIndexOf(arr, listener) === -1) {
-        arr.push(listener);
+    if(indexOfListener(arr, listener) === -1) {
+        arr.push({listener: listener, doomed: false});
     }
     return;
 };
@@ -29,10 +38,13 @@ REventTarget.prototype.removeEventListener = function (eventType, listener) {
         return;
     }
     var arr = this._listeners[eventType];
-    var idx = utils.arrIndexOf(arr, listener);
+    var idx = indexOfListener(arr, listener);
     if (idx !== -1) {
+        // Removing an event listener that has not yet run
+        // mid-dispatch causes the listener to not run.
+        arr[idx].doomed = true;
         if(arr.length > 1) {
-            this._listeners[eventType] = arr.slice(0, idx).concat( arr.slice(idx+1) );
+            arr.splice(idx, 1);
         } else {
             delete this._listeners[eventType];
         }
@@ -44,12 +56,20 @@ REventTarget.prototype.removeEventListener = function (eventType, listener) {
 REventTarget.prototype.dispatchEvent = function (event) {
     var t = event.type;
     var args = Array.prototype.slice.call(arguments, 0);
+    // TODO: This doesn't match the real behavior; per spec, onfoo get
+    // their place in line from the /first/ time they're set from
+    // non-null. Although WebKit bumps it to the end every time it's
+    // set.
     if (this['on'+t]) {
         this['on'+t].apply(this, args);
     }
     if (this._listeners && t in this._listeners) {
-        for(var i=0; i < this._listeners[t].length; i++) {
-            this._listeners[t][i].apply(this, args);
+        // Make a copy of the listeners list; listeners added
+        // mid-dispatch should NOT run.
+        var listeners = this._listeners[t].slice(0);
+        for(var i=0; i < listeners.length; i++) {
+            if (!listeners[i].doomed)
+                listeners[i].listener.apply(this, args);
         }
     }
 };
diff --git a/tests/html/src/unittests.coffee b/tests/html/src/unittests.coffee
index cf7b340..4931243 100644
--- a/tests/html/src/unittests.coffee
+++ b/tests/html/src/unittests.coffee
@@ -222,7 +222,7 @@ test 'detectProtocols', ->
             ['websocket', 'iframe-htmlfile', 'iframe-xhr-polling'])
 
 test "EventEmitter", ->
-    expect(4)
+    expect(6)
     r = new SockJS('//1.2.3.4/wrongurl', null,
                    {protocols_whitelist: []})
     r.addEventListener 'message', -> ok(true)
@@ -236,6 +236,27 @@ test "EventEmitter", ->
     r.removeEventListener 'message', bluff
     r.dispatchEvent({type:'message'})
 
+    # Listeners added mid-dispatch do not get run, however listeners
+    # removed mid-dispatch that have not yet run also don't run.
+    log = []
+    handler0 = -> log.push(0)
+    handler1 = ->
+        log.push(1)
+        r.removeEventListener 'test', handler0
+        r.removeEventListener 'test', handler2
+        r.addEventListener 'test', handler3
+    handler2 = -> log.push(2)
+    handler3 = -> log.push(3)
+    r.addEventListener 'test', handler0
+    r.addEventListener 'test', handler1
+    r.addEventListener 'test', handler2
+
+    r.dispatchEvent({type:'test'})
+    deepEqual(log, [0, 1])
+    log = []
+    r.dispatchEvent({type:'test'})
+    deepEqual(log, [1, 3])
+
     # Adding the same eventlistener should be indempotent (sockjs-client #4).
     single = -> ok(true)
     r.addEventListener 'close', single

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/sockjs-client.git