Skip to content

Commit

Permalink
Prepare DWDS 23.0.0+1 hotfix (#2355)
Browse files Browse the repository at this point in the history
  • Loading branch information
elliette authored Jan 23, 2024
1 parent fd6ec44 commit ca38a3a
Show file tree
Hide file tree
Showing 16 changed files with 618 additions and 203 deletions.
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

0 comments on commit ca38a3a

Please sign in to comment.