Some cleanups before introducing Listenable handling to goog.events:

* Add goog.events.cleanUp that cleans up goog.events internal
data structure given a ListenableKey.

* Move key number generation to goog.events.ListenableKey.

* Rejigger code in goog.events to refer to
goog.events.listeners_ in as little places as possible
(subsequent logic to unify handling for DOM events and
Listenable will be cleaner, as listen_ will just return a
ListenableKey and listen/listenOnce will handle putting the
object into listeners_ array).

R=nicksantos,gboyer
DELTA=137 (104 added, 16 deleted, 17 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=6169


git-svn-id: http://closure-library.googlecode.com/svn/trunk@2452 0b95b8e8-c90f-11de-9d4f-f947ee5921c8

closure/goog/events/events.js

@@ -245,16 +245,14 @@ goog.events.listen_ = function(
   }
 
   var proxy = goog.events.getProxy();
-  proxy.src = src;
   listenerObj = new goog.events.Listener();
   listenerObj.init(listener, proxy, src, type, capture, opt_handler);
   listenerObj.callOnce = callOnce;
 
-  var key = listenerObj.key;
-  proxy.key = key;
+  proxy.src = src;
+  proxy.listener = listenerObj;
 
   listenerArray.push(listenerObj);
-  goog.events.listeners_[key] = listenerObj;
 
   if (!goog.events.sources_[srcUid]) {
     goog.events.sources_[srcUid] = [];
@@ -275,6 +273,8 @@ goog.events.listen_ = function(
     src.attachEvent(goog.events.getOnString_(type), proxy);
   }
 
+  var key = listenerObj.key;
+  goog.events.listeners_[key] = listenerObj;
   return key;
 };
 
@@ -288,10 +288,10 @@ goog.events.getProxy = function() {
   // Use a local var f to prevent one allocation.
   var f = goog.events.BrowserFeature.HAS_W3C_EVENT_SUPPORT ?
       function(eventObject) {
-        return proxyCallbackFunction.call(f.src, f.key, eventObject);
+        return proxyCallbackFunction.call(f.src, f.listener, eventObject);
       } :
       function(eventObject) {
-        var v = proxyCallbackFunction.call(f.src, f.key, eventObject);
+        var v = proxyCallbackFunction.call(f.src, f.listener, eventObject);
         // NOTE(user): In IE, we hack in a capture phase. However, if
         // there is inline event handler which tries to prevent default (for
         // example <a href="..." onclick="return false">...</a>) in a
@@ -491,6 +491,22 @@ goog.events.unlistenWithWrapper = function(src, wrapper, listener, opt_capt,
 };
 
 
+/**
+ * Cleans up goog.events internal data structure. This should be
+ * called by all implementations of goog.events.Listenable when
+ * removing listeners.
+ *
+ * TODO(user): Once we remove numeric key support from
+ * goog.events.listen and friend, we will be able to remove this
+ * requirement.
+ *
+ * @param {goog.events.ListenableKey} listenableKey The key to clean up.
+ */
+goog.events.cleanUp = function(listenableKey) {
+  delete goog.events.listeners_[listenableKey.key];
+};
+
+
 /**
  * Cleans up the listener array as well as the listener tree
  * @param {string} type  The type of the event.
@@ -953,23 +969,19 @@ goog.events.protectBrowserEventEntryPoint = function(errorHandler) {
  * Handles an event and dispatches it to the correct listeners. This
  * function is a proxy for the real listener the user specified.
  *
- * @param {number} key Unique key for the listener.
+ * @param {goog.events.Listener} listener The listener object.
  * @param {Event=} opt_evt Optional event object that gets passed in via the
  *     native event handlers.
  * @return {boolean} Result of the event handler.
  * @this {goog.events.EventTarget|Object} The object or Element that
  *     fired the event.
  * @private
  */
-goog.events.handleBrowserEvent_ = function(key, opt_evt) {
-  // If the listener isn't there it was probably removed when processing
-  // another listener on the same event (e.g. the later listener is
-  // not managed by closure so that they are both fired under IE)
-  if (!goog.events.listeners_[key]) {
+goog.events.handleBrowserEvent_ = function(listener, opt_evt) {
+  if (listener.removed) {
     return true;
   }
 
-  var listener = goog.events.listeners_[key];
   var type = listener.type;
   var map = goog.events.listenerTree_;
 

closure/goog/events/eventtarget.js

@@ -312,9 +312,7 @@ goog.events.EventTarget.prototype.unlisten = function(
       listenerArray, listener, opt_useCapture, opt_listenerScope);
   if (index > -1) {
     var listenerObj = listenerArray[index];
-    // We still need to mark this as removed as unlisten may be called
-    // when a listener fired (the listener may already be queued to be
-    // fired in the same dispatch sequence).
+    goog.events.cleanUp(listenerObj);
     listenerObj.removed = true;
     return goog.array.removeAt(listenerArray, index);
   }
@@ -331,7 +329,12 @@ goog.events.EventTarget.prototype.unlistenByKey = function(key) {
     return false;
   }
 
-  return goog.array.remove(this.eventTargetListeners_[type], key);
+  var removed = goog.array.remove(this.eventTargetListeners_[type], key);
+  if (removed) {
+    goog.events.cleanUp(key);
+    key.removed = true;
+  }
+  return removed;
 };
 
 
@@ -342,7 +345,11 @@ goog.events.EventTarget.prototype.removeAllListeners = function(
   for (var type in this.eventTargetListeners_) {
     if (!opt_type || type == opt_type) {
       var listenerArray = this.eventTargetListeners_[type];
-      count += listenerArray.length;
+      for (var i = 0; i < listenerArray.length; i++) {
+        ++count;
+        goog.events.cleanUp(listenerArray[i]);
+        listenerArray[i].removed = true;
+      }
       listenerArray.length = 0;
     }
   }

closure/goog/events/eventtarget_test.html

@@ -32,6 +32,9 @@
   var unlistenFn = function(src, type, listener, opt_capt, opt_handler) {
     return src.unlisten(type, listener, opt_capt, opt_handler);
   };
+  var unlistenByKeyFn = function(src, key) {
+    return src.unlistenByKey(key);
+  };
   var listenOnceFn = function(src, type, listener, opt_capt, opt_handler) {
     return src.listenOnce(type, listener, opt_capt, opt_handler);
   };
@@ -46,7 +49,7 @@
   };
 
   goog.events.eventTargetTester.setUp(
-      listenFn, unlistenFn,
+      listenFn, unlistenFn, unlistenByKeyFn,
       listenOnceFn, dispatchEventFn,
       removeAllFn, getListenersFn,
       goog.events.eventTargetTester.KeyType.NUMBER,

closure/goog/events/eventtarget_via_googevents_test.html

@@ -22,8 +22,11 @@
 <script>
 
 function setUp() {
+  var unlistenByKeyFn = function(src, key) {
+    return goog.events.unlistenByKey(key);
+  };
   goog.events.eventTargetTester.setUp(
-      goog.events.listen, goog.events.unlisten,
+      goog.events.listen, goog.events.unlisten, unlistenByKeyFn,
       goog.events.listenOnce, goog.events.dispatchEvent,
       goog.events.removeAll, goog.events.getListeners,
       goog.events.eventTargetTester.KeyType.NUMBER,

closure/goog/events/eventtarget_via_w3cinterface_test.html

@@ -32,7 +32,7 @@
   };
 
   goog.events.eventTargetTester.setUp(
-      listenFn, unlistenFn,
+      listenFn, unlistenFn, null /* unlistenByKeyFn */,
       null /* listenOnceFn */, dispatchEventFn, null /* removeAllFn */,
       null /* getListenersFn */,
       goog.events.eventTargetTester.KeyType.UNDEFINED,

closure/goog/events/eventtargettester.js

@@ -38,6 +38,9 @@ goog.require('goog.testing.recordFunction');
  * @param {Function} unlistenFn Function that, given the same
  *     signature as goog.events.unlisten, will remove listener from
  *     the given event target.
+ * @param {Function} unlistenByKeyFn Function that, given 2
+ *     parameters: src and key, will remove the corresponding
+ *     listener.
  * @param {Function} listenOnceFn Function that, given the same
  *     signature as goog.events.listenOnce, will add a one-time
  *     listener to the given event target.
@@ -58,11 +61,12 @@ goog.require('goog.testing.recordFunction');
  *     be set to false.
  */
 goog.events.eventTargetTester.setUp = function(
-    listenFn, unlistenFn, listenOnceFn, dispatchEventFn,
-    removeAllFn, getListenersFn,
+    listenFn, unlistenFn, unlistenByKeyFn, listenOnceFn,
+    dispatchEventFn, removeAllFn, getListenersFn,
     listenKeyType, unlistenFnReturnType) {
   listen = listenFn;
   unlisten = unlistenFn;
+  unlistenByKey = unlistenByKeyFn;
   listenOnce = listenOnceFn;
   dispatchEvent = dispatchEventFn;
   removeAll = removeAllFn;
@@ -164,7 +168,7 @@ var EventType = {
 };
 
 
-var listen, unlisten, listenOnce, dispatchEvent;
+var listen, unlisten, unlistenByKey, listenOnce, dispatchEvent;
 var removeAll, getListeners;
 var keyType, unlistenReturnType;
 var eventTargets, listeners;
@@ -711,6 +715,40 @@ function testUnlistenInListen() {
 }
 
 
+function testUnlistenByKeyInListen() {
+  if (!unlistenByKey) {
+    return;
+  }
+
+  var key1, key2;
+  listeners[1] = createListener(
+      function(e) {
+        unlistenByKey(eventTargets[0], key1);
+        unlistenByKey(eventTargets[0], key2);
+      });
+  listen(eventTargets[0], EventType.A, listeners[0]);
+  key1 = listen(eventTargets[0], EventType.A, listeners[1]);
+  key2 = listen(eventTargets[0], EventType.A, listeners[2]);
+  listen(eventTargets[0], EventType.A, listeners[3]);
+
+  dispatchEvent(eventTargets[0], EventType.A);
+
+  assertListenerIsCalled(listeners[0], times(1));
+  assertListenerIsCalled(listeners[1], times(1));
+  assertListenerIsCalled(listeners[2], times(0));
+  assertListenerIsCalled(listeners[3], times(1));
+  assertNoOtherListenerIsCalled();
+  resetListeners();
+
+  dispatchEvent(eventTargets[0], EventType.A);
+  assertListenerIsCalled(listeners[0], times(1));
+  assertListenerIsCalled(listeners[1], times(0));
+  assertListenerIsCalled(listeners[2], times(0));
+  assertListenerIsCalled(listeners[3], times(1));
+  assertNoOtherListenerIsCalled();
+}
+
+
 function testSetParentEventTarget() {
   assertNull(eventTargets[0].getParentEventTarget());
 

closure/goog/events/listenable.js

@@ -124,6 +124,8 @@ goog.events.Listenable.prototype.listenOnce;
 /**
  * Removes an event listener which was added with listen() or listenOnce().
  *
+ * Implementation needs to call goog.events.cleanUp.
+ *
  * @param {string} type Event type or array of event types.
  * @param {!Function} listener Callback method, or an object
  *     with a handleEvent function. TODO(user): Consider whether
@@ -141,6 +143,8 @@ goog.events.Listenable.prototype.unlisten;
  * Removes an event listener which was added with listen() by the key
  * returned by listen().
  *
+ * Implementation needs to call goog.events.cleanUp.
+ *
  * @param {goog.events.ListenableKey} key The key returned by
  *     listen() or listenOnce().
  * @return {boolean} Whether any listener was removed.
@@ -169,6 +173,9 @@ goog.events.Listenable.prototype.dispatchEvent;
  * it will only remove listeners of the particular type. otherwise all
  * registered listeners will be removed.
  *
+ * Implementation needs to call goog.events.cleanUp for each removed
+ * listener.
+ *
  * @param {string=} opt_type Type of event to remove, default is to
  *     remove all types.
  * @return {number} Number of listeners removed.
@@ -213,6 +220,24 @@ goog.events.Listenable.prototype.getListeners;
 goog.events.ListenableKey = function() {};
 
 
+/**
+ * Counter used to create a unique key
+ * @type {number}
+ * @private
+ */
+goog.events.ListenableKey.counter_ = 0;
+
+
+/**
+ * Reserves a key to be used for ListenableKey#key field.
+ * @return {number} A number to be used to fill ListenableKey#key
+ *     field.
+ */
+goog.events.ListenableKey.reserveKey = function() {
+  return ++goog.events.ListenableKey.counter_;
+};
+
+
 /**
  * The source event target.
  * @type {!Object}
@@ -247,3 +272,10 @@ goog.events.ListenableKey.prototype.capture;
  * @type {Object}
  */
 goog.events.ListenableKey.prototype.handler;
+
+
+/**
+ * A globally unique number to identify the key.
+ * @type {number}
+ */
+goog.events.ListenableKey.prototype.key;

closure/goog/events/listener.js

@@ -35,14 +35,6 @@ goog.events.Listener = function() {
 };
 
 
-/**
- * Counter used to create a unique key
- * @type {number}
- * @private
- */
-goog.events.Listener.counter_ = 0;
-
-
 /**
  * @define {boolean} Whether to enable the monitoring of the
  *     goog.events.Listener instances. Switching on the monitoring is only
@@ -106,6 +98,7 @@ goog.events.Listener.prototype.handler;
 /**
  * The key of the listener.
  * @type {number}
+ * @override
  */
 goog.events.Listener.prototype.key = 0;
 
@@ -162,7 +155,7 @@ goog.events.Listener.prototype.init = function(listener, proxy, src, type,
   this.capture = !!capture;
   this.handler = opt_handler;
   this.callOnce = false;
-  this.key = ++goog.events.Listener.counter_;
+  this.key = goog.events.ListenableKey.reserveKey();
   this.removed = false;
 };