Refactor Dart_TimelineGetTrace to use a StreamConsumer callback

Refactor Dart_TimelineGetTrace to use a StreamConsumer callback

BUG=
R=asiva@google.com

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

[Cutch, 2015-08-11 16:08:09]

johnmccutchan@google.com changed reviewers:
+ asiva@google.com

[siva, 2015-08-11 16:54:33]

LGTM with some comments.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
File runtime/include/dart_tools_api.h (right):

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:916: * stream.
Should this be:
until there is no more data in a stream and there are no more streams ?

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:920: * \param buffer A pointer to data from the
stream.
"A pointer to the stream data" maybe?

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:922: * \param user_data The user data pointer
passed in when requesting the stream.
instead of user_data maybe name it stream_callback_data to be consistent with
some of the callback data we have elsewhere in this file.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:934: intptr_t state,
Dart_StreamConsumer_State state,

(see below).

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:945: #define DART_STREAM_CONSUMER_STATE_FINISH
2
why not use a enum here:

typedef enum {
  Dart_StreamConsumer_kStart = 0,
  ...
  ...
} Dart_StreamConsumer_State;

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5826: intptr_t remaining = output_length - cursor;
intptr_t remaining = output_length;

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5854: user_data);
Why does start and end have to have a NULL buffer why can't they have a buffer
too (would reduce the number of callback calls to be made).

[Cutch, 2015-08-11 17:08:39]

Committed patchset #1 (id:1) manually as
8be0e8a9c578b658408da39079a91b294863e34c (presubmit successful).

[Cutch, 2015-08-11 17:08:56]

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
File runtime/include/dart_tools_api.h (right):

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:916: * stream.
On 2015/08/11 16:54:33, siva wrote:
> Should this be:
> until there is no more data in a stream and there are no more streams ?

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:920: * \param buffer A pointer to data from the
stream.
On 2015/08/11 16:54:33, siva wrote:
> "A pointer to the stream data" maybe?

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:922: * \param user_data The user data pointer
passed in when requesting the stream.
On 2015/08/11 16:54:33, siva wrote:
> instead of user_data maybe name it stream_callback_data to be consistent with
> some of the callback data we have elsewhere in this file.

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:934: intptr_t state,
On 2015/08/11 16:54:33, siva wrote:
> Dart_StreamConsumer_State state,
> 
> (see below).

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/include/dart_tools_...
runtime/include/dart_tools_api.h:945: #define DART_STREAM_CONSUMER_STATE_FINISH
2
On 2015/08/11 16:54:33, siva wrote:
> why not use a enum here:
> 
> typedef enum {
>   Dart_StreamConsumer_kStart = 0,
>   ...
>   ...
> } Dart_StreamConsumer_State;

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5826: intptr_t remaining = output_length - cursor;
On 2015/08/11 16:54:33, siva wrote:
> intptr_t remaining = output_length;

Done.

https://codereview.chromium.org/1287543003/diff/1/runtime/vm/dart_api_impl.cc...
runtime/vm/dart_api_impl.cc:5854: user_data);
On 2015/08/11 16:54:33, siva wrote:
> Why does start and end have to have a NULL buffer why can't they have a buffer
> too (would reduce the number of callback calls to be made).

I find this API simpler to work with.