Make event-handlers edge-triggered and move socket-state to Dart.

Make event-handlers edge-triggered and move socket-state to Dart.

This eliminates to pause/resume calls made in the previous one-shot based model.

BUG=
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=32780

[Søren Gjesse, 2014-02-18 14:28:00]

LGTM, with comments

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
File runtime/bin/eventhandler_linux.cc (left):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_linux.cc:316: } else {
As far as I remember we had the code below because EPOLLRDHUP caused some
issues. Might be that this is not an issue any more - would be nice. Take a look
at http://stackoverflow.com/questions/6437879/how-do-i-use-epollhup. We should
try without and see if we run into issues.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
File runtime/bin/eventhandler_win.cc (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:376: if (pending_read_ != NULL) return true;
Should this ever happen?

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:994: // Only set mask if we turned on kInEvent
or kOutEvent.
What are the cases where neither in nor out events are turned on?

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:1010: // are no pending writes, post an out
event immediately.
You should probably explain that no pending write means that it has already
completed.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:243: if (short_write) {
Can we post a write event instead of this "trick"?

If not please rename short_write to simulate_short_write.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
runtime/bin/socket_patch.dart:510: if (result < 0) result = -result;
If the negative result is needed please explain it in a comment.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
runtime/bin/socket_patch.dart:632: issueReadEvent();
Why is there no "delayed: false" for this as well?

https://codereview.chromium.org/169383003/diff/200001/tests/standalone/io/htt...
File tests/standalone/io/http_basic_test.dart (right):

https://codereview.chromium.org/169383003/diff/200001/tests/standalone/io/htt...
tests/standalone/io/http_basic_test.dart:323: //  testStartStop();
Please re-activate these tests

[Anders Johnsen, 2014-02-19 09:43:02]

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
File runtime/bin/eventhandler_linux.cc (left):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_linux.cc:316: } else {
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> As far as I remember we had the code below because EPOLLRDHUP caused some
> issues. Might be that this is not an issue any more - would be nice. Take a
look
> at http://stackoverflow.com/questions/6437879/how-do-i-use-epollhup. We should
> try without and see if we run into issues.

It was needed to avoid the recv, etc.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
File runtime/bin/eventhandler_win.cc (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:376: if (pending_read_ != NULL) return true;
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> Should this ever happen?

Yes, in this case only. Will add comment.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:994: // Only set mask if we turned on kInEvent
or kOutEvent.
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> What are the cases where neither in nor out events are turned on?

close/shutdown before we listen for data. An example is stdin.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/eventhandle...
runtime/bin/eventhandler_win.cc:1010: // are no pending writes, post an out
event immediately.
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> You should probably explain that no pending write means that it has already
> completed.

Done.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket.cc
File runtime/bin/socket.cc (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket.cc#n...
runtime/bin/socket.cc:243: if (short_write) {
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> Can we post a write event instead of this "trick"?
> 
> If not please rename short_write to simulate_short_write.

It's extremely custom how we post such an event. I'll document this.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
File runtime/bin/socket_patch.dart (right):

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
runtime/bin/socket_patch.dart:510: if (result < 0) result = -result;
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> If the negative result is needed please explain it in a comment.

Done.

https://codereview.chromium.org/169383003/diff/200001/runtime/bin/socket_patc...
runtime/bin/socket_patch.dart:632: issueReadEvent();
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> Why is there no "delayed: false" for this as well?

It's always delayed. If it wasn't I was having trouble with order of read vs
close events.

https://codereview.chromium.org/169383003/diff/200001/tests/standalone/io/htt...
File tests/standalone/io/http_basic_test.dart (right):

https://codereview.chromium.org/169383003/diff/200001/tests/standalone/io/htt...
tests/standalone/io/http_basic_test.dart:323: //  testStartStop();
On 2014/02/18 14:28:00, Søren Gjesse wrote:
> Please re-activate these tests

Done.

[Anders Johnsen, 2014-02-19 09:44:45]

Committed patchset #7 manually as r32780 (presubmit successful).