Refactor Dart_TimelineGetTrace to use a StreamConsumer callback

runtime/include/dart_tools_api.h

Index: runtime/include/dart_tools_api.h
diff --git a/runtime/include/dart_tools_api.h b/runtime/include/dart_tools_api.h
index a70380c96faae140b16376faf1a74f0b053c2836..15c549a6b5bce076dcc0e18d40082db1c999c12e 100644
--- a/runtime/include/dart_tools_api.h
+++ b/runtime/include/dart_tools_api.h
@@ -910,19 +910,52 @@ DART_EXPORT Dart_Handle Dart_ServiceSendDataEvent(const char* stream_id,
 DART_EXPORT void Dart_TimelineSetRecordedStreams(int64_t stream_mask);
 
 /**
- * Get the timeline for the current isolate in trace-event format
+ * A stream consumer callback function.
+ *
+ * This function will be called repeatedly until there is no more data in a
+ * stream.

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

Should this be:
until there is no more data in a stream and there are no more streams ?

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

Done.

+ *
+ * \param state Indicates a new stream, data, or a finished stream.
+ * \param stream_name A name for this stream. Not guaranteed to be meaningful.
+ * \param buffer A pointer to data from the stream.

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

"A pointer to the stream data" maybe?

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

Done.

+ * \param buffer_length The number of bytes at buffer that should be consumed.
+ * \param user_data The user data pointer passed in when requesting the stream.

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

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.

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

Done.

  *
- * \param output The address of the trace buffer output
- * \param output_length The length of the trace buffer output
+ * At the start of each stream state will be DART_STREAM_CONSUMER_STATE_START
+ * and buffer will be NULL.
+ *
+ * For each chunk of data the state will be DART_STREAM_CONSUMER_STATE_DATA
+ * and buffer will not be NULL.
+ *
+ * At the end of each stream state will be DART_STREAM_CONSUMER_STATE_FINISH
+ * and buffer will be NULL.
+ */
+typedef void (*Dart_StreamConsumer)(
+    intptr_t state,

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

Dart_StreamConsumer_State state,

(see below).

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

Done.

+    const char* stream_name,
+    uint8_t* buffer,
+    intptr_t buffer_length,
+    void* user_data);
+
+/** Indicates a new stream is being output */
+#define DART_STREAM_CONSUMER_STATE_START 0
+/** Data for the current stream */
+#define DART_STREAM_CONSUMER_STATE_DATA 1
+/** Indicates stream is finished */
+#define DART_STREAM_CONSUMER_STATE_FINISH 2

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

why not use a enum here:

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

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

Done.

+
+/**
+ * Get the timeline for the current isolate in trace-event format
  *
- * NOTE: output is allocated in the C heap and must be freed by the caller.
+ * \param consumer A Dart_StreamConsumer.
+ * \param user_data User data passed into consumer.
  *
  * NOTE: The trace-event format is documented here: https://goo.gl/hDZw5M
  *
- * \return True if a trace was output. The outputted trace may be empty.
+ * \return True if a stream was output.
  */
-DART_EXPORT bool Dart_TimelineGetTrace(const char** output,
-                                       intptr_t* output_length);
+DART_EXPORT bool Dart_TimelineGetTrace(Dart_StreamConsumer consumer,
+                                       void* user_data);
 
 /**
  * Add a duration timeline event to the embedder stream for the current isolate.