[dart:io][windows] Implements RawSynchronousSocket

[dart:io][windows] Implements RawSynchronousSocket

related #29343
fixes #29333

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/bdb4eb6338991a63dc325e0261e0747677845e04

[zra, 2017-04-21 15:48:02]

Description was changed from

==========
[dart:io][windows] Implements RawSynchronousSocket
==========

to

==========
[dart:io][windows] Implements RawSynchronousSocket

related #29343
fixes #29333
==========

[zra, 2017-04-21 15:48:13]

zra@google.com changed reviewers:
+ asiva@google.com, bkonyi@google.com

[siva, 2017-04-21 16:40:37]

LGTM with some comments.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
File runtime/bin/sync_socket_macos.cc (right):

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_macos.cc:60: }
SynchronousSocket::Initialize() seems to be missing in this file?

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
File runtime/bin/sync_socket_win.cc (right):

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:21: }
This function seems to be the same in all versions why should it not just be in
sync_socket.cc?

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:88: return recv(socket,
reinterpret_cast<char*>(buffer), num_bytes, 0);
The behavior in SocketBase::Read for EAGAIN seems to be different than what is
implemented here (returns 0 vs -1 here).

Maybe SocketBase should ASSERT that the error can never be EAGAIN as it is a
sync read.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:96: return send(socket, reinterpret_cast<const
char*>(buffer), num_bytes, 0);
Ditto comment about behavior being different from SocketBase when EAGAIN error
happens.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:115: }
I suppose datagram sync sockets are not implemented yet and that is why there is
no equivalent for SocketBase::RecvFrom here?

Are there any plans or requirements fot sync datagram sockets?

[zra, 2017-04-22 06:29:28]

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
File runtime/bin/sync_socket_macos.cc (right):

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_macos.cc:60: }
On 2017/04/21 16:40:36, siva wrote:
> SynchronousSocket::Initialize() seems to be missing in this file?

Whoops. Added back.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
File runtime/bin/sync_socket_win.cc (right):

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:21: }
On 2017/04/21 16:40:36, siva wrote:
> This function seems to be the same in all versions why should it not just be
in
> sync_socket.cc?

Done.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:88: return recv(socket,
reinterpret_cast<char*>(buffer), num_bytes, 0);
On 2017/04/21 16:40:36, siva wrote:
> The behavior in SocketBase::Read for EAGAIN seems to be different than what is
> implemented here (returns 0 vs -1 here).
> 
> Maybe SocketBase should ASSERT that the error can never be EAGAIN as it is a
> sync read.

Added an extra parameter to SocketBase::Read(), etc. to indicate whether they're
called for a synchronous or asynchronous operation.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:96: return send(socket, reinterpret_cast<const
char*>(buffer), num_bytes, 0);
On 2017/04/21 16:40:36, siva wrote:
> Ditto comment about behavior being different from SocketBase when EAGAIN error
> happens.

Done.

https://codereview.chromium.org/2830273002/diff/20001/runtime/bin/sync_socket...
runtime/bin/sync_socket_win.cc:115: }
On 2017/04/21 16:40:36, siva wrote:
> I suppose datagram sync sockets are not implemented yet and that is why there
is
> no equivalent for SocketBase::RecvFrom here?

Right.

> Are there any plans or requirements fot sync datagram sockets?

I don't think we've had any request for that.

[zra, 2017-04-22 06:29:55]

Description was changed from

==========
[dart:io][windows] Implements RawSynchronousSocket

related #29343
fixes #29333
==========

to

==========
[dart:io][windows] Implements RawSynchronousSocket

related #29343
fixes #29333

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/bdb4eb6338991a63dc325e0261e0747677845e04
==========

[zra, 2017-04-22 06:29:58]

Committed patchset #5 (id:80001) manually as
bdb4eb6338991a63dc325e0261e0747677845e04 (presubmit successful).