Add the c++ code part of bindings2/

Add the c++ code part of bindings2/

This is all the bindings-level dart code we had to write
to enable Dart in Sky.  We wrote this over the last 2
weeks in:
https://github.com/eseidel/skydart

R=abarth@chromium.org, jamesr@chromium.org
BUG=454613
Committed: https://chromium.googlesource.com/external/mojo/+/02574316f45210a33003dd08a2c9ee914a57dbf0

[abarth-chromium, 2015-02-12 20:52:33]

LGTM

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
File sky/engine/bindings2/builtin_natives.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:136: // TODO(dart): Hook up to developer
console (if/when that's a thing).
This comment should be indented two spaces

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
File sky/engine/bindings2/builtin_sky.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_sky.cc:26: ToDart(dart_state->CurrentWindow()))));
Can you add a TODO comment here about needing to retain the document wrapper?

[eseidel, 2015-02-12 21:09:37]

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin.cc
File sky/engine/bindings2/builtin.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin.cc:11: #include "mojo_natives.h"
This looks wrong?

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
File sky/engine/bindings2/builtin_natives.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:59: for (size_t i = 0; i < num_entries;
i++) {
I wonder if this is wroth having has an array or if a hashtable would be simpler
(and not slower enough to matter).

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:91: Dart_Handle internal_name =
Dart_NewStringFromCString("dart:_internal");
We should have a helper.  Dart_Handle LookupLibrary(char*) which does these 3
lines.

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:101: Dart_Handle timer_name =
Dart_NewStringFromCString("timerFactory");
We should consider a dartString(char*) helper.  Possibly one which handles
atomizing/caching.

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:144: fflush(stdout);
I think this only makes sense in the printf case, or?  Does log(INFO) always go
to out?

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:157: DartState* dart_state =
DartState::Current();
Do we need/want to get the state before checking the closure?

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/dart_ev...
File sky/engine/bindings2/dart_event_listener.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/dart_ev...
sky/engine/bindings2/dart_event_listener.cc:20:
CHECK(!Dart_IsError(Dart_GetPeer(handle, &peer)));
It's a bit strange to have GetPeer inside this CHECK.  I would split it out.

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/dart_ev...
sky/engine/bindings2/dart_event_listener.cc:21: if (DartEventListener* listener
= static_cast<DartEventListener*>(peer))
Should we just have toDartEventHandler(handle)?

[abarth-chromium, 2015-02-12 21:17:54]

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin.cc
File sky/engine/bindings2/builtin.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin.cc:11: #include "mojo_natives.h"
On 2015/02/12 at 21:09:37, eseidel wrote:
> This looks wrong?

sky/engine/bindings2/mojo_natives.h

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
File sky/engine/bindings2/builtin_natives.cc (right):

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:91: Dart_Handle internal_name =
Dart_NewStringFromCString("dart:_internal");
On 2015/02/12 at 21:09:37, eseidel wrote:
> We should have a helper.  Dart_Handle LookupLibrary(char*) which does these 3
lines.

We should just add DartConverter<char*>.  Then you can use ToDart("...")

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:144: fflush(stdout);
On 2015/02/12 at 21:09:37, eseidel wrote:
> I think this only makes sense in the printf case, or?  Does log(INFO) always
go to out?

Yeah, this should be in the printf branch.

https://codereview.chromium.org/918333002/diff/1/sky/engine/bindings2/builtin...
sky/engine/bindings2/builtin_natives.cc:157: DartState* dart_state =
DartState::Current();
On 2015/02/12 at 21:09:37, eseidel wrote:
> Do we need/want to get the state before checking the closure?

No.  It should move down.

[eseidel, 2015-02-12 21:41:40]

New patchsets have been uploaded after l-g-t-m from abarth@chromium.org

[eseidel, 2015-02-12 21:42:16]

Committed patchset #2 (id:20001) manually as
02574316f45210a33003dd08a2c9ee914a57dbf0.