Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register service extensions when the Service stream is listened to #2384

Closed
wants to merge 4 commits into from

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Mar 7, 2024

Work towards flutter/devtools#7313

Copy the VM service behavior of registering service extensions when a client subscribes to the Service stream: https://github.com/dart-lang/sdk/blob/main/sdk/lib/vmservice/vmservice.dart#L588

Not a complete fix, because DWDS' streamListen implementation is not being called in response to DevTools streamListen here: https://github.com/flutter/devtools/blob/20c66f853252bb7199c9bc2dafa17f3f28dee0d0/packages/devtools_app_shared/lib/src/service/service_manager.dart#L343

@elliette elliette changed the title Register service callbacks when the Service stream is listened to Register service extensions when the Service stream is listened to Mar 7, 2024
@elliette elliette requested a review from bkonyi March 7, 2024 23:32
@bkonyi
Copy link
Collaborator

bkonyi commented Mar 8, 2024

To be clear, the VM service's implementation doesn't actually register the extensions when the Service stream is listened to, it just posts ServiceRegistered events for each extension so the client knows which services are currently registered. In theory, the client doesn't have to listen to the Service stream and can get the list of service extensions from the Isolate object and invoke them that way.

Is there no way for DWDS to register these service extensions at startup?

@bkonyi
Copy link
Collaborator

bkonyi commented Mar 8, 2024

Ah, nevermind, we register them against each individual client... let's meet and discuss when you have a chance, I think I might be missing some context.

@elliette
Copy link
Contributor Author

More work is required here, closing this.

@elliette elliette closed this Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants