Send the isolate service ID from the engine to the embedder #9324

jason-simmons · 2019-06-14T01:11:50Z

Applications can use an embedder API to obtain the isolate ID and then use it in calls to the Dart service protocol.

tvolkert · 2019-06-14T01:21:52Z

Just curious: is this to enable Espresso's use case?

jason-simmons · 2019-06-14T01:24:25Z

Yes - this was requested by Espresso

chinmaygarde

This is susceptible to racy behavior when it comes to isolate initialization across threads as well the consumption of the isolate ID in Dart code on isolate launch.

I suggest using the root isolate create callback to fire the request to fetch the ID from Dart code and have the caller in Dart install a watcher. Also, please wire up a unit test in shell_unittests. I believe the fixtures necessary to test this are already in place.

chinmaygarde · 2019-06-14T02:22:59Z

shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java

+   * in queries to the Dart service protocol.
+   */
+  public String getIsolateServiceId() {
+    return isolateServiceId;


This is racy. How is the caller going to know to wait for the isolate launch to be completed?

With a getter API the client would need to poll until the isolate ID is available, or otherwise wait for another signal of app liveness such as the first frame callback. I'll add a callback specifically for the isolate ID.

chinmaygarde · 2019-06-14T02:25:41Z

shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java

  public DartExecutor(@NonNull FlutterJNI flutterJNI) {
    this.flutterJNI = flutterJNI;
    this.messenger = new DartMessenger(flutterJNI);
+    messenger.setMessageHandler("flutter/isolate", isolateChannelMessageHandler);


This is racy to changes in the engine that affect isolate initialization. This executor is initialized on the platform thread while the engine is racing to creating the isolate on the UI thread. It the engine wins, the message get dropped on the floor.

Not sure I understand the concern here. In the embedder DartExecutor will be created and the message handler will be installed at FlutterNativeView/FlutterEngine construction time. This will happen before the app can call runBundle and start the isolate.

(Note that the message containing the isolate ID is being sent to the platform host side, not to Dart code)

matthew-carroll · 2019-06-14T06:59:45Z

shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java

+    return isolateServiceId;
+  }
+
+  private BinaryMessenger.BinaryMessageHandler isolateChannelMessageHandler =


Would you mind moving this up in between the field declarations and the constructor declaration? And can you mark this as final?

matthew-carroll · 2019-06-14T07:01:01Z

shell/platform/android/io/flutter/embedding/engine/dart/DartExecutor.java

+      new BinaryMessenger.BinaryMessageHandler() {
+        @Override
+        public void onMessage(ByteBuffer message, final BinaryReply callback) {
+          isolateServiceId = StringCodec.INSTANCE.decodeMessage(message);


I'm wondering if we need a listener for this? The value is not available immediately upon construction, but instead appears an unknown time later. I'm not sure how clients will know, in general, when they can query this...but I can always add that later if we're not sure if we need it.

Added a listener interface

matthew-carroll

Thanks for putting this together.

Applications can use an embedder API to obtain the isolate ID and then use it in calls to the Dart service protocol.

jason-simmons · 2019-06-14T21:05:11Z

Added an embedder unit test - PTAL

…lutter/engine#9324)

flutter/engine@20d3861...05c034e git log 20d3861..05c034e --no-merges --oneline 05c034e Update component manifests for ambient replace-as-executable (flutter/engine#9350) 66022ce Roll src/third_party/skia 55091020435c..4b203ad7ac02 (6 commits) (flutter/engine#9352) fcff2d6 Use the DartServiceIsolate status callback to publish the observatory URI to the Android embedder (flutter/engine#9337) ea7ca98 Send the isolate service ID from the engine to the embedder (flutter/engine#9324) 675033f Check for invalid indexes when performing InputAdpator backspace. (flutter/engine#9322) e5918d1 Roll src/third_party/skia 92b81e14d9c2..55091020435c (6 commits) (flutter/engine#9348) e00ac47 Reorganize darwin for shared ios/macOS (flutter/engine#9255) 9da409c Fix crash on Huawei device with AndroidView (flutter/engine#9192) a0f8554 Removed an unused class definition for iOS code. (flutter/engine#9346) 96a1a84 Replace lock_guard with scoped_lock and use class template argument deduction. (flutter/engine#9338) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (liyuqian@google.com), and stop the roller if necessary.

…lutter/engine#9324)

flutter/engine@20d3861...05c034e git log 20d3861..05c034e --no-merges --oneline 05c034e Update component manifests for ambient replace-as-executable (flutter/engine#9350) 66022ce Roll src/third_party/skia 55091020435c..4b203ad7ac02 (6 commits) (flutter/engine#9352) fcff2d6 Use the DartServiceIsolate status callback to publish the observatory URI to the Android embedder (flutter/engine#9337) ea7ca98 Send the isolate service ID from the engine to the embedder (flutter/engine#9324) 675033f Check for invalid indexes when performing InputAdpator backspace. (flutter/engine#9322) e5918d1 Roll src/third_party/skia 92b81e14d9c2..55091020435c (6 commits) (flutter/engine#9348) e00ac47 Reorganize darwin for shared ios/macOS (flutter/engine#9255) 9da409c Fix crash on Huawei device with AndroidView (flutter/engine#9192) a0f8554 Removed an unused class definition for iOS code. (flutter/engine#9346) 96a1a84 Replace lock_guard with scoped_lock and use class template argument deduction. (flutter/engine#9338) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (liyuqian@google.com), and stop the roller if necessary.

googlebot added cla: yes labels Jun 14, 2019

jason-simmons force-pushed the executor_isolate_id branch from c43cc66 to 95c07f7 Compare June 14, 2019 01:14

jason-simmons requested review from chinmaygarde and matthew-carroll June 14, 2019 01:15

chinmaygarde suggested changes Jun 14, 2019

View reviewed changes

matthew-carroll reviewed Jun 14, 2019

View reviewed changes

jason-simmons force-pushed the executor_isolate_id branch from 95c07f7 to 5c5395b Compare June 14, 2019 17:23

matthew-carroll approved these changes Jun 14, 2019

View reviewed changes

Send the isolate service ID from the engine to the embedder

e5ad3a7

Applications can use an embedder API to obtain the isolate ID and then use it in calls to the Dart service protocol.

jason-simmons force-pushed the executor_isolate_id branch from 5c5395b to e5ad3a7 Compare June 14, 2019 21:04

chinmaygarde approved these changes Jun 17, 2019

View reviewed changes

jason-simmons merged commit ea7ca98 into flutter:master Jun 17, 2019

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

d15e77d

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 18, 2019

Roll engine 20d3861ac8e1..05c034e5bb0a (10 commits) flutter/flutter#34621

Merged

matthew-carroll mentioned this pull request Jun 18, 2019

Add APIs for querying FlutterView for a FlutterEngine and listening for attachment/detachment (#29114). #9356

Merged

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..4c9fa4940877 (24 commits) flutter/flutter#34697

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

bfb1d88

…lutter/engine#9324)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

aad8fbb

…lutter/engine#9324)

This was referenced Jun 19, 2019

Roll engine 20d3861ac8e1..c764a7628f6f (25 commits) flutter/flutter#34702

Closed

Roll engine 20d3861ac8e1..3f7a9f09ff35 (26 commits) flutter/flutter#34707

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

5f5a547

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..f75e7228faf4 (27 commits) flutter/flutter#34715

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

525ef8b

…lutter/engine#9324)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

8147c36

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..a263abfba0b3 (28 commits) flutter/flutter#34722

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

ae25d1e

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..9ab6550b45cd (29 commits) flutter/flutter#34730

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

1926ef1

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..69b32a9c34a0 (30 commits) flutter/flutter#34733

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2019

ea7ca98 Send the isolate service ID from the engine to the embedder (f…

0e82ffc

…lutter/engine#9324)

engine-flutter-autoroll mentioned this pull request Jun 19, 2019

Roll engine 20d3861ac8e1..54f88ab5da4d (31 commits) flutter/flutter#34740

Closed

Send the isolate service ID from the engine to the embedder #9324

Send the isolate service ID from the engine to the embedder #9324

Uh oh!

Conversation

jason-simmons commented Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tvolkert commented Jun 14, 2019

Uh oh!

jason-simmons commented Jun 14, 2019

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthew-carroll left a comment

Choose a reason for hiding this comment

Uh oh!

jason-simmons commented Jun 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jason-simmons commented Jun 14, 2019 •

edited

Loading