-
Notifications
You must be signed in to change notification settings - Fork 6k
Allow embedders to specify arbitrary data to the isolate on launch. #13047
Conversation
fml/mapping.cc
Outdated
| } | ||
|
|
||
| DataMapping::DataMapping(const std::string& string) | ||
| : DataMapping(CreateVectorFromString(string)) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be reduced to data_(string.begin(), string.end())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
lib/ui/window.dart
Outdated
| /// This data is persistent for the duration of the Flutter application and is | ||
| /// available even after isolate restarts. Because of this lifecycle, the size | ||
| /// of this data must be kept to a minimum and platform channels used for | ||
| /// communication that does not require synchronous embedder to isolate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be easier to understand if the part about platform channels was split into another sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
lib/ui/window/window.cc
Outdated
| return; | ||
| } | ||
|
|
||
| auto data_handle = Dart_NewTypedData(Dart_TypedData_kByteData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reuse ToByteDatahere. ToByteData can use void* and length arguments, and you can add wrappers that take an fml::Mapping or vector
Tonic's DartByteData class might also be a good place for this helper function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
Addressed all comments. |
4ee9572 to
6314042
Compare
Since this is currently only meant to be used by the embedding internally, the setter in Objective-C is only exposed via the FlutterDartProject private class extension. Unit tests have been added to the shell_unittests harness. Fixes flutter/flutter#37641
7096aa6 to
dada016
Compare
git@github.com:flutter/engine.git/compare/5162413111b8...cef6751 git log 5162413..cef6751 --no-merges --oneline 2019-10-10 chinmaygarde@gmail.com Revert "Test child isolates are terminated when root is shutdown (#13048)" (flutter/engine#13067) 2019-10-10 garyq@google.com Test child isolates are terminated when root is shutdown (flutter/engine#13048) 2019-10-10 chinmaygarde@google.com Allow embedders to specify arbitrary data to the isolate on launch. (flutter/engine#13047) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
git@github.com:flutter/engine.git/compare/5162413111b8...cef6751 git log 5162413..cef6751 --no-merges --oneline 2019-10-10 chinmaygarde@gmail.com Revert "Test child isolates are terminated when root is shutdown (flutter#13048)" (flutter/engine#13067) 2019-10-10 garyq@google.com Test child isolates are terminated when root is shutdown (flutter/engine#13048) 2019-10-10 chinmaygarde@google.com Allow embedders to specify arbitrary data to the isolate on launch. (flutter/engine#13047) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
Since this is currently only meant to be used by the embedding internally, the setter in Objective-C is only exposed via the FlutterDartProject private class extension. Unit tests have been added to the shell_unittests harness.
Fixes flutter/flutter#37641