Add Dart entrypoint to execute the dart plugin registrant. #31720
Conversation
gaaclarke
changed the title
gaaclarke
force-pushed
the
background-isolate-dart-registrant
branch
6 times, most recently
from
6328d18 to
c6fa534
Compare
gaaclarke
force-pushed
the
background-isolate-dart-registrant
branch
from
c6fa534 to
cacef01
Compare
|
I need to add a negative test too, right now it just has a positive test. I'll add that this morning as well as the web_ui definitions. (edit: done) |
gaaclarke
force-pushed
the
background-isolate-dart-registrant
branch
from
6e6a684 to
39a0c3c
Compare
|
|
||
| /// Makes sure the that the Dart Plugin Registrant has been called for this | ||
| /// isolate. This can safely be executed multiple times on the same isolate, | ||
| /// but should not be called on the Root isolate. |
There was a problem hiding this comment.
Is the app developer responsible for calling this method?
There was a problem hiding this comment.
Yep, potentially plugin developers too.
There was a problem hiding this comment.
if the check _wasInitialized is moved to DartPluginRegistrant_EnsureInitialized, would it handle the root isolate case?
There was a problem hiding this comment.
Nope, that wouldn't allow it to be called from the background isolates which is what we want. If we did that we'd have to keep a map between the isolate and a bool to determine if it was called or not. That would be a bit involved since it would mean uniquely identifying the isolate some how and exposing that to c++.
There was a problem hiding this comment.
C++ side could only check for the root isolate, and the dart side for the custom isolate. Maybe the map is cleaner.
There was a problem hiding this comment.
If we think that this is a problem, the easiest solution would be to transfer the bool into the _PluginRegistrant and making the registrant idempotent.
There was a problem hiding this comment.
I filed an issue for it: flutter/flutter#99444
| /// Helper functions for Dart Plugin Registrants. | ||
| class DartPluginRegistrant { | ||
| /// Makes sure the that the Dart Plugin Registrant has been called for this | ||
| /// isolate. This can safely be executed multiple times on the same isolate, |
There was a problem hiding this comment.
what is an isolate in the web? a web worker? @yjbanov
| @pragma('vm:entry-point') | ||
| void callDartPluginRegistrantFromBackgroundIsolate() async { | ||
| ReceivePort receivePort = ReceivePort(); | ||
| Isolate isolate = await Isolate.spawn(dartPluginRegistrantIsolate, receivePort.sendPort); |
There was a problem hiding this comment.
It would be great if the Dart C++ API supported registering a callback that triggers whenever a new isolate is spawn.
The main benefit is that it would keep DartPluginRegistrant.ensureInitialized() as implementation detail.
There was a problem hiding this comment.
We want people to have the option of not calling the dart plugin registrant on background isolates though for performance reasons. If you are spawning an isolate to do background encryption you don't want to start executing whatever happens in whatever plugins you are using elsewhere in your app.
There was a problem hiding this comment.
right. Although, the registration phase in federated plugins is about binding to a specific implementation of the shared interface, so the cost might just be constructing that specific implementation object for the plugins that do have a Dart plugin registrant.
2 participants
fixes flutter/flutter#98591
This PR depends on flutter/flutter#98046.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.