Dart: Simplifies the handle watcher. Various cleanups and bugfixes.

mojo/public/dart/src/handle.dart

Index: mojo/public/dart/src/handle.dart
diff --git a/mojo/public/dart/src/handle.dart b/mojo/public/dart/src/handle.dart
index 79acbd70c1e49f4c595877cf10d29fdb2e63dd46..d8f4ed4a24c66c710881f98f90400dd135c1dda0 100644
--- a/mojo/public/dart/src/handle.dart
+++ b/mojo/public/dart/src/handle.dart
@@ -34,8 +34,8 @@ class RawMojoHandle {
     return new MojoResult(result);
   }
 
-  bool _ready(int signal) {
-    MojoResult res = wait(signal, 0);
+  bool _ready(MojoHandleSignals signal) {
+    MojoResult res = wait(signal.value, 0);
     switch (res) {
       case MojoResult.OK:
         return true;
@@ -46,12 +46,12 @@ class RawMojoHandle {
         return false;
       default:
         // Should be unreachable.
-        throw new Exception("Unreachable");
+        throw "Unexpected result $res for wait on $h";
     }
   }
 
-  bool readyRead() => _ready(MojoHandleSignals.READABLE);
-  bool readyWrite() => _ready(MojoHandleSignals.WRITABLE);
+  bool get readyRead => _ready(MojoHandleSignals.READABLE);
+  bool get readyWrite => _ready(MojoHandleSignals.WRITABLE);
 
   static int waitMany(List<int> handles,
                       List<int> signals,
@@ -94,59 +94,40 @@ class MojoHandle extends Stream<int> {
   ReceivePort _receivePort;
 
   // The signals on this handle that we're interested in.
-  int _signals;
+  MojoHandleSignals _signals;
 
-  // Whether the handle has been added to the handle watcher.
-  bool _eventHandlerAdded;
+  // Whether listen has been called.
+  bool _isListening;
 
-  MojoHandle(this._handle) :
-      _signals = MojoHandleSignals.READABLE,
-      _eventHandlerAdded = false {
+  MojoHandle(RawMojoHandle handle,
+             [MojoHandleSignals signals = MojoHandleSignals.READABLE]) :
+      _handle = handle,
+      _signals = signals,
+      _isListening = false {
     MojoResult result = RawMojoHandle.register(this);
     if (!result.isOk) {
-      throw new Exception("Failed to register the MojoHandle");
+      throw "Failed to register the MojoHandle: $result.";
     }
   }
 
   void close() {
-    if (_eventHandlerAdded) {
+    if (_handle != null) {
       MojoHandleWatcher.close(_handle);
-      _eventHandlerAdded = false;
-    } else {
-      // If we're not in the handle watcher, then close the handle manually.
       _handle.close();
+      _handle = null;
     }
     if (_receivePort != null) {
       _receivePort.close();
+      _receivePort = null;
     }
   }
 
-  // We wrap the callback provided by clients in listen() with some code to
-  // handle adding and removing the handle to/from the handle watcher. Because
-  // the handle watcher removes this handle whenever it receives an event,
-  // we have to re-add it when the callback is finished.
-  Function _onDataClosure(origOnData) {
-    return ((int event) {
-      // The handle watcher removes this handle from its set on an event.
-      _eventHandlerAdded = false;
-      origOnData(event);
-
-      // The callback could have closed the handle. If so, don't add it back to
-      // the MojoHandleWatcher.
-      if (_handle.isValid) {
-        assert(!_eventHandlerAdded);
-        var res = MojoHandleWatcher.add(_handle, _sendPort, _signals);
-        if (!res.isOk) {
-          throw new Exception("Failed to re-add handle: $res");
-        }
-        _eventHandlerAdded = true;
-      }
-    });
-  }
-
-  StreamSubscription<int> listen(
-      void onData(int event),
+  StreamSubscription<List<int>> listen(
+      void onData(List event),
       {Function onError, void onDone(), bool cancelOnError}) {
+    if (_isListening) {
+      throw "Listen has already been called: $_handle.";
+    }
     _receivePort = new ReceivePort();
     _sendPort = _receivePort.sendPort;
     _controller = new StreamController(sync: true,
@@ -156,41 +137,38 @@ class MojoHandle extends Stream<int> {
         onResume: _onPauseStateChange);
     _controller.addStream(_receivePort);
 
-    assert(!_eventHandlerAdded);
-    var res = MojoHandleWatcher.add(_handle, _sendPort, _signals);
-    if (!res.isOk) {
-      throw new Exception("MojoHandleWatcher add failed: $res");
+    if (_signals != MojoHandleSignals.NONE) {
+      var res = MojoHandleWatcher.add(_handle, _sendPort, _signals.value);
+      if (!res.isOk) {
+        throw "MojoHandleWatcher add failed: $res";
+      }
     }
-    _eventHandlerAdded = true;
 
+    _isListening = true;
     return _controller.stream.listen(
-        _onDataClosure(onData),
+        onData,
         onError: onError,
         onDone: onDone,
         cancelOnError: cancelOnError);
   }
 
   bool writeEnabled() => MojoHandleSignals.isWritable(_signals);
+  bool readEnabled() => MojoHandleSignals.isReadable(_signals);

[siva, 2014/12/29 23:20:44]

These should also just be getters?

[zra, 2014/12/30 16:29:33]

Yes. These aren't actually be used anywhere though, so I've removed them.

 
-  void toggleWriteEvents() {
-    _signals = MojoHandleSignals.toggleWrite(_signals);
-    if (_eventHandlerAdded) {
-      var res = MojoHandleWatcher.toggleWrite(_handle);
-      if (!res.isOk) {
-        throw new Exception("MojoHandleWatcher failed to toggle write: $res");
-      }
+  void enableSignals(MojoHandleSignals signals) {
+    _signals = signals;
+    if (!_isListening) {
+      throw "Cannot enable signals if listen() has not been called.";
+    }
+    var res = MojoHandleWatcher.add(_handle, _sendPort, signals.value);
+    if (!res.isOk) {
+      throw "MojoHandleWatcher add failed: $res";
     }
   }
 
-  void enableWriteEvents() {
-    assert(!writeEnabled());
-    toggleWriteEvents();
-  }
-
-  void disableWriteEvents() {
-    assert(writeEnabled());
-    toggleWriteEvents();
-  }
+  void enableReadEvents() => enableSignals(MojoHandleSignals.READABLE);
+  void enableWriteEvents() => enableSignals(MojoHandleSignals.WRITABLE);
+  void enableAllEvents() => enableSignals(MojoHandleSignals.READWRITE);
 
   void _onSubscriptionStateChange() {
     if (!_controller.hasListener) {
@@ -200,23 +178,20 @@ class MojoHandle extends Stream<int> {
 
   void _onPauseStateChange() {
     if (_controller.isPaused) {
-      if (_eventHandlerAdded) {
-        var res = MojoHandleWatcher.remove(_handle);
-        if (!res.isOk) {
-          throw new Exception("MojoHandleWatcher add failed: $res");
-        }
-        _eventHandlerAdded = false;
+      var res = MojoHandleWatcher.remove(_handle);
+      if (!res.isOk) {
+        throw "MojoHandleWatcher add failed: $res";
       }
     } else {
-      if (!_eventHandlerAdded) {
-        var res = MojoHandleWatcher.add(_handle, _sendPort, _signals);
-        if (!res.isOk) {
-          throw new Exception("MojoHandleWatcher add failed: $res");
-        }
-        _eventHandlerAdded = true;
+      var res = MojoHandleWatcher.add(_handle, _sendPort, _signals.value);
+      if (!res.isOk) {
+        throw "MojoHandleWatcher add failed: $res";
       }
     }
   }
 
+  bool get readyRead => _handle.readyRead;
+  bool get readyWrite => _handle.readyWrite;
+
   String toString() => "$_handle";
 }