Add ability for embedder to provide private timeline trace data

Add ability for embedder to provide private timeline trace data

- Add three new embedder hooks registered with Dart_SetEmbedderTimelineCallbacks
  - Dart_EmbedderTimelineStartRecording
  - Dart_EmbedderTimelineStopRecording
  - Dart_EmbedderTimelineGetTimeline

- Add unit test for record start / stop notification.
- Add unit test for get timeline.

This should allow Flutter to remove their hacked up timeline service RPCs.

R=chinmaygarde@google.com, rmacnak@google.com

Committed: https://github.com/dart-lang/sdk/commit/53cae0aed917827e24e5ff5e267e2dae51610a36

[Cutch, 2016-03-10 19:56:49]

Description was changed from

==========
Add ability for embedder to provide private timeline trace data

BUG=
==========

to

==========
Add ability for embedder to provide private timeline trace data

- Add three new embedder hooks registered with Dart_SetEmbedderTimelineCallbacks
  - Dart_EmbedderTimelineStartRecording
  - Dart_EmbedderTimelineStopRecording
  - Dart_EmbedderTimelineGetTimeline

- Add unit test for record start / stop notification.
- Add unit test for get timeline.

This should allow Flutter to remove their hacked up timeline service RPCs.
==========

[Cutch, 2016-03-10 19:56:50]

johnmccutchan@google.com changed reviewers:
+ chinmaygarde@google.com, rmacnak@google.com

[Chinmay, 2016-03-10 20:04:35]

On 2016/03/10 at 19:56:56, johnmccutchan wrote:
> 

lgtm (not an owner though)

Thanks!

[rmacnak, 2016-03-10 20:39:21]

lgtm

[Cutch, 2016-03-10 21:30:33]

Description was changed from

==========
Add ability for embedder to provide private timeline trace data

- Add three new embedder hooks registered with Dart_SetEmbedderTimelineCallbacks
  - Dart_EmbedderTimelineStartRecording
  - Dart_EmbedderTimelineStopRecording
  - Dart_EmbedderTimelineGetTimeline

- Add unit test for record start / stop notification.
- Add unit test for get timeline.

This should allow Flutter to remove their hacked up timeline service RPCs.
==========

to

==========
Add ability for embedder to provide private timeline trace data

- Add three new embedder hooks registered with Dart_SetEmbedderTimelineCallbacks
  - Dart_EmbedderTimelineStartRecording
  - Dart_EmbedderTimelineStopRecording
  - Dart_EmbedderTimelineGetTimeline

- Add unit test for record start / stop notification.
- Add unit test for get timeline.

This should allow Flutter to remove their hacked up timeline service RPCs.

R=chinmaygarde@google.com, rmacnak@google.com

Committed:
https://github.com/dart-lang/sdk/commit/53cae0aed917827e24e5ff5e267e2dae51610a36
==========

[Cutch, 2016-03-10 21:30:35]

Committed patchset #2 (id:20001) manually as
53cae0aed917827e24e5ff5e267e2dae51610a36 (presubmit successful).

[yjbanov, 2016-03-10 22:45:15]

yjbanov@google.com changed reviewers:
+ yjbanov@google.com

[yjbanov, 2016-03-10 22:45:16]

https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
File runtime/include/dart_tools_api.h (right):

https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
runtime/include/dart_tools_api.h:1097: * synchronously.
I'm not sure Flutter's implementation can complete synchronously. We need to
wait for the log file to be finalized some time after the call to
Dart_EmbedderTimelineStopRecording.

[Bill Hesse, 2016-03-11 11:42:12]

whesse@google.com changed reviewers:
+ whesse@google.com

[Bill Hesse, 2016-03-11 11:42:12]

This CL is failing compilation on MacOS, due to the ordering of the exported
function:

FAILED: ./gyp-mac-tool package-framework 'Chromium Framework.framework' A &&
(export
BUILT_PRODUCTS_DIR=/b/build/slave/dartium-mac-inc-be/build/src/out/Release;
export CONFIGURATION=Release; export CONTENTS_FOLDER_PATH="Chromium
Framework.framework/Versions/A"; export
DYLIB_INSTALL_NAME_BASE=@executable_path/../Versions/45.0.2454.0; export
EXECUTABLE_NAME="Chromium Framework"; export EXECUTABLE_PATH="Chromium
Framework.framework/Versions/A/Chromium Framework"; export
FULL_PRODUCT_NAME="Chromium Framework.framework"; export
INFOPLIST_PATH="Chromium Framework.framework/Versions/A/Resources/Info.plist";
export LD_DYLIB_INSTALL_NAME="@executable_path/../Versions/45.0.2454.0/Chromium
Framework.framework/Chromium Framework"; export MACH_O_TYPE=mh_dylib; export
PRODUCT_NAME="Chromium Framework"; export
PRODUCT_TYPE=com.apple.product-type.framework; export
SDKROOT=/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk;
export
SRCROOT=/b/build/slave/dartium-mac-inc-be/build/src/out/Release/../../chrome;
export SOURCE_ROOT="${SRCROOT}"; export
TARGET_BUILD_DIR=/b/build/slave/dartium-mac-inc-be/build/src/out/Release; export
TEMP_DIR="${TMPDIR}"; export UNLOCALIZED_RESOURCES_FOLDER_PATH="Chromium
Framework.framework/Versions/A/Resources"; export WRAPPER_NAME="Chromium
Framework.framework"; (cd ../../chrome && ../build/mac/tweak_info_plist.py
"--breakpad=1" "--breakpad_uploads=0" "--keystone=0" "--scm=1"
"--branding=Chromium" && ln -fns Versions/Current/Libraries
"${BUILT_PRODUCTS_DIR}/${WRAPPER_NAME}/Libraries" &&
tools/build/mac/verify_order _ChromeMain
"${BUILT_PRODUCTS_DIR}/${EXECUTABLE_PATH}"); G=$?; ((exit $G) || rm -rf
'Chromium Framework.framework') && exit $G) && touch 'Chromium
Framework.framework'
tools/build/mac/verify_order: unordered symbols in
/b/build/slave/dartium-mac-inc-be/build/src/out/Release/Chromium
Framework.framework/Versions/A/Chromium Framework:
_Dart_SetEmbedderTimelineCallbacks

[siva, 2016-03-11 12:56:17]

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

[siva, 2016-03-11 12:56:17]

https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
File runtime/include/dart_tools_api.h (right):

https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
runtime/include/dart_tools_api.h:1125: Dart_EmbedderTimelineGetTimeline
get_timeline);
Does it make sense for any of these callbacks to be NULL?
I am wondering if we could error out if any of the passed in callbacks are NULL,
that would avoid the need to do a NULL check before every invocation of the
callbacks.

[Cutch, 2016-03-11 16:16:23]

On 2016/03/11 11:42:12, Bill Hesse wrote:
> This CL is failing compilation on MacOS, due to the ordering of the exported
> function:
> 
> FAILED: ./gyp-mac-tool package-framework 'Chromium Framework.framework' A &&
> (export
> BUILT_PRODUCTS_DIR=/b/build/slave/dartium-mac-inc-be/build/src/out/Release;
> export CONFIGURATION=Release; export CONTENTS_FOLDER_PATH="Chromium
> Framework.framework/Versions/A"; export
> DYLIB_INSTALL_NAME_BASE=@executable_path/../Versions/45.0.2454.0; export
> EXECUTABLE_NAME="Chromium Framework"; export EXECUTABLE_PATH="Chromium
> Framework.framework/Versions/A/Chromium Framework"; export
> FULL_PRODUCT_NAME="Chromium Framework.framework"; export
> INFOPLIST_PATH="Chromium Framework.framework/Versions/A/Resources/Info.plist";
> export
LD_DYLIB_INSTALL_NAME="@executable_path/../Versions/45.0.2454.0/Chromium
> Framework.framework/Chromium Framework"; export MACH_O_TYPE=mh_dylib; export
> PRODUCT_NAME="Chromium Framework"; export
> PRODUCT_TYPE=com.apple.product-type.framework; export
>
SDKROOT=/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk;
> export
> SRCROOT=/b/build/slave/dartium-mac-inc-be/build/src/out/Release/../../chrome;
> export SOURCE_ROOT="${SRCROOT}"; export
> TARGET_BUILD_DIR=/b/build/slave/dartium-mac-inc-be/build/src/out/Release;
export
> TEMP_DIR="${TMPDIR}"; export UNLOCALIZED_RESOURCES_FOLDER_PATH="Chromium
> Framework.framework/Versions/A/Resources"; export WRAPPER_NAME="Chromium
> Framework.framework"; (cd ../../chrome && ../build/mac/tweak_info_plist.py
> "--breakpad=1" "--breakpad_uploads=0" "--keystone=0" "--scm=1"
> "--branding=Chromium" && ln -fns Versions/Current/Libraries
> "${BUILT_PRODUCTS_DIR}/${WRAPPER_NAME}/Libraries" &&
> tools/build/mac/verify_order _ChromeMain
> "${BUILT_PRODUCTS_DIR}/${EXECUTABLE_PATH}"); G=$?; ((exit $G) || rm -rf
> 'Chromium Framework.framework') && exit $G) && touch 'Chromium
> Framework.framework'
> tools/build/mac/verify_order: unordered symbols in
> /b/build/slave/dartium-mac-inc-be/build/src/out/Release/Chromium
> Framework.framework/Versions/A/Chromium Framework:
> _Dart_SetEmbedderTimelineCallbacks

Bill,

Do you have any idea how to fix this error? Standalone Mac is fine this is a
Chromium specific check.

[Cutch, 2016-03-11 16:17:59]

On 2016/03/11 12:56:17, siva wrote:
>
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> File runtime/include/dart_tools_api.h (right):
> 
>
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> runtime/include/dart_tools_api.h:1125: Dart_EmbedderTimelineGetTimeline
> get_timeline);
> Does it make sense for any of these callbacks to be NULL?
> I am wondering if we could error out if any of the passed in callbacks are
NULL,
> that would avoid the need to do a NULL check before every invocation of the
> callbacks.

These are optional callbacks that only the Flutter embedder will use. So, we
could make no-op impls and use those instead of NULL but I think it's cleaner as
is.

[Cutch, 2016-03-11 16:19:53]

On 2016/03/10 22:45:16, yjbanov wrote:
>
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> File runtime/include/dart_tools_api.h (right):
> 
>
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> runtime/include/dart_tools_api.h:1097: * synchronously.
> I'm not sure Flutter's implementation can complete synchronously. We need to
> wait for the log file to be finalized some time after the call to
> Dart_EmbedderTimelineStopRecording.

Chinmay- is this interface not sufficient for Flutter's needs? I was under the
impression it was.

[Chinmay, 2016-03-14 14:16:25]

On 2016/03/11 at 16:19:53, johnmccutchan wrote:
> On 2016/03/10 22:45:16, yjbanov wrote:
> >
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> > File runtime/include/dart_tools_api.h (right):
> > 
> >
https://codereview.chromium.org/1783953002/diff/20001/runtime/include/dart_to...
> > runtime/include/dart_tools_api.h:1097: * synchronously.
> > I'm not sure Flutter's implementation can complete synchronously. We need to
> > wait for the log file to be finalized some time after the call to
> > Dart_EmbedderTimelineStopRecording.
> 
> Chinmay- is this interface not sufficient for Flutter's needs? I was under the
impression it was.

I am pretty sure we can finalize log data synchronously (even if we have to
write to the log file from another thread and wait). I am going to attempt
wiring this up today.