Skip to content

Hide internal implementation details in type objects #2103

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

Merged
merged 14 commits into from
May 8, 2023
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Do not show async frame errors on evaluation. - [#2073](https://github.com/dart-lang/webdev/pull/2073)
- Refactor code for presenting record instances. - [#2074](https://github.com/dart-lang/webdev/pull/2074)
- Display record types concisely. - [#2070](https://github.com/dart-lang/webdev/pull/2070)
- Display type objects concisely. - [#2103](https://github.com/dart-lang/webdev/pull/2103)

## 19.0.0

Expand Down
10 changes: 6 additions & 4 deletions dwds/lib/src/debugging/classes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,11 @@ class ClassHelper extends Domain {
fieldDescriptors.forEach((name, descriptor) {
final classMetaData = ClassMetaData(
jsName: descriptor['classRefName'],
libraryId: descriptor['classRefLibraryId'],
dartName: descriptor['classRefDartName'],
runtimeKind: RuntimeObjectKind.type,
classRef: classRefFor(
descriptor['classRefLibraryId'],
descriptor['classRefDartName'],
),
);
fieldRefs.add(
FieldRef(
Expand All @@ -209,8 +212,7 @@ class ClassHelper extends Domain {
declaredType: InstanceRef(
identityHashCode: createId().hashCode,
id: createId(),
kind: InstanceKind.kType,
// TODO(elliette): Is this the same as classRef?
kind: classMetaData.kind,
classRef: classMetaData.classRef,
),
isConst: descriptor['isConst'] as bool,
Expand Down
6 changes: 3 additions & 3 deletions dwds/lib/src/debugging/dart_scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:dwds/src/debugging/debugger.dart';
import 'package:dwds/src/utilities/domain.dart';
import 'package:dwds/src/utilities/objects.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

Expand All @@ -25,7 +25,7 @@ final previousDdcTemporaryVariableRegExp =
///
/// See chromedevtools.github.io/devtools-protocol/tot/Debugger#type-CallFrame.
Future<List<Property>> visibleProperties({
required Debugger debugger,
required AppInspectorInterface inspector,
required WipCallFrame frame,
}) async {
final allProperties = <Property>[];
Expand All @@ -48,7 +48,7 @@ Future<List<Property>> visibleProperties({
for (var scope in filterScopes(frame).reversed) {
final objectId = scope.object.objectId;
if (objectId != null) {
final properties = await debugger.getProperties(objectId);
final properties = await inspector.getProperties(objectId);
allProperties.addAll(properties);
}
}
Expand Down
147 changes: 4 additions & 143 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:math' as math;

import 'package:dwds/src/debugging/dart_scope.dart';
import 'package:dwds/src/debugging/frame_computer.dart';
import 'package:dwds/src/debugging/location.dart';
import 'package:dwds/src/debugging/metadata/class.dart';
import 'package:dwds/src/debugging/remote_debugger.dart';
import 'package:dwds/src/debugging/skip_list.dart';
import 'package:dwds/src/loaders/strategy.dart';
import 'package:dwds/src/services/chrome_debug_exception.dart';
import 'package:dwds/src/utilities/conversions.dart';
import 'package:dwds/src/utilities/dart_uri.dart';
import 'package:dwds/src/utilities/domain.dart';
import 'package:dwds/src/utilities/objects.dart' show Property;
Expand Down Expand Up @@ -394,15 +391,16 @@ class Debugger extends Domain {
/// The variables visible in a frame in Dart protocol [BoundVariable] form.
Future<List<BoundVariable>> variablesFor(WipCallFrame frame) async {
// TODO(alanknight): Can these be moved to dart_scope.dart?
final properties = await visibleProperties(debugger: this, frame: frame);
final properties =
await visibleProperties(inspector: inspector, frame: frame);
final boundVariables = await Future.wait(
properties.map(_boundVariable),
);

// Filter out variables that do not come from dart code, such as native
// JavaScript objects
return boundVariables
.where((bv) => isDisplayableObject(bv?.value))
.where((bv) => inspector.isDisplayableObject(bv?.value))
.toList()
.cast();
}
Expand Down Expand Up @@ -432,84 +430,6 @@ class Debugger extends Domain {
return null;
}

static bool _isEmptyRange({
required int length,
int? offset,
int? count,
}) {
if (count == 0) return true;
if (offset == null) return false;
return offset >= length;
}

static bool _isSubRange({
int? offset,
int? count,
}) {
if (offset == 0 && count == null) return false;
return offset != null || count != null;
}

/// Compute the last possible element index in the range of [offset]..end
/// that includes [count] elements, if available.
static int? _calculateRangeEnd({
int? count,
required int offset,
required int length,
}) =>
count == null ? null : math.min(offset + count, length);

/// Calculate the number of available elements in the range.
static int _calculateRangeCount({
int? count,
required int offset,
required int length,
}) =>
count == null ? length - offset : math.min(count, length - offset);

/// Find a sub-range of the entries for a Map/List when offset and/or count
/// have been specified on a getObject request.
///
/// If the object referenced by [id] is not a system List or Map then this
/// will just return a RemoteObject for it and ignore [offset], [count] and
/// [length]. If it is, then [length] should be the number of entries in the
/// List/Map and [offset] and [count] should indicate the desired range.
Future<RemoteObject> _subRange(
String id, {
required int offset,
int? count,
required int length,
}) async {
// TODO(#809): Sometimes we already know the type of the object, and
// we could take advantage of that to short-circuit.
final receiver = remoteObjectFor(id);
final end =
_calculateRangeEnd(count: count, offset: offset, length: length);
final rangeCount =
_calculateRangeCount(count: count, offset: offset, length: length);
final args =
[offset, rangeCount, end].map(dartIdFor).map(remoteObjectFor).toList();
// If this is a List, just call sublist. If it's a Map, get the entries, but
// avoid doing a toList on a large map using skip/take to get the section we
// want. To make those alternatives easier in JS, pass both count and end.
final expression = '''
function (offset, count, end) {
const sdk = ${globalLoadStrategy.loadModuleSnippet}("dart_sdk");
if (sdk.core.Map.is(this)) {
const entries = sdk.dart.dload(this, "entries");
const skipped = sdk.dart.dsend(entries, "skip", [offset])
const taken = sdk.dart.dsend(skipped, "take", [count]);
return sdk.dart.dsend(taken, "toList", []);
} else if (sdk.core.List.is(this)) {
return sdk.dart.dsendRepl(this, "sublist", [offset, end]);
} else {
return this;
}
}
''';
return await inspector.jsCallFunctionOn(receiver, expression, args);
}

// TODO(elliette): https://github.com/dart-lang/webdev/issues/1501 Re-enable
// after checking with Chrome team if there is a way to check if the Chrome
// DevTools is showing an overlay. Both cannot be shown at the same time:
Expand All @@ -533,48 +453,6 @@ class Debugger extends Domain {
// _pausedOverlayVisible = false;
// }

/// Calls the Chrome Runtime.getProperties API for the object with [objectId].
///
/// Note that the property names are JS names, e.g.
/// Symbol(DartClass.actualName) and will need to be converted. For a system
/// List or Map, [offset] and/or [count] can be provided to indicate a desired
/// range of entries. They will be ignored if there is no [length].
Future<List<Property>> getProperties(
String objectId, {
int? offset,
int? count,
int? length,
}) async {
String rangeId = objectId;
// Ignore offset/count if there is no length:
if (length != null) {
if (_isEmptyRange(offset: offset, count: count, length: length)) {
return [];
}
if (_isSubRange(offset: offset, count: count)) {
final range = await _subRange(
objectId,
offset: offset ?? 0,
count: count,
length: length,
);
rangeId = range.objectId ?? rangeId;
}
}
final jsProperties = await sendCommandAndValidateResult<List>(
_remoteDebugger,
method: 'Runtime.getProperties',
resultField: 'result',
params: {
'objectId': rangeId,
'ownProperties': true,
},
);
return jsProperties
.map<Property>((each) => Property(each as Map<String, dynamic>))
.toList();
}

/// Returns a Dart [Frame] for a JS [frame].
Future<Frame?> calculateDartFrameFor(
WipCallFrame frame,
Expand Down Expand Up @@ -658,7 +536,7 @@ class Debugger extends Domain {
if (map['type'] == 'object') {
final obj = RemoteObject(map);
exception = await inspector.instanceRefFor(obj);
if (exception != null && isNativeJsError(exception)) {
if (exception != null && inspector.isNativeJsError(exception)) {
if (obj.description != null) {
// Create a string exception object.
final description =
Expand Down Expand Up @@ -845,23 +723,6 @@ Future<T> sendCommandAndValidateResult<T>(
return result;
}

/// Returns true for objects we display for the user.
bool isDisplayableObject(Object? object) =>
object is Sentinel ||
object is InstanceRef &&
!isNativeJsObject(object) &&
!isNativeJsError(object);

/// Returns true for non-dart JavaScript objects.
bool isNativeJsObject(InstanceRef instanceRef) {
return isNativeJsObjectRef(instanceRef.classRef);
}

/// Returns true of JavaScript exceptions.
bool isNativeJsError(InstanceRef instanceRef) {
return instanceRef.classRef == classRefForNativeJsError;
}

/// Returns the Dart line number for the provided breakpoint.
int _lineNumberFor(Breakpoint breakpoint) =>
int.parse(breakpoint.id!.split('#').last.split(':').first);
Expand Down
Loading