The `Match.operator[]` does the same thing and is
generally recommended (and shorter).
(I want to deprecate `group` and `groups`)
Tested: Refactoring.
CoreLibraryReviewExempt: Calling equivalent function.
Change-Id: I4c758968ae622fe16b7322be1b29b05b91e7fcd9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489021
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
When running a large test file with `solo: true` on one test, the output looks like:
```
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
! test name
Skip: does not have "solo"
✓ solo test
! test name
Skip: does not have "solo"
```
While custom skip messages might be useful, when the skip messages are from using solo:true this just makes the output spammy and adds extra noise to scan through for the actual solo test you ran.
This change prevents creating output events for such skip messages (but still sends them for custom messages, since those are likely far less frequent).
The `print` events themselves are still forwarded to the DAP client, so it still has the data for this that it can present elsewhere (for example by showing the test node as skipped, if appropriate).
Fixes part of https://github.com/Dart-Code/Dart-Code/issues/5938
Change-Id: I1d26751207e7b89f77e13be0dfcc74710e7d75ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/486780
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This response header entry prevents DevTools from being embedded in
iframes, which is needed to embed the DevTools inspector within the
Flutter widget preview environment.
TEST=Manually verified with `flutter widget-preview start`
Change-Id: Ic4ec3cf0e23509a6c0cbc94ab636a18c6abf733f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/468020
Auto-Submit: Ben Konyi <bkonyi@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
In order for "Copy Value" to work in VS Code's debugger windows, we need to provide an expression for each variable (the "evaluateName"). To carry this down through the child fields we need to store it by the object ID on the server, but we were not doing so for global fields - this resulted in "Copy Value" giving truncated values and "Copy Expression" being missing.
The fix is to ensure we call `storeEvaluateName` for globals too.
Fixes https://github.com/Dart-Code/Dart-Code/issues/5778
Change-Id: I9fed4b5483099341799548aea0225c9cef03d211
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459560
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Jessy Yameogo <yjessy@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
While investigating the flaky test at https://github.com/dart-lang/sdk/issues/60187#issuecomment-3431336127 I found that we had two paths into `handleThreadStartup` - one which explicitly sent a "pause-on-entry" event, and one that did not. The method sets a flag so it only ever runs once.
This means depending on which instance of `handleThreadStartup` occurs, the client may or may not get the pause-on-entry event.
The case where it would is where the isolate already had a pause-start event when we found it. The case where it would not, is if the isolate did not have a pause event (for example it was None because the isolate was just being created).
I can't come up with a good explanation for why we would try to pre-emptively run startup for an isolate we just discovered with no pauseEvent, so I have removed this branch on the assumption that we would always then see a `PauseStart` event for the isolate and then handled it through the same code path (and therefore, always sent the `pause-on-entry` event prior to calling readyToResume).
Fixes https://github.com/dart-lang/sdk/issues/60187
Change-Id: I213e9865480942b074d4d8470703377ea7ddb519
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/457040
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
A recent change somewhere caused some Dart-Code tests to fail. The expression "DateTime.now().ye" used to fail to compile and resulted in a failed RCP call to the VM service ("Expression compilation error") but now it returns a valid result that is a `vm.ErrorRef`.
This applies the same regex to the error message in this path to extract the most useful part of the error text when in the "watch" context so that the visible part of the message is not just "Unhandled exception".
Fixes https://github.com/Dart-Code/Dart-Code/issues/5759
Change-Id: I2933f9d811ec28446f4d2a857c1fd5bd18c94816
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/456241
Reviewed-by: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Due to variable shadowing and lazy initialization of the DevTools URI
within DDS, the DevTools URI reported by DDS did not include the `uri`
query parameter used to automatically establish connection to the
current DDS instance.
This change updates variable names to avoid shadowing and also fixes
issues where the DevTools URI would be reported even if DevTools was not
enabled in the DDS configuration.
Change-Id: I1ab0f5c58ee7e583e6eb4f4fb7381cddc29629b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448480
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Auto-Submit: Ben Konyi <bkonyi@google.com>
This reverts two commits (in separate patch sets in the CL):
- commit 482a7caed7: [dap] Simplify URI handling in IsolateManager.addBreakpoint
- commit d0a7ef4459: [dds/dap] Add/remove breakpoints as required instead of replacing the whole set
The first revert is to avoid conflicts while reverting the second, and will be reapplied later (likely after the release branch, since it is not critical to include). The second revert is because this change resulted in leaked Script objects in the VM.
There is an additional change (patch set 3) to fix up the changelog/versions so they don't go backwards.
Change-Id: Id55949e1622d3367ced87c9a307c43881b030cbb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440162
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
IsolateManager.addBreakpoint was forcing resolution of
google3:// URIs via lookupPackageUris vm-service call which is
not necessary.
addBreakpoint vm-service method is itself capable of handling
both original package:// URIs and their resolved forms (irrespective
of whether resolved form is file URI or a URI with a special
scheme introduced by multi-root filesystem).
Change-Id: Iedc74d0678ae020dd4689cb0f632e9cb941c5101
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/438880
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Helin Shiah <helinx@google.com>
- split the Dart CLI tool out of the VM into it's own embedder which
runs in AOT mode. The pure Dart VM executable is called 'dartvm' and
has no Dart CLI functionality in it
- the Dart CLI executable parses the CLI commands and invokes the rest
of the AOT tools in the same process, for the 'run' and 'test'
commands it execs a process which runs 'dartvm' to run
- 'dart hello.dart' execs the 'dartvm' process and runs 'hello.dart'
- the Dart CLI is not generated for ia32 as we are not shipping a
Dart SDK for ia32 anymore (support to execute the 'dartvm' for ia32
architecture is retained)
- the Dart CLI tool is not built in the internal Dart SDK builds
TEST=ci
Some performance improvement numbers
'dart format pkg/dartdev' goes from 1.17 secs to 0.22 secs
'dart doc pkg/dartdev' goes from 100.2 secs to 66.6 secs
'dart fix pkg/dartdev' goes from 19.3 secs to 14.5 secs
Change-Id: I66984a26cb2ab014b34dc1873f1f3d2884e13518
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364202
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
The DAP APIs for breakpoints send the whole set of breakpoints for a given file at once. Previously, we would just delete all breakpoints and then re-add them all, however since we added support for resolving breakpoints, this can result in all breakpoints in a file flickering to unresolved then back to resolved (as well as generally being slower).
This change splits the method that would replace all breakpoints into methods for add+remove, and then skips over any breakpoints (in setBreakpoints) that already match breakpoints we have.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4678
Change-Id: I489c6c295bac3ebd3a0851895a7a81e330d571e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/433700
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
TEST=pkg/vm_service/test/get_queued_microtasks_rpc_test
CoreLibraryReviewExempt: This CL does not include any core library API
changes, only VM Service implementation changes within
sdk/lib/vmservice/.
Change-Id: Ie5488f498e4d0f3d201e3f31423fd5029b74a726
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425160
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
The change at https://dart-review.googlesource.com/c/sdk/+/410760 to fix a race incorrectly assumed that once we had handled startup for a thread, we would never need to send an automatic resume again. However this was not the case - after a hot reload, we need to resume the thread even though we had technically already handled startup.
This is essentially a partial revert of 84e6ed0784, with a test to verify we trigger readyToResume on PausePostRequest.
Change-Id: I1e171043f04f38dd6f52e1692d9257d34e5248be
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416580
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
We start isolates paused so that we can send breakpoints before any code runs. This means we need to resume after initialization is complete.
It's important we don't try to resume multiple times during initialization (regardless of the order of isolate events or whether the isolate was found when we queried for isolates during connection).. This is done with the flag `startupHandled`.
One code path was not taking this flag into account, which meant multiple resumes were still possible. This seemed to occur on Linux during test runs (I've noticed the order of events being different on Linux in the past).
This change extracts the checking of `startupHandled` before sending `readyToResume` and uses it in all places that handle this kind of startup resume.
Fixes https://github.com/dart-lang/sdk/issues/60128
Change-Id: Ie2679fc806ab3edf007259298da82dbc8b802a6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410760
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Handler functions may return a Future<Response> that completes with an error. If that happens, then the top level error handling in the Shelf server will try to log the error to stderr.
But this will fail because DDS closes its stderr at startup. So Shelf will not write any response to the client, and the client will time out.
This PR adds a middleware to the DDS pipeline that receives errors thrown by the pipeline's handlers and returns a failure response.
Change-Id: I31beaf6abb0cbb42137c6b03f6081809de5d7119
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405681
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Jake Macdonald