This change reworks the client resume permissions API to make it easier
for clients to deal with user provided `--pause-isolates-on-start` and
`--pause-isolates-on-exit` flags.
`requireUserPermissionToResume` should be called by the tool that
launches the Dart process to indicate whether or not the user provided
`--pause-isolates-on-{start,exit}`. The default behavior is to assume
that a tool set these flags for its own use (e.g., resetting breakpoints
after a hot restart in Flutter), where isolates will resume immediately
after each client that has indicated interest in that pause event has
invoked `readyToResume`.
If a user provided one of the previously mentioned flags, isolates will
not immediately resume after each relevant client has invoked
`readyToResume`. Instead, a call to `resume()` must be made to indicate
the user has triggered the resume request instead of tooling. If the
user permissions to resume are changed while the isolate is paused and
all relevant clients have invoked `readyToResume`, the isolate will
automatically resume if the user no longer requires us to wait for a
user resume.
`resume()` now also acts as a "force resume", bypassing any required
permissions set by tooling.
This behavior change is breaking, so the DDS protocol version is being
bumped to 2.0.
`package:dds_service_extensions` has also been updated to include the
following DDS RPCs:
- `requireUserPermissionToResume`
- `readyToResume`
Change-Id: Id5f0806b3c56507d39eb00b6305b8896bab13ae7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357420
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This adds support for printing the DTD connection information to stdout
when --print-dtd-uri is passed.
This change also fixes an issue where DDS would fail to spawn an isolate
with the DTD snapshot when DDS was running in AOT mode. This means the
SDK must be shipped with both AppJIT and AOT DTD snapshots, at least
until dartdev is moved to run from AOT.
Fixes https://github.com/dart-lang/sdk/issues/55034
TEST=run_test.dart
Change-Id: I788ef9bfe76297a8d594992a2aac440ed9e2ecac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358541
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
The move to URIs introduced a subtle bug here - we would use the mapped URI (which is almost always `file:`) to decide whether to show a file path, instead of doing it only if the unmapped URI was a `file` (eg. it could have been `package:` or `dart:`.
This wasn't caught by any tests here, but was caught in the Flutter roll (https://github.com/flutter/flutter/pull/145235). This fixes it and adds a new test that we verify the Source names for local files, packages, and dart: URIs.
Change-Id: I282bf935b9fa4016abeafd556dfd368950ee9611
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357325
Commit-Queue: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
The VM service may now give us `dart-macro+package:` URIs in addition to normal `package:` URIs. These can be mapped to `dart-macro+file:` URIs in the same way that we map `package:` to `file:`.
If the client editor has advertised that they support our custom Dart URIs, we can send these through to the client and let them use the analyzer-provided local sources. Otherwise, we'll treat them like other unknown sources and download them from the VM.
Change-Id: If67523a615c66c469db7b1f52d281c63241eb859
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357143
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Fix an issue where DDS would fail to initialize when an isolate in the target process was unable to handle service requests (e.g., when executing FFI code or blocked on a system call).
Fixes b/323386606
Change-Id: I659ebaf750e2c800e9819809d1104e024cb059da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354681
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
- devtools server can now take the dtd uri as a parameter on startup
- dtdUri is passed to the server handler so that devtools can request it.
- FileService is implemented inside the Dart Tooling Daemon (dtd_impl)
- Added FileService.setProjectRoots
- Added unit tests and rpc exceptions
- on startup dtd now prints { uri, secret } so that the caller has the secret that allows them to call FileService.setProjectRoots
Fixes: https://github.com/dart-lang/sdk/issues/54790
Change-Id: I7a66aca2f8f06f6a611a46c727a9d5c2485fbe3e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346922
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Dan Chevalier <danchevalier@google.com>
This non-null assertion is an error. If any URIs are returned from the VM that are not `file://` URIs and also return `false` from `isResolvableUri` then this would fail, even though the return values allow `null`s and should be used.
I can't currently write a test for this because in theory there aren't any cases where would actually happen, but I triggered it due to another bug (https://github.com/dart-lang/sdk/issues/54824) and think it's better removed.
Also added some TODOs about some behaviour that may need to change as macro support progresses (it's not far enough for me to make these changes yet).
Change-Id: I4108963b6b88bbbff4bb47f69909391900b5d3ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350300
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
* Replace `.whereNotNull()` with `.nonNulls` which is now in Dart core.
* `.firstOrNull`, `.lastOrNull`, `.singleOrNull` and `.elementAtOrNull(i)` are also in Dart core and even under the same name, so simply drop the import of 'package:collection' whenever possible.
Bug: b/322159291
Change-Id: I81eb93426680dc78a608bbcc714eb26c0eca5c62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348260
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Oleh Prypin <oprypin@google.com>
The initial global evaluation support only worked when the open script was loaded into the VM as a `file:///` URI (eg. things in a `bin/` folder, and not a `lib/` folder). This is because we short-cut resolving file paths to `package:` URIs for most VM functionality (for example setting breakpoints) because it's unnecessary (the VM supports setting breakpoints with file:/// URIs even for `lib/`).
This change forces us to resolve paths to their resolved URIs (eg. `package:` where applicable) in the case where we're looking up a script for global evaluation.
We could enable this lookup for all cases (to remove the additional `force` flag here), but since it's called much more from the other path (breakpoints) we should add caching (which was more than I wanted to change for this fix).
Fixes https://github.com/Dart-Code/Dart-Code/issues/4932
Change-Id: I57a99ec3b7c726d9d120e6cda7d0b938fec397bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346400
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
I noticed a race while running from source today.. If you run a Dart CLI app we add --pause-isolates-on-exit to allow sending async output events before exiting, then we resume.
The stopped event is still propagated to the client (since there can be a delay between the stop and the resume while we wait for output events) and the client may ask for a StackTrace. If this arrives just after we resume the isolate, we would generate an error that could be shown to the user.
This adds a check that the thread ID is really invalid before throwing an error, and if it's valid (eg. it belongs to an earlier isolate) we just return an empty stack trace instead of an error.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4907
Change-Id: I0a3241ffd634acea49b4d1dc7b3cd6b6b1c8d2d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342585
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
We accidentally started showing the stack traces in all errors rendered in VS Code. This changes the format of the message to not include the stack - although it keeps it in the payload to aid debugging when capturing logs.
It also slightly tweaks the error message for global eval to be less cryptic for VS Code.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4899
Change-Id: Id4a0c4e6ef26065059e3becec62bab5d873e7be3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342621
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
+ add a failing test for https://github.com/dart-lang/sdk/issues/52632 / https://github.com/flutter/flutter/issues/137163
This does not resolve the issue, it only makes it clearer from the error message what the problem is. Unfortunately there is no good workaround when we hit this bug because we can't tell which items in the response are valid/invalid. If we treat them all as invalid (eg. complete all of the completes with null), many things that involve mappings of URIs<->files won't work correctly (breakpoints, opening the right file when we break, etc.).
A workaround is for users not to have commas in their filenames, although it's not clear if there may be other characters triggering this right now.
Change-Id: I6ac7cbd82b726fac76c27d1e00a06f2a07d4757f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342800
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
It's only valid to reuse a client that is not showing an embedded page, however we only get the embedded flag when a client sends a "currentPage" event.
There is a period between a client connecting and sending this event where we would consider it reusable when it's not. This fixes that by keeping a flag to indicate if a client has completed initializing (that is, it has sent its initial page).
See https://github.com/Dart-Code/Dart-Code/issues/4832
Change-Id: I9f2d43d1537ee97e4e231a844831e7548cf07beb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336041
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Some of these tests failed if `Directory.systemTemp` had a lowercase drive letter because DAP always normalises outbound paths to uppercase and we were comparing them directly.
This updates offending tests to compare with uppercase drive letters as is expected, regardless of the original temp directory (used for "cwd" and "program").
Fixes https://github.com/dart-lang/sdk/issues/53949
Change-Id: Ibaa6b6ecfe3d77aeff5b806174167c9452167db6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/334160
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This adds a check that scripts exist before we create a sourceReference to assume we can download them.
I don't have a good way to test this (in the Dart SDK) because this never happens for Dart - we can always download the scripts. However this code is triggered in Flutter because of https://github.com/flutter/flutter/issues/128880 - we can't find the source so we will try to set it up for downloading.
Fixing that would avoid the need for this change (and make the behaviour more consistent with Dart), but in the meantime I think it's better that we don't produce sourceReferences for sources that don't exist.
Change-Id: I4de3f6e87fe3ff867a1ff7e6d3a5d79f7bf9fa1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330780
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
To reduce the headache associated with adding parameters to VM service
APIs, VmServiceInterface has been removed from package:vm_service and
pulled into its own dedicated package:vm_service_interface. This will
help reduce the need for major version bumps of package:vm_service,
which requires manual version bumps through >8 packages in order to make
the latest version available to flutter_tools and DevTools.
This separation of the VmService client from the interface will reduce
the frequency of major version bumps to `package:vm_service` as adding
optional parameters to existing APIs would cause implementers of the
interface to break.
package:vm_service continues to expose a copy of the contents of package:vm_service_interface to avoid breaking google3 rolls until package:dwds can migrate to package:vm_service_interface. package:vm_service will not be published until this copy is removed.
This change also includes:
- some code cleanup and modernization to both the code generator and
generated code
- >=3.0.0 SDK version requirement to allow for new language features
Change-Id: Ib1859c1b4e153fef7ee1f91e67e881bbf42652c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330500
Reviewed-by: Derek Xu <derekx@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Normally in the debugger we return truncated values, but there are some cases where the user expects the full value. The legacy DAP handled these, but the new DAP did not.
With this change:
- "Copy Value" (context: clipboard) actions will always copy the full string (and without quotes)
- the REPL/Debug Console will include the full untruncated string
Fixes https://github.com/Dart-Code/Dart-Code/issues/4730
Change-Id: I6e43f8e21ca606d4ea1e9f1c30a111d7a4d82f20
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/326642
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This CL passes the offset and uri of the file (here called a script uri
as opposed to a library uri, the two will be different if we're in a
part) when doing expression compilation.
This CL only passes the data, but doesn't actually use it.
Future CL(s) will use this data to calculate the static type of
available variables which is needed for an upcomming feature.
TEST=Existing tests.
CoreLibraryReviewExempt: Not changing SDK APIs.
Change-Id: I67ead461ab4bb9341424e693946f3e4afe35ce92
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329322
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Ben Konyi