dart-lang / webdev

A CLI for Dart web development.
https://pub.dev/packages/webdev
212 stars 75 forks source link

Help flutter register callbacks on chrome proxy service #1315

Open annagrin opened 3 years ago

annagrin commented 3 years ago

Flutter engine in native world registers _flutter.listViews extension on the VM. App registers extensions on the VM as well, and clients such as dart-code that are connected to flutter daemon call those extensions. Flutter daemon calls _flutter.listViews before proceeding to call methods registered by the app.

The web engine does not have information about the VM and its isolates so it cannot implement _flutter.listViews method that requires this information. For a short term solution, we are implementing _flutter.listViews directly in dwds, but it would be cleaner and less bug prone to implement a solution somewhere in flutter (in case that API changes, for example).

Possible solutions:

annagrin commented 3 years ago

@sigmundch @grouma what do you think about the solution options above, or any alternative solutions?

sigmundch commented 3 years ago

I'd admit I have little context here, so I don't have the full picture at the moment.

Could you comment on what is the purpose behind the listViews API?

I can see from the flutter_tools code base that it is often use to retrieve the enclosing isolate (and in the web we don't expect to have more than one). Is this the standard mechanism to register callbacks or does it have other purposes? If the idea is to detect the enclosing isolate in order to later register extensions, should we have an API to retrieve the web-isolate id directly?

Also, I noticed sprinkled through the flutter-tools code some if-then-elses to use a different protocol API for the web (example). That kind of code feels similar to your suggestion of having a between hook for registering extensions.

Could it be that the problem here is that getFlutterViews should only be used in the VM target and that other use-sites in flutter_tools need to be adapted?

sigmundch commented 3 years ago

OK reading through flutter/flutter#81516 I see the reference to the same location I found: https://github.com/flutter/flutter/blob/ea9d9ee9fdfe3301d10757842a0a63e358226c66/packages/flutter_tools/lib/src/resident_runner.dart#L408

And I see that they can work around it using this indirection, but they'd like it to be abstracted away.

A third idea would be to see if we can extend the service protocol to have a separate API like "getFlutterViewsIsolateIds" that bypasses the creation of the flutter views and directly returns the ids (since we see a lot of the sequence of getting views just to get the corresponding isolate ids as a next step). That would allow flutter_tools to use a single API that is consistent for both platforms

/cc @jacob314

annagrin commented 3 years ago

@sigmundch As far as I understand FlutterView is a ui isolate, so your third option could be just adding an VM Service API getUIIsolates()instead of being flutter specific. Not sure if VM knows what UI isolates are though. In web case we only have one isolate so the implementation is simple. /cc @bkonyi - does it seem like a good extension of the VM Service API?

@sigmundch Clarification - by my first suggestion I meant calling some JS function (that would return runtime info about VM service, including the vm uri) from https://github.com/dart-lang/sdk/blob/82ea8416ed2a22e40c5c67252cb37bebec3a57e8/sdk/lib/_internal/js_runtime/lib/developer_patch.dart#L102

This way the engine could find the information it needs from the vm service using Service.getInfo() call to get the vm uri: https://github.com/dart-lang/sdk/blob/82ea8416ed2a22e40c5c67252cb37bebec3a57e8/sdk/lib/developer/service.dart#L62

This would be similar to what happens in the native VM - does it sound possible? Also, does it sound too complicated?:)

bkonyi commented 3 years ago

@sigmundch As far as I understand FlutterView is a ui isolate, so your third option could be just adding an VM Service API getUIIsolates()instead of being flutter specific. Not sure if VM knows what UI isolates are though. In web case we only have one isolate so the implementation is simple. /cc @bkonyi - does it seem like a good extension of the VM Service API?

The VM doesn't know anything about UI isolates as that's solely a Flutter construct (Dart programs don't have UI), so this wouldn't be something we'd want to add to the service protocol. However, isolates can be named AFAIK which should make it possible to pick a UI isolate out of a list, assuming the engine is naming its isolates.

jacob314 commented 3 years ago

My preference would be to have a dart vm api the native flutter engine call to indicate than an isolate is a ui/main/event loop isolate and surface that information over the VM service. We currently guess that the very first isolate is the main/UI isolate in DevTools which is a bit error prone and is broken if there is more than one flutter view.

sigmundch commented 3 years ago

+1 on a more direct API, so that we are not "guessing" the data that the engine needs.

@annagrin - on your suggestion, what would the runtime "VM" info look like for the web?

annagrin commented 3 years ago

My preference would be to have a dart vm api the native flutter engine call to indicate than an isolate is a ui/main/event loop isolate and surface that information over the VM service. We currently guess that the very first isolate is the main/UI isolate in DevTools which is a bit error prone and is broken if there is more than one flutter view.

@jacob314 Can this be done with isolate naming that @bkonyi suggested? Anyway, we would still need to solve the problem on the web where the engine cannot currently call the VM APIs but can register extensions. Hence my suggestion of implementing the VM info in https://github.com/dart-lang/sdk/blob/82ea8416ed2a22e40c5c67252cb37bebec3a57e8/sdk/lib/_internal/js_runtime/lib/developer_patch.dart#L102

so the web engine can get a VM uri and call VM api, including naming or getting the isolates.

Alternatively, the web debugger will need to mark its only isolate as a ui/main/event loop isolate to match native flutter engine behavior - which seems too flutter-specific, for example, in case of a change of the marking.

annagrin commented 3 years ago

@annagrin - on your suggestion, what would the runtime "VM" info look like for the web?

@sigmundch it seems to be already defined in https://github.com/dart-lang/sdk/blob/82ea8416ed2a22e40c5c67252cb37bebec3a57e8/sdk/lib/developer/service.dart#L11

we would only actually need the VM uri, which (I think) could be stored as some runtime data by the debugger.