Dart: Use a RawReceivePort to receive events for Mojo handles.

Dart: Use a RawReceivePort to receive events for Mojo handles.

This makes MojoEventStream more of an event handler, and
MojoEventStreamListener more of the controller of the event handler, so
I've changed the names accordingly.

Since handlers on RawReceivePorts don't throw exceptions into the current
Zone, but rather into the root Zone, errors thrown by event handlers are
now propagated to the event controller's onError function rather than
relying on clients using Zones.

This change has a noticible improvement in startup time due to avoiding
compiling Stream related functions. ~130ms -> ~120ms on my machine, and
so hopeuflly ~150ms -> 130-140ms on the perf bot.

BUG=
R=johnmccutchan@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/ca0cdfd1e380ab79c68c280bbd7ff6ae4ad1b6e5

[zra, 2015-11-04 21:21:09]

zra@google.com changed reviewers:
+ johnmccutchan@google.com

[zra, 2015-11-11 16:03:44]

ping

[Cutch, 2015-11-11 17:47:17]

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/embedder/io/s...
File mojo/dart/embedder/io/socket_patch.dart (left):

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/embedder/io/s...
mojo/dart/embedder/io/socket_patch.dart:345: onDone: _onOutputDone);
Who calls _onOutputError and _onOutputDone now?

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
File mojo/dart/test/validation_test.dart (right):

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
mojo/dart/test/validation_test.dart:98: return completer.future.then((_) {
... (_) async {

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
mojo/dart/test/validation_test.dart:100: conformanceImpl.close().then((_) {
await conformanceImpl.close();
...

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:7: class MojoEventHandler {
This class doesn't handle events. It wraps a subscription to events about a
specific handle.

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:39: void handleEvents(void
router(List<int> event)) {
I find this name confusing because handleEvents doesn't actually handle any
events. It just sets up a callback to called when events come in from the handle
watcher isolate.

Possible better names:

onEvent
registerForEvents
...

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:223: void handleRead() {}
Maybe this class should be called 'MojoEventHandler' because it actually invokes
the 'handleRead', 'handleWrite' methods.

[zra, 2015-11-11 18:44:33]

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/embedder/io/s...
File mojo/dart/embedder/io/socket_patch.dart (left):

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/embedder/io/s...
mojo/dart/embedder/io/socket_patch.dart:345: onDone: _onOutputDone);
On 2015/11/11 17:47:17, Cutch wrote:
> Who calls _onOutputError and _onOutputDone now?

These and _onInput{Error,Done} aren't needed anymore, and were redundant with
the PEER_CLOSED case of _on{Input,Output}Data previously. I've removed them.

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
File mojo/dart/test/validation_test.dart (right):

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
mojo/dart/test/validation_test.dart:98: return completer.future.then((_) {
On 2015/11/11 17:47:17, Cutch wrote:
> ... (_) async {

Acknowledged.

https://codereview.chromium.org/1414483010/diff/80001/mojo/dart/test/validati...
mojo/dart/test/validation_test.dart:100: conformanceImpl.close().then((_) {
On 2015/11/11 17:47:17, Cutch wrote:
> await conformanceImpl.close();
> ...

Changed to return instead of await since the result of runTest() is awaited.

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:7: class MojoEventHandler {
On 2015/11/11 17:47:17, Cutch wrote:
> This class doesn't handle events. It wraps a subscription to events about a
> specific handle.

Renamed MojoEventSubscription

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:39: void handleEvents(void
router(List<int> event)) {
On 2015/11/11 17:47:17, Cutch wrote:
> I find this name confusing because handleEvents doesn't actually handle any
> events. It just sets up a callback to called when events come in from the
handle
> watcher isolate.
> 
> Possible better names:
> 
> onEvent
> registerForEvents
> ...
> 

Renamed subscribe

https://codereview.chromium.org/1414483010/diff/80001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/event_stream.dart:223: void handleRead() {}
On 2015/11/11 17:47:17, Cutch wrote:
> Maybe this class should be called 'MojoEventHandler' because it actually
invokes
> the 'handleRead', 'handleWrite' methods.

Done.

[Cutch, 2015-11-11 19:08:16]

https://codereview.chromium.org/1414483010/diff/120001/mojo/dart/test/handle_...
File mojo/dart/test/handle_finalizer_test.dart (right):

https://codereview.chromium.org/1414483010/diff/120001/mojo/dart/test/handle_...
mojo/dart/test/handle_finalizer_test.dart:20: // After making a
MojoEventHandler, the underlying mojo handle will have
s/MojoEventHandler/MojoEventSubscription

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:7: class MojoEventSubscription {
Consider 'MojoHandleEventSubscription'

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:22: // Whether listen has been
called.
s/listen/subscribe

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:116: class MojoEventHandler {
Consider 'MojoHandleEventHandler'

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:161: void handleEvents() {
handleEvents -> beginHandlingEvents()

[zra, 2015-11-11 20:43:26]

https://codereview.chromium.org/1414483010/diff/120001/mojo/dart/test/handle_...
File mojo/dart/test/handle_finalizer_test.dart (right):

https://codereview.chromium.org/1414483010/diff/120001/mojo/dart/test/handle_...
mojo/dart/test/handle_finalizer_test.dart:20: // After making a
MojoEventHandler, the underlying mojo handle will have
On 2015/11/11 19:08:15, Cutch wrote:
> s/MojoEventHandler/MojoEventSubscription

Done.

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:7: class MojoEventSubscription {
On 2015/11/11 19:08:16, Cutch wrote:
> Consider 'MojoHandleEventSubscription'

Since Handles are the only source of events, and since
MojoHandleEventSubscription is a bit verbose, I'd like to stick with
MojoEventSubscription.

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:22: // Whether listen has been
called.
On 2015/11/11 19:08:15, Cutch wrote:
> s/listen/subscribe

Done.

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:116: class MojoEventHandler {
On 2015/11/11 19:08:16, Cutch wrote:
> Consider 'MojoHandleEventHandler'

Like above, my feeling is that this is too verbose.

https://codereview.chromium.org/1414483010/diff/120001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:161: void handleEvents() {
On 2015/11/11 19:08:16, Cutch wrote:
> handleEvents -> beginHandlingEvents()

Done.

[Cutch, 2015-11-11 20:57:56]

LGTM with some nits

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
File mojo/dart/embedder/io/socket_patch.dart (right):

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
mojo/dart/embedder/io/socket_patch.dart:470: MojoHandleSignals.READABLE);
formatting

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
mojo/dart/embedder/io/socket_patch.dart:478: MojoHandleSignals.WRITABLE);
formatting

https://codereview.chromium.org/1414483010/diff/140001/mojo/public/dart/mojo/...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/140001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:163: throw new
MojoApiError("MojoEventStreamListener is unbound.");
s/MojoEventStreamListener/MojoEventHandler

[zra, 2015-11-11 21:51:41]

Thanks!

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
File mojo/dart/embedder/io/socket_patch.dart (right):

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
mojo/dart/embedder/io/socket_patch.dart:470: MojoHandleSignals.READABLE);
On 2015/11/11 20:57:55, Cutch wrote:
> formatting

Done.

https://codereview.chromium.org/1414483010/diff/140001/mojo/dart/embedder/io/...
mojo/dart/embedder/io/socket_patch.dart:478: MojoHandleSignals.WRITABLE);
On 2015/11/11 20:57:55, Cutch wrote:
> formatting

Done.

https://codereview.chromium.org/1414483010/diff/140001/mojo/public/dart/mojo/...
File mojo/public/dart/mojo/lib/src/event_stream.dart (right):

https://codereview.chromium.org/1414483010/diff/140001/mojo/public/dart/mojo/...
mojo/public/dart/mojo/lib/src/event_stream.dart:163: throw new
MojoApiError("MojoEventStreamListener is unbound.");
On 2015/11/11 20:57:55, Cutch wrote:
> s/MojoEventStreamListener/MojoEventHandler

Done.

[zra, 2015-11-11 21:56:34]

Committed patchset #9 (id:160001) manually as
ca0cdfd1e380ab79c68c280bbd7ff6ae4ad1b6e5 (presubmit successful).