Modern-ify KeyEvent handling.

sdk/lib/html/dartium/html_dartium.dart

Index: sdk/lib/html/dartium/html_dartium.dart
diff --git a/sdk/lib/html/dartium/html_dartium.dart b/sdk/lib/html/dartium/html_dartium.dart
index e1424ad95943e37b21fdcefa22297da2c4db64b1..157cfbb36fd10754c6798e09cb358c9ea1c07c26 100644
--- a/sdk/lib/html/dartium/html_dartium.dart
+++ b/sdk/lib/html/dartium/html_dartium.dart
@@ -33522,17 +33522,10 @@ class _CustomEventStreamProvider<T extends Event>
 
 
 /**
- * Works with KeyboardEvent and KeyEvent to determine how to expose information
- * about Key(board)Events. This class functions like an EventListenerList, and
- * provides a consistent interface for the Dart
- * user, despite the fact that a multitude of browsers that have varying
- * keyboard default behavior.
- *
- * This class is very much a work in progress, and we'd love to get information
- * on how we can make this class work with as many international keyboards as
- * possible. Bugs welcome!
+ * Internal class that does the actual calculations to determine keyCode and
+ * charCode for keydown, keypress, and keyup events for all browsers.
  */
-class KeyboardEventController {

[blois, 2013/03/25 07:01:20]

I think that KeyboardEventController and the EventStreamProvider should be
separate classes. And the provider just creates a controller when needed.

See comment below in forTarget.

+class _KeyboardEventHandler implements EventStreamProvider<KeyEvent> {
   // This code inspired by Closure's KeyHandling library.
   // http://closure-library.googlecode.com/svn/docs/closure_goog_events_keyhandler.js.source.html
 
@@ -33554,8 +33547,8 @@ class KeyboardEventController {
   // The distance to shift from upper case alphabet Roman letters to lower case.
   final int _ROMAN_ALPHABET_OFFSET = "a".codeUnits[0] - "A".codeUnits[0];
 
-  StreamSubscription _keyUpSubscription, _keyDownSubscription,
-      _keyPressSubscription;
+  /** Controller to produce KeyEvents for the stream. */
+  StreamController _controller;
 
   /**
    * An enumeration of key identifiers currently part of the W3C draft for DOM3
@@ -33588,53 +33581,41 @@ class KeyboardEventController {
     'Insert': KeyCode.INSERT
   };
 
-  /** Named constructor to add an onKeyPress event listener to our handler. */
-  KeyboardEventController.keypress(EventTarget target) {
-    _KeyboardEventController(target, 'keypress');
-  }
-
-  /** Named constructor to add an onKeyUp event listener to our handler. */
-  KeyboardEventController.keyup(EventTarget target) {
-    _KeyboardEventController(target, 'keyup');
-  }
+  /**
+   * Gets the type of the event which this would listen for on the specified
+   * event target.
+   */
+  String getEventType(EventTarget target) => 'KeyEvent';
 
-  /** Named constructor to add an onKeyDown event listener to our handler. */
-  KeyboardEventController.keydown(EventTarget target) {
-    _KeyboardEventController(target, 'keydown');
+  /** Return a stream for KeyEvents for the specified target. */
+  Stream<KeyEvent> forTarget(EventTarget e, {bool useCapture: false}) {

[blois, 2013/03/25 07:01:20]

What I imagined was that forTarget would do something like-

return new _KeyboardEventController.keydown(target).stream;

[Emily Fortuna, 2013/03/25 17:29:15]

How is this better from my existing KeyboardEventStream class which does the
same thing?

[Emily Fortuna, 2013/03/25 17:35:52]

Especially since this class is now private. I purposely created the
KeyboardEvenStream below to much more closely mimic the current way of
establishing event listeners to the streams like the rest of the html library.
Also note the KeyboardEventController class no longer exists.

[blois, 2013/03/26 17:46:20]

The primary thing is that there's the static _KeyboardEventHandler on KeyEvent,
and having these two merged causes conflicts in that shared instance.

Separating these would remove the per-element state from that static and ideally
we can add that to Element and make it even closer to the rest of the events. 

+    _initializeAllEventListeners(e);
+    return _controller.stream;
   }
 
   /**
    * General constructor, performs basic initialization for our improved
    * KeyboardEvent controller.
    */
-  _KeyboardEventController(EventTarget target, String type) {
-    _callbacks = [];
+  _KeyboardEventHandler(String type) {
     _type = type;
-    _target = target;
+    _controller = new StreamController.broadcast();
+    _callbacks = [];
   }
 
   /**
    * Hook up all event listeners under the covers so we can estimate keycodes
    * and charcodes when they are not provided.
    */
-  void _initializeAllEventListeners() {
+  _initializeAllEventListeners(EventTarget target) {
+    _target = target;
     _keyDownList = [];
-    if (_keyDownSubscription == null) {
-      _keyDownSubscription = Element.keyDownEvent.forTarget(
-          _target, useCapture: true).listen(processKeyDown);
-      _keyPressSubscription = Element.keyPressEvent.forTarget(
-          _target, useCapture: true).listen(processKeyUp);
-      _keyUpSubscription = Element.keyUpEvent.forTarget(
-          _target, useCapture: true).listen(processKeyPress);
-    }
-  }
-
-  /** Add a callback that wishes to be notified when a KeyEvent occurs. */
-  void add(void callback(KeyEvent)) {
-    if (_callbacks.length == 0) {
-      _initializeAllEventListeners();
-    }
-    _callbacks.add(callback);
+    Element.keyDownEvent.forTarget(_target, useCapture: true).listen(
+        processKeyDown);
+    Element.keyPressEvent.forTarget(_target, useCapture: true).listen(
+        processKeyPress);
+    Element.keyUpEvent.forTarget(_target, useCapture: true).listen(
+        processKeyUp);
   }
 
   /**
@@ -33642,31 +33623,8 @@ class KeyboardEventController {
    * occurred.
    */
   bool _dispatch(KeyEvent event) {
-    if (event.type == _type) {
-      // Make a copy of the listeners in case a callback gets removed while
-      // dispatching from the list.
-      List callbacksCopy = new List.from(_callbacks);
-      for(var callback in callbacksCopy) {
-        callback(event);
-      }
-    }
-  }
-
-  /** Remove the given callback from the listeners list. */
-  void remove(void callback(KeyEvent)) {
-    var index = _callbacks.indexOf(callback);
-    if (index != -1) {
-      _callbacks.removeAt(index);
-    }
-    if (_callbacks.length == 0) {
-      // If we have no listeners, don't bother keeping track of keypresses.
-      _keyDownSubscription.cancel();
-      _keyDownSubscription = null;
-      _keyPressSubscription.cancel();
-      _keyPressSubscription = null;
-      _keyUpSubscription.cancel();
-      _keyUpSubscription = null;
-    }
+    if (event.type == _type)
+      _controller.add(event);
   }
 
   /** Determine if caps lock is one of the currently depressed keys. */
@@ -33925,6 +33883,37 @@ class KeyboardEventController {
     _dispatch(e);
   }
 }
+
+
+/**
+ * Records KeyboardEvents that occur on a particular element, and provides a
+ * stream of outgoing KeyEvents with cross-browser consistent keyCode and
+ * charCode values despite the fact that a multitude of browsers that have
+ * varying keyboard default behavior.
+ *
+ * Example usage:
+ *
+ *     new KeyboardEventStream.onKeyDown(document.body).listen(
+ *         keydownHandlerTest);
+ *
+ * This class is very much a work in progress, and we'd love to get information
+ * on how we can make this class work with as many international keyboards as
+ * possible. Bugs welcome!
+ */
+class KeyboardEventStream extends Stream<KeyEvent> {
+
+  /** Named constructor to produce a stream for onKeyPress events. */
+  factory KeyboardEventStream.onKeyPress(EventTarget target) =>
+      new _KeyboardEventHandler('keypress').forTarget(target);
+
+  /** Named constructor to produce a stream for onKeyUp events. */
+  factory KeyboardEventStream.onKeyUp(EventTarget target) =>
+      new _KeyboardEventHandler('keyup').forTarget(target);
+
+  /** Named constructor to produce a stream for onKeyDown events. */
+  factory KeyboardEventStream.onKeyDown(EventTarget target) =>
+    new _KeyboardEventHandler('keydown').forTarget(target);
+}
 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
@@ -35272,6 +35261,9 @@ class _DocumentFragmentFactoryProvider {
  * inconsistencies, and also provide both keyCode and charCode information
  * for all key events (when such information can be determined).
  *
+ * KeyEvent tries to provide a higher level, more polished keyboard event
+ * information on top of the "raw" [KeyboardEvent].
+ *
  * This class is very much a work in progress, and we'd love to get information
  * on how we can make this class work with as many international keyboards as
  * possible. Bugs welcome!
@@ -35310,7 +35302,7 @@ class KeyEvent implements KeyboardEvent {
   /** Accessor to the underlying altKey value is the parent event. */
   bool get _realAltKey => _parent.altKey;
 
-  /** Construct a KeyEvent with [parent] as event we're emulating. */
+  /** Construct a KeyEvent with [parent] as the event we're emulating. */
   KeyEvent(KeyboardEvent parent) {
     _parent = parent;
     _shadowAltKey = _realAltKey;
@@ -35318,6 +35310,16 @@ class KeyEvent implements KeyboardEvent {
     _shadowKeyCode = _realKeyCode;
   }
 
+  /** Accessor to provide a stream of KeyEvents on the desired target. */
+  static EventStreamProvider<KeyEvent> keyDownEvent =
+    _KeyboardEventHandler('keydown');
+  /** Accessor to provide a stream of KeyEvents on the desired target. */
+  static EventStreamProvider<KeyEvent> keyUpEvent =
+    _KeyboardEventHandler('keyup');
+  /** Accessor to provide a stream of KeyEvents on the desired target. */
+  static EventStreamProvider<KeyEvent> keyPressEvent =
+    _KeyboardEventHandler('keypress');
+
   /** True if the altGraphKey is pressed during this event. */
   bool get altGraphKey => _parent.altGraphKey;
   bool get bubbles => _parent.bubbles;