[CP] Fixes for web debugging when using the new DAP

[nshahan]

Commit(s) to merge

389d66a, 67e052d

Target

beta

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/348186, https://dart-review.googlesource.com/c/sdk/+/348205

Issue Description

The new DAP used in communication during web debugging is calling previously unused APIs to show the values of getters in the debugger. This caused expression evaluation errors when trying to show .runtimeType.

What is the fix

Update how runtime type objects are labeled and queried to avoid errors when they are displayed in developer tools.

Why cherry-pick

The dart debugger and vscode were triggering errors for every variable in scope by automatically trying to show the .runtimeType getter. These changes fix those errors.

Risk

low

Issue link(s)

#54694, dart-lang/webdev#2343, dart-lang/webdev#2351

Extra Info

No response

[nshahan]

@annagrin Please feel free to correct my descriptions if needed.

[nshahan]

cc @sigmundch

[sigmundch]

The risk is extremely low, both CLs modify only a couple lines in debugger.dart: fix 1 and fix 2. Based on that alone, I'm happy to support the cherry-picks. That said, I also want to check in more detail how necessary these are and what's the corresponding impact.

To be clear - the related dwds fixes are important to cherry-pick in flutter (which is a separate process). Errors on every .runtimeType and .hashCode property are too visible to ignore and will degrade the user experience visibly with the DAP rollout.

Circling back here. I sense these two SDK changes may be less critical. In particular:

• It seems fix 1 is only needed if users ever have a record variable to inspect. Is that correct? Is this a regression from where we are today?

• It seems fix 2 is not going to be visible unless we revert the RTI back to the old representation in DDC. Is that correct?

Based on the low risk and the polish value added, I'd be inclined to cherry-pick cl/348186. Assuming we don't need it yet, I'd be inclined to hold off and not cherry-pick cl/348205.

@nshahan @annagrin - what are your thoughts?

[nshahan]

@elliette Could you comment on the visibility of these issues as well?

[elliette]

Some context here:

The new SDK DAPs send evaluate requests for variables, which DWDS never supported. This is fixed in dart-lang/webdev#2343, and will be cherry-picked into Flutter here: flutter/flutter#142169

When working on dart-lang/webdev#2343, @annagrin discovered that RecordTypes were still incorrect, and required these SDK changes. See comments starting here: dart-lang/webdev#2309 (comment)

So the commits proposed here will fix the display of RecordTypes, whereas the DWDS cherrypick to Flutter will fix the display of getters more generally.

[sigmundch]

Thanks! That matches my understanding.

I'm supportive of cherry-picking the fix to the new rti to display RecordTypes properly, but not bother with the one of the old RTI unless we end up needing it.