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

Prepare DWDS 23.0.0+1 hotfix #2355

Merged
merged 4 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 23.0.0+1

- Filter out internal type properties from the new DDC type system. - [#2348](https://github.com/dart-lang/webdev/pull/2348)
- Fix errors on displaying getters when debugging in VS Code. - [#2343](https://github.com/dart-lang/webdev/pull/2343)

## 23.0.0
- Restructure `LoadStrategy` to provide build settings. - [#2270](https://github.com/dart-lang/webdev/pull/2270)
- Add `FrontendServerLegacyStrategyProvider` and update bootstrap generation logic for `LegacyStrategy` - [#2285](https://github.com/dart-lang/webdev/pull/2285)
Expand Down
4 changes: 2 additions & 2 deletions dwds/lib/src/debugging/dart_scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ final ddcTemporaryTypeVariableRegExp = RegExp(r'^__t[\$\w*]+$');
final previousDdcTemporaryVariableRegExp =
RegExp(r'^(t[0-9]+\$?[0-9]*|__t[\$\w*]+)$');

/// Find the visible Dart properties from a JS Scope Chain, coming from the
/// Find the visible Dart variables from a JS Scope Chain, coming from the
/// scopeChain attribute of a Chrome CallFrame corresponding to [frame].
///
/// See chromedevtools.github.io/devtools-protocol/tot/Debugger#type-CallFrame.
Future<List<Property>> visibleProperties({
Future<List<Property>> visibleVariables({
required AppInspectorInterface inspector,
required WipCallFrame frame,
}) async {
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ class Debugger extends Domain {
Future<List<BoundVariable>> variablesFor(WipCallFrame frame) async {
// TODO(alanknight): Can these be moved to dart_scope.dart?
final properties =
await visibleProperties(inspector: inspector, frame: frame);
await visibleVariables(inspector: inspector, frame: frame);
final boundVariables = await Future.wait(
properties.map(_boundVariable),
);
Expand Down
9 changes: 9 additions & 0 deletions dwds/lib/src/debugging/inspector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,18 @@ class AppInspector implements AppInspectorInterface {
);
return jsProperties
.map<Property>((each) => Property(each as Map<String, dynamic>))
.where(_isVisibleProperty)
.toList();
}

bool _isVisibleProperty(Property property) {
// Filter out any RTI objects from the new DDC type system. See:
// https://github.com/dart-lang/webdev/issues/2316
final isRtiObject =
property.value?.className?.startsWith('dart_rti.Rti') ?? false;
return !isRtiObject;
}

/// Calculate the number of available elements in the range.
static int _calculateRangeCount({
int? count,
Expand Down
78 changes: 37 additions & 41 deletions dwds/lib/src/debugging/instance.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,29 +238,49 @@ class InstanceHelper extends Domain {
final objectId = remoteObject.objectId;
if (objectId == null) return null;

final fields = await _getInstanceFields(
metaData,
remoteObject,
offset: offset,
count: count,
);

final result = Instance(
kind: InstanceKind.kPlainInstance,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef,
fields: fields,
);
return result;
}

Future<List<BoundField>> _getInstanceFields(
ClassMetaData metaData,
RemoteObject remoteObject, {
int? offset,
int? count,
}) async {
final objectId = remoteObject.objectId;
if (objectId == null) throw StateError('Object id is null for instance');

final properties = await inspector.getProperties(
objectId,
offset: offset,
count: count,
length: metaData.length,
);

final dartProperties = await _dartFieldsFor(properties, remoteObject);
var boundFields = await Future.wait(
final boundFields = await Future.wait(
dartProperties
.map<Future<BoundField>>((p) => _fieldFor(p, metaData.classRef)),
);
boundFields = boundFields

return boundFields
.where((bv) => inspector.isDisplayableObject(bv.value))
.toList()
..sort(_compareBoundFields);
final result = Instance(
kind: InstanceKind.kPlainInstance,
id: objectId,
identityHashCode: remoteObject.objectId.hashCode,
classRef: metaData.classRef,
fields: boundFields,
);
return result;
}

int _compareBoundFields(BoundField a, BoundField b) {
Expand Down Expand Up @@ -700,7 +720,13 @@ class InstanceHelper extends Domain {
final objectId = remoteObject.objectId;
if (objectId == null) return null;

final fields = await _typeFields(metaData.classRef, remoteObject);
final fields = await _getInstanceFields(
metaData,
remoteObject,
offset: offset,
count: count,
);

return Instance(
identityHashCode: objectId.hashCode,
kind: InstanceKind.kType,
Expand All @@ -714,36 +740,6 @@ class InstanceHelper extends Domain {
);
}

/// The field types for a Dart RecordType.
///
/// Returns a range of [count] field types, if available, starting from
/// the [offset].
///
/// If [offset] is `null`, assumes 0 offset.
/// If [count] is `null`, return all field types starting from the offset.
Future<List<BoundField>> _typeFields(
ClassRef classRef,
RemoteObject type,
) async {
// Present the type as an instance of `core.Type` class and
// hide the internal implementation.
final expression = _jsRuntimeFunctionCall('getTypeFields(this)');

final result = await inspector.jsCallFunctionOn(type, expression, []);
final hashCodeObject = await inspector.loadField(result, 'hashCode');
final runtimeTypeObject = await inspector.loadField(result, 'runtimeType');

final properties = [
Property({'name': 'hashCode', 'value': hashCodeObject}),
Property({'name': 'runtimeType', 'value': runtimeTypeObject}),
];

final boundFields = await Future.wait(
properties.map<Future<BoundField>>((p) => _fieldFor(p, classRef)),
);
return boundFields;
}

/// Return the available count of elements in the requested range.
/// Return `null` if the range includes the whole object.
/// [count] is the range length requested by the `getObject` call.
Expand Down
1 change: 1 addition & 0 deletions dwds/lib/src/debugging/metadata/provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class MetadataProvider {
'dart:_js_primitives',
'dart:_metadata',
'dart:_native_typed_data',
'dart:_rti',
'dart:async',
'dart:collection',
'dart:convert',
Expand Down
41 changes: 39 additions & 2 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,35 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
await isCompilerInitialized;
_checkIsolate('evaluate', isolateId);

final library = await inspector.getLibrary(targetId);
late Obj object;
try {
object = await inspector.getObject(targetId);
} catch (_) {
return ErrorRef(
kind: 'error',
message: 'Evaluate is called on an unsupported target:'
'$targetId',
id: createId(),
);
}

final library =
object is Library ? object : inspector.isolate.rootLib;

if (object is Instance) {
// Evaluate is called on a target - convert this to a dart
// expression and scope by adding a target variable to the
// expression and the scope, for example:
//
// Library: 'package:hello_world/main.dart'
// Expression: 'hashCode' => 'x.hashCode'
// Scope: {} => { 'x' : targetId }

final target = _newVariableForScope(scope);
expression = '$target.$expression';
scope = (scope ?? {})..addAll({target: targetId});
}

return await _getEvaluationResult(
isolateId,
() => evaluator.evaluateExpression(
Expand All @@ -631,7 +659,7 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
);
}
throw RPCError(
'evaluateInFrame',
'evaluate',
RPCErrorKind.kInvalidRequest.code,
'Expression evaluation is not supported for this configuration.',
);
Expand All @@ -640,6 +668,15 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
);
}

String _newVariableForScope(Map<String, String>? scope) {
// Find a new variable not in scope.
var candidate = 'x';
while (scope?.containsKey(candidate) ?? false) {
candidate += '\$1';
}
return candidate;
}

@override
Future<Response> evaluateInFrame(
String isolateId,
Expand Down
59 changes: 43 additions & 16 deletions dwds/test/evaluate_common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ void testAll({
await getInstanceRef(event.topFrame!.index!, 'stream');
final instance = await getInstance(instanceRef);

expect(instance, matchInstance('_AsBroadcastStream<int>'));
expect(instance, matchInstanceClassName('_AsBroadcastStream<int>'));
});
});

Expand Down Expand Up @@ -667,24 +667,24 @@ void testAll({
tearDown(() async {});

evaluate(
libraryId,
targetId,
expr, {
scope,
}) async =>
await context.service.evaluate(
isolateId,
libraryId,
targetId,
expr,
scope: scope,
);

getInstanceRef(
libraryId,
targetId,
expr, {
scope,
}) async {
final result = await evaluate(
libraryId,
targetId,
expr,
scope: scope,
);
Expand All @@ -698,6 +698,28 @@ void testAll({
return isolate.rootLib!.id!;
}

test(
'RecordType getters',
() async {
final libraryId = getRootLibraryId();

final type = await getInstanceRef(libraryId, '(0,1).runtimeType');
final result = await getInstanceRef(type.id, 'hashCode');

expect(result, matchInstanceRefKind('Double'));
},
skip: 'https://github.com/dart-lang/sdk/issues/54609',
);

test('Object getters', () async {
final libraryId = getRootLibraryId();

final type = await getInstanceRef(libraryId, 'Object()');
final result = await getInstanceRef(type.id, 'hashCode');

expect(result, matchInstanceRefKind('Double'));
});

test('with scope', () async {
final libraryId = getRootLibraryId();

Expand Down Expand Up @@ -761,15 +783,13 @@ void testAll({
final evaluation2 = evaluate(libraryId, 'MainClass(1,1).toString()');

final results = await Future.wait([evaluation1, evaluation2]);

expect(
results[0],
matchErrorRef(contains('No batch result object ID')),
);
expect(
results[1],
matchErrorRef(contains('No batch result object ID')),
matchErrorRef(
contains('Evaluate is called on an unsupported target'),
),
);
expect(results[1], matchInstanceRef('1, 1'));
});

test('with scope override', () async {
Expand Down Expand Up @@ -900,13 +920,20 @@ Future<String> _setBreakpointInInjectedClient(WipDebugger debugger) async {
return result.json['result']['breakpointId'];
}

Matcher matchInstanceRef(dynamic value) => isA<InstanceRef>().having(
(instance) => instance.valueAsString,
'valueAsString',
value,
Matcher matchInstanceRefKind(String kind) =>
isA<InstanceRef>().having((instance) => instance.kind, 'kind', kind);

Matcher matchInstanceRef(dynamic value) => isA<InstanceRef>()
.having((instance) => instance.valueAsString, 'valueAsString', value);

Matcher matchInstanceClassName(dynamic className) => isA<Instance>().having(
(instance) => instance.classRef!.name,
'class name',
className,
);

Matcher matchInstance(dynamic className) => isA<Instance>().having(
Matcher matchInstanceRefClassName(dynamic className) =>
isA<InstanceRef>().having(
(instance) => instance.classRef!.name,
'class name',
className,
Expand Down
5 changes: 0 additions & 5 deletions dwds/test/inspector_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import 'package:dwds/src/debugging/inspector.dart';
import 'package:dwds/src/utilities/conversions.dart';
import 'package:test/test.dart';
import 'package:test_common/test_sdk_configuration.dart';
import 'package:test_common/utilities.dart';
import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

Expand Down Expand Up @@ -161,10 +160,6 @@ void main() {
final names =
properties.map((p) => p.name).where((x) => x != '__proto__').toList();
final expected = [
if (dartSdkIsAtLeast(
newDdcTypeSystemVersion,
))
'\$ti',
'_privateField',
'abstractField',
'closure',
Expand Down
Loading
Loading