Allow for embedder provided service request handlers

Allow for embedder provided service request handlers

BUG=
R=turnidge@google.com

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

[turnidge, 2014-02-18 21:27:04]

https://codereview.chromium.org/170943003/diff/20001/runtime/bin/vmservice_im...
File runtime/bin/vmservice_impl.cc (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/bin/vmservice_im...
runtime/bin/vmservice_impl.cc:99: return strdup("{\"type\":
\"EmbedderString\",\"value\":\"Standalone\"}");
Maybe Embedder or EmbedderId instead of EmbedderString?

https://codereview.chromium.org/170943003/diff/20001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/include/dart_api...
runtime/include/dart_api.h:2522: void* user_data);
Do we want to be able to regiater top-level (i.e. non-isolate-based) callbacks?

https://codereview.chromium.org/170943003/diff/20001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:574: EmbedderHandleIsolateMessage(e_handler, &js);
Do we want to allow for asynchronous replies?  Should we have a TODO for this?

https://codereview.chromium.org/170943003/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1232: void* user_data) {
Add a heartbeat unit test for embedder callbacks in service_test.cc

[Cutch, 2014-02-18 22:18:51]

https://codereview.chromium.org/170943003/diff/20001/runtime/bin/vmservice_im...
File runtime/bin/vmservice_impl.cc (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/bin/vmservice_im...
runtime/bin/vmservice_impl.cc:99: return strdup("{\"type\":
\"EmbedderString\",\"value\":\"Standalone\"}");
On 2014/02/18 21:27:05, turnidge wrote:
> Maybe Embedder or EmbedderId instead of EmbedderString?

Done.

https://codereview.chromium.org/170943003/diff/20001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/include/dart_api...
runtime/include/dart_api.h:2522: void* user_data);
On 2014/02/18 21:27:05, turnidge wrote:
> Do we want to be able to regiater top-level (i.e. non-isolate-based)
callbacks?

Done here and elsewhere.

https://codereview.chromium.org/170943003/diff/20001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/170943003/diff/20001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1232: void* user_data) {
On 2014/02/18 21:27:05, turnidge wrote:
> Add a heartbeat unit test for embedder callbacks in service_test.cc

Done.

[turnidge, 2014-02-18 22:25:12]

lgtm after addressing removing comments...

https://codereview.chromium.org/170943003/diff/80001/runtime/bin/vmservice_im...
File runtime/bin/vmservice_impl.cc (right):

https://codereview.chromium.org/170943003/diff/80001/runtime/bin/vmservice_im...
runtime/bin/vmservice_impl.cc:102: tb.AddString("{\"type\":
\"Embedder\",\"value\":\"Standalone\"}");
Shouldn't this be a root-level request?

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:585: if (handler == NULL) {
What if you moved the EmbedderServiceHandler lookup inside this if?  Would this
get rid of the duplicate js.Setup stuff?

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:591: // TODO(johnmccutchan): Support asynchronous replies.
Do we need this TODO here?

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1180: if (handler == NULL) {
Similarly as above.

[turnidge, 2014-02-18 22:25:30]

um, "remaining" instead of "removing"

[Cutch, 2014-02-18 22:34:10]

https://codereview.chromium.org/170943003/diff/80001/runtime/bin/vmservice_im...
File runtime/bin/vmservice_impl.cc (right):

https://codereview.chromium.org/170943003/diff/80001/runtime/bin/vmservice_im...
runtime/bin/vmservice_impl.cc:102: tb.AddString("{\"type\":
\"Embedder\",\"value\":\"Standalone\"}");
On 2014/02/18 22:25:12, turnidge wrote:
> Shouldn't this be a root-level request?

I'm dropping these for now.

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:585: if (handler == NULL) {
On 2014/02/18 22:25:12, turnidge wrote:
> What if you moved the EmbedderServiceHandler lookup inside this if?  Would
this
> get rid of the duplicate js.Setup stuff?

Done and it does!

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:591: // TODO(johnmccutchan): Support asynchronous replies.
On 2014/02/18 22:25:12, turnidge wrote:
> Do we need this TODO here?

Done.

https://codereview.chromium.org/170943003/diff/80001/runtime/vm/service.cc#ne...
runtime/vm/service.cc:1180: if (handler == NULL) {
On 2014/02/18 22:25:12, turnidge wrote:
> Similarly as above.

Done.

[Cutch, 2014-02-18 22:38:17]

Committed patchset #4 manually as r32769 (presubmit successful).