Add support for the node net module.

Add support for several modules.

+ os
+ net
+ dns
+ http
+ url
+ querystring
+ cluster
+ zlib
+ Buffer
+ typed arrays
+ crypto

Put each node module in its own dart library.

Create interfaces for the different types of streams,
so that readable, writable, and readWrite streams
all share protocol.

Add simple echoServer and httpServer samples.
Committed: https://code.google.com/p/dart/source/detail?r=3471

[Jennifer Messerly, 2012-01-21 01:00:31]

Few minor comments but overall lgtm. Very cool to see more APIs getting exposed.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/child_process...
File frog/lib/node/child_process.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/child_process...
frog/lib/node/child_process.dart:11: class ChildProcess implements EventEmitter
native "ChildProcess" {
Does this need to be marked "native"? It looks like it just wraps a native
class.

Could you add a TODO that ideally we wouldn't be wrapping _childprocess?
In other words, it'd be nicer to have something like:

class ChildProcess implements EventEmitter native "ChildProcess" {

  void removeAllListeners(String event) native;
  void setMaxListeners(num n) native;

  ... etc ..
}

I can see needing the wrapper to workaround problems--we've done that in some
places too. But we generally try not to wrap if possible.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/child_process...
frog/lib/node/child_process.dart:61: return new ChildProcess(_spawn(_cp,
command, args, options));
_spawn doesn't take an options parameter -- does that work?

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/child_process...
frog/lib/node/child_process.dart:73: static var _spawn(var cp, String command,
List<String> args)
style nit: personally I wouldn't use "var" as an annotation, e.g.
  static _spawn(cp, String command, List<String> args)

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/child_process...
frog/lib/node/child_process.dart:81: var get child_process() {
I wonder if this should lazy initialize?

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/cluster.dart
File frog/lib/node/cluster.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/cluster.dart#...
frog/lib/node/cluster.dart:13: * JavaScript is run from node.js.
nice comment. I think this is an issue with our partial node.js sandbox. I bet
if we used the process APIs you ported to run the nested code in a new JS
process it would work.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart
File frog/lib/node/node.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart#new...
frog/lib/node/node.dart:11: #library('node');
nice to see this file simplified

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart#new...
frog/lib/node/node.dart:17: var require(String module) native;
I'm thinking this should be "_require", since the raw JS object is not usable
unless a typed wrapper for it exists.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart#new...
frog/lib/node/node.dart:338: class _EnvMap extends
NativeMapPrimitiveValue<String>{
This code does a wonderful job of working around it while staying clean, but I
think it makes a great case that we should have the compiler deal with
marshaling. Would you mind adding a TODO (it can be TODO(jmesserly)) that Frog
should marshal Maps to JS itself? (perhaps based on your NativeMapPrimitiveValue
wrapper)

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart#new...
frog/lib/node/node.dart:577: class Int8Array implements
TypedArrayBufferView<int> native "Int8Array" {
Do the various versions all have basically the same Dart interface?

I wonder if the code would be simpler to have:
    class TypedArrayBuffer native "*TypedArrayBuffer" {
       ... most of the members here, except ones that need to create instances
...
    }
    class Int8Array extends TypedArrayBuffer ... {
       ... same as before but without repeating the methods in TypedArrayBuffer
...
    }

I'm not 100% sure if that would work, although we play similar tricks in the
DOM, so it might. It would mainly just be a way to factor out the repeated code.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/node.dart#new...
frog/lib/node/node.dart:652: native """if (byteOffset === undefined) return new
Uint8Array(buffer);
this should be "byteOffset == null" which will check for null and undefined.
(Frog can use either one to represent this missing arg). Same comment on the
line below.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/nodeimpl.dart
File frog/lib/node/nodeimpl.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/nodeimpl.dart...
frog/lib/node/nodeimpl.dart:16: class NativeListBase<E> implements List<E>{
Do you know what the cases were that required wrapping Arrays?

JS Arrays should work just fine as Dart Lists (indeed that's how they're
implemented). If they aren't working we messed something up. (and I wouldn't
worry about the generic type arg--native functions will return a List<Dynamic>
which Dart accepts as convertible to List<SomeStrongerType>).

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/nodeimpl.dart...
frog/lib/node/nodeimpl.dart:46: class NativeMapBase<V> implements Map<String, V>
{
This is very cool. I've been wanting to test out something like this as an
implementation of Map<String, V> in Frog and see if it's faster than
HashMapImplementation (I suspect yes)

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/nodeimpl.dart...
frog/lib/node/nodeimpl.dart:95: native "var v = object[key];return v ==
undefined ? null : v;";
should this use hasOwnProperty?

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/nodeimpl.dart...
frog/lib/node/nodeimpl.dart:162: // Assume computation is more expensive than
allocation, so use a
Agree. This is usually the right assumption.

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/os.dart
File frog/lib/node/os.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/os.dart#newcode7
frog/lib/node/os.dart:7: class Os {
This is another class where I'm not sure if wrapping is needed?

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/os.dart#newco...
frog/lib/node/os.dart:22: var _cpus() native "return this._os.cpus()";
style nit: I'd leave off the "var" in return values

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/readline.dart
File frog/lib/node/readline.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/readline.dart...
frog/lib/node/readline.dart:34: void removeAllListeners(String event)
does this work as:
  void removeAllListeners(String event) native; ?

same comment for setMaxListeners

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/repl.dart
File frog/lib/node/repl.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/lib/node/repl.dart#new...
frog/lib/node/repl.dart:11: class repl native "require('repl')" {
interesting :)
this gives us a JS REPL, I presume?

http://codereview.chromium.org/9034014/diff/32020/frog/samples/node/echoServe...
File frog/samples/node/echoServer.dart (right):

http://codereview.chromium.org/9034014/diff/32020/frog/samples/node/echoServe...
frog/samples/node/echoServer.dart:7: // Note: the callback function must use the
type "Socket" for the socket argument,
hmmm. There's some sort of bug here then. You mean Socket.write was not getting
generated in the JS?

if you wouldn't mind it'd be great to file this as an issue on the tracker

[sra1, 2012-01-21 04:42:21]

I think this CL broke compilation of the Mac vserion of Dartium.

http://chromegw.corp.google.com/i/client.dashium/builders/dashium-mac-inc/bui...


____CompileC
../../xcodebuild/libxml.build/Release/libxml.build/Objects-normal/i386/xpath.o
    cd /b/build/slave/dashium-mac-inc/build/src/third_party/libxml
    setenv LANG en_US.US-ASCII
   
/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../third_party/llvm-build/Release+Asserts/bin/clang
-x c -arch i386 -fmessage-length=0 -pipe -fdiagnostics-print-source-range-info
-std=c99 -Wno-trigraphs -O3 -Wnewline-eof -DCHROMIUM_BUILD
-DWEBUI_TASK_MANAGER=1 -DENABLE_REMOTING=1 -DENABLE_P2P_APIS=1
-DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS
-DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1
-DENABLE_REGISTER_PROTOCOL_HANDLER=1 -DLIBXML_STATIC= -D_REENTRANT
-DU_USING_ICU_NAMESPACE=0 -DU_STATIC_IMPLEMENTATION -DNDEBUG -DNVALGRIND
-DDYNAMIC_ANNOTATIONS_ENABLED=0 -isysroot /Developer/SDKs/MacOSX10.5.sdk
-mmacosx-version-min=10.5 -gdwarf-2 -fvisibility=hidden -Wendif-labels
-Wno-unused-parameter -Wno-missing-field-initializers -Wheader-hygiene
-Wno-char-subscripts -Wno-unused-function -Wno-unnamed-type-template-args
-Wno-c++0x-compat -Wno-c++11-extensions
-F/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../xcodebuild/Release
-I/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../xcodebuild/Release/include
-I../icu/public/common -Imac -Imac/include -Isrc/include -I../zlib
-I/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../xcodebuild/libxml.build/Release/libxml.build/DerivedSources/i386
-I/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../xcodebuild/libxml.build/Release/libxml.build/DerivedSources
-fno-strict-aliasing -c
/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/src/xpath.c -o
/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/../../xcodebuild/libxml.build/Release/libxml.build/Objects-normal/i386/xpath.o
/b/build/slave/dashium-mac-inc/build/src/third_party/libxml/src/xpath.c:12676:23:{12676:24-12676:25}:
warning: use of unary operator that may be intended as compound assignment (+=)
                 total =+ xmlXPathCompOpEvalFilterFirst(ctxt, op, first);
                       ^~

[sra1, 2012-01-23 07:25:37]

https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.dart
File frog/lib/node/net.dart (right):

https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.d...
frog/lib/node/net.dart:17: native "return
this.createConnection(path,connectListener)";
'this' makes no sense in a static method.

Perhaps this should be the Dart-only body
{ return createConnection(path, connectionListener); }

[jackpal, 2012-01-23 13:13:38]

https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.dart
File frog/lib/node/net.dart (right):

https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.d...
frog/lib/node/net.dart:17: native "return
this.createConnection(path,connectListener)";
On 2012/01/23 07:25:37, sra1 wrote:
> 'this' makes no sense in a static method.

I am pretty sure the "this" is required. The Dart static method is implemented
in JavaScript as a normal method on a singleton instance. Without the "this"
there's no way for JavaScript to know which method to call.

[Jennifer Messerly, 2012-01-23 18:33:36]

On 2012/01/23 13:13:38, jackpal wrote:
>
https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.dart
> File frog/lib/node/net.dart (right):
> 
>
https://chromiumcodereview.appspot.com/9034014/diff/35002/frog/lib/node/net.d...
> frog/lib/node/net.dart:17: native "return
> this.createConnection(path,connectListener)";
> On 2012/01/23 07:25:37, sra1 wrote:
> > 'this' makes no sense in a static method.
> 
> I am pretty sure the "this" is required. The Dart static method is implemented
> in JavaScript as a normal method on a singleton instance. Without the "this"
> there's no way for JavaScript to know which method to call.

Yeah, that sounds right to me.