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.

Sign up for GitHub

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

Jump to bottom

Register service extensions on a client that is connected to DDS #2388

Merged
merged 42 commits into from
May 8, 2024
Merged

Register service extensions on a client that is connected to DDS #2388

merged 42 commits into from
May 8, 2024

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Mar 19, 2024 •
edited
Loading

Fixes flutter/devtools#7313

Fixes an issue where, when Dart DevTools reconnects to a web app, it is no longer aware of any of the registered service extensions.

Why was this happening?

  • In DWDS, all service extensions (including hot-restart) are registered on a client that is directly connected to the VmService, instead of connected to DDS
  • When a user re-connects to a web app from DevTools, DevTools calls streamListen on the Service stream
    DDS intercepts the streamListen request (so it never gets to DWDS) and sends back all of the service extensions it is aware of
  • However, because DWDS registers the service extensions on a client that is directly connected to the VMService, DDS is unaware of the service extensions and therefore can’t send them back.

What is the fix?

@elliette elliette requested a review from bkonyi March 19, 2024 21:27
bkonyi
bkonyi approved these changes Mar 20, 2024
View reviewed changes
Copy link
Collaborator

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. Also, package:vm_service_interface 1.1.0 has been published, so you can update the pubspec and submit when you're ready.

dwds/lib/src/services/chrome_proxy_service.dart Outdated Show resolved Hide resolved
dwds/lib/src/dwds_vm_client.dart Outdated Show resolved Hide resolved
@elliette elliette mentioned this pull request Mar 25, 2024
Merged
@elliette elliette mentioned this pull request Mar 25, 2024
Open
@elliette elliette mentioned this pull request May 1, 2024
Closed
@elliette elliette mentioned this pull request May 6, 2024
Merged
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 7, 2024
@elliette
[flutter web] Listen for service extension registration events to det…
b487b8c 
…ermine hot-restart method name (#147897)

### Some background:

* Flutter registers the hot-restart service extension for all devices **except** web devices. For web devices, DWDS registers the service extensions.
* When a user is debugging their code via VS Code, the VS Code Dart extension [sends a hot-restart](https://github.com/Dart-Code/Dart-Code/blob/94cb81c5526adc7c8a1203cba201e8c1d99adf82/src/debug/run_daemon_base.ts#L100) request via stdin/out to flutter_tools
* flutter_tools then [calls the "hotRestart" service extension](https://github.com/flutter/flutter/blob/f3978c7a46f083c8e91690b8b8be3e03b1e38155/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart#L447) (which, again, has been registered by DWDS)

### Why is this change necessary?

In DWDS, we are changing how we register the "hotRestart" service extension (here is the PR for that: dart-lang/webdev#2388). Previously, we registered the "hotRestart" service extension on a client that was directly connected to the VmService. With dart-lang/webdev#2388, we will be registering the "hotRestart" service extension on a client that is connected to DDS. 

When a service extension is registered against DDS, DDS adds a prefix to the service extension method name (e.g. "hotRestart" becomes "s0.hotRestart"). It informs clients of the service extension name via `kServiceRegistered` events sent to the `Service` stream. 

Therefore, this change simply listens to those service extension registered events, and uses them to determine the "hotRestart" service extension's method name.
@elliette elliette merged commit 239cb7f into dart-lang:master May 8, 2024
47 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request May 13, 2024
@devoncarew
[deps] rev dartdoc, ecosystem, fixnum, glob, html, http, http_multi_s…
d4f3288 
…erver, http_parser, json_rpc_2, markdown, matcher, mockito, package_config, path, pool, pub_semver, shelf, test, test_descriptor, test_process, test_reflective_loader, web, web_socket_channel, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/6e9c1ea..2e706be):
  2e706be8  2024-05-10  Sam Rawlins  Fix bug around ref resolution of type mentioned in type arguments (dart-lang/dartdoc#3768)
  d4a5f0b3  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/dartdoc#3766)

ecosystem (https://github.com/dart-lang/ecosystem/compare/9ce560b..5890417):
  5890417  2024-05-10  Parker Lougheed  Update dart_flutter_team_lints to depend on lints v4 (dart-lang/ecosystem#263)

fixnum (https://github.com/dart-lang/fixnum/compare/2c9f25d..ac892ad):
  ac892ad  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/fixnum#128)

glob (https://github.com/dart-lang/glob/compare/44e8c22..ee48ea8):
  ee48ea8  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/glob#93)
  c6362fb  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/glob#92)

html (https://github.com/dart-lang/html/compare/44613e8..00d3461):
  00d3461  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/html#245)
  e676b96  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/html#244)

http (https://github.com/dart-lang/http/compare/dd31e64..fdb0d36):
  fdb0d36  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http#1194)

http_multi_server (https://github.com/dart-lang/http_multi_server/compare/e45a674..4a791af):
  4a791af  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/http_multi_server#69)
  ff49aed  2024-05-06  Devon Carew  blast_repo fixes (dart-lang/http_multi_server#68)

http_parser (https://github.com/dart-lang/http_parser/compare/ad05810..702698a):
  702698a  2024-05-06  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/http_parser#91)
  d900d8e  2024-05-06  Devon Carew  blast_repo fixes (dart-archive/http_parser#90)

json_rpc_2 (https://github.com/dart-lang/json_rpc_2/compare/7547bb1..3187f7b):
  3187f7b  2024-05-08  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/json_rpc_2#114)
  94b6c49  2024-05-08  Devon Carew  blast_repo fixes (dart-archive/json_rpc_2#113)

markdown (https://github.com/dart-lang/markdown/compare/6aaa152..7463999):
  7463999  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/markdown#610)
  a2313a5  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/markdown#609)

matcher (https://github.com/dart-lang/matcher/compare/b42bf10..4ac4096):
  4ac4096  2024-05-07  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/matcher#248)
  f22df8a  2024-05-07  Devon Carew  blast_repo fixes (dart-archive/matcher#247)

mockito (https://github.com/dart-lang/mockito/compare/81ecb88..4be52e1):
  4be52e1  2024-05-07  Devon Carew  blast_repo fixes (dart-lang/mockito#749)

package_config (https://github.com/dart-lang/package_config/compare/a36e496..3909676):
  3909676  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/package_config#154)
  ca40813  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/package_config#153)

path (https://github.com/dart-lang/path/compare/f411b96..9be79e7):
  9be79e7  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-archive/path#164)
  283f31a  2024-05-09  Devon Carew  blast_repo fixes (dart-archive/path#163)

pool (https://github.com/dart-lang/pool/compare/e6df05a..1a6f2df):
  1a6f2df  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pool#86)
  f2f8a8b  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pool#85)

pub_semver (https://github.com/dart-lang/pub_semver/compare/df834e1..f57c9c3):
  f57c9c3  2024-05-09  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/pub_semver#103)
  2071731  2024-05-09  Devon Carew  blast_repo fixes (dart-lang/pub_semver#102)

shelf (https://github.com/dart-lang/shelf/compare/c8d8058..d9f82bf):
  d9f82bf  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/shelf#431)
  96d7283  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/shelf#430)
  468e2d9  2024-05-06  Brian Quinlan  Remove `dependency_overrides` (dart-lang/shelf#429)
  9e57c33  2024-05-06  Brian Quinlan  Prepare shelf_web_socket 2.0 for release (dart-lang/shelf#425)

test (https://github.com/dart-lang/test/compare/0562baf..84d2a2b):
  84d2a2bc  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test#2220)
  b7fa27d5  2024-05-07  Nate Bosch  Point README links to the test library (dart-lang/test#2219)
  d1e27677  2024-05-06  Brian Quinlan  Allow the next semver major versions of shelf_web_socket and web_socket_channel (dart-lang/test#2204)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/6695954..d61bf6c):
  d61bf6c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_descriptor#66)
  2861ba3  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_descriptor#65)

test_process (https://github.com/dart-lang/test_process/compare/69c6894..4ab3f1c):
  4ab3f1c  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_process#58)
  6efaf7f  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_process#57)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/537ab5b..f8807e0):
  f8807e0  2024-05-10  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.5 in the github-actions group (dart-lang/test_reflective_loader#62)
  1c62cd9  2024-05-10  Devon Carew  blast_repo fixes (dart-lang/test_reflective_loader#61)

web (https://github.com/dart-lang/web/compare/ff7185c..c36d962):
  c36d962  2024-05-12  mnordine  Fix typo (dart-lang/web#236)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/d86313d..5f65bae):
  5f65bae  2024-05-06  Brian Quinlan  Remove dependency_overrides (dart-lang/web_socket_channel#354)
  324d932  2024-05-06  Brian Quinlan  Make shelf_web_socket a dev dependency (dart-lang/web_socket_channel#353)
  0eca60d  2024-05-06  Brian Quinlan  Remove "wip" label for 3.0 release (dart-lang/web_socket_channel#347)

webdev (https://github.com/dart-lang/webdev/compare/c233e45..9108903):
  91089031  2024-05-10  Elliott Brooks  Update CONTRIBUTING doc for DWDS (dart-lang/webdev#2429)
  635b0d9d  2024-05-10  Elliott Brooks  Delete MV2 Dart Debug Extension (dart-lang/webdev#2428)
  ae43e8e9  2024-05-09  Elliott Brooks  Remove all unsound tests (dart-lang/webdev#2426)
  239cb7f3  2024-05-08  Elliott Brooks  Register service extensions on a client that is connected to DDS  (dart-lang/webdev#2388)

Change-Id: Ic0de7f6e41b4e93e3d26b7d1a74fe012bc7b086e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366161
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkonyi bkonyi bkonyi approved these changes

Assignees
No one assigned
Labels
package:dwds package:webdev
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hot-restart button is lost when connecting to a new web app
2 participants
@elliette @bkonyi