Skip to content

Return only scripts included in a library for library object #1424

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 2 commits into from
Oct 7, 2021
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
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Various VM API
- Hot restart
- Http request handling exceptions
- Only return scripts included in the library with Library object.
- Add `ext.dwds.sendEvent` service extension to dwds so other tools
can send events to the debugger.
Event format:
Expand Down
30 changes: 18 additions & 12 deletions dwds/lib/src/debugging/inspector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ class AppInspector extends Domain {
/// Map of [ScriptRef] id to containing [LibraryRef] id.
final _scriptIdToLibraryId = <String, String>{};

/// Map of [Library] id to included [ScriptRef]s.
final _libraryIdToScriptRefs = <String, List<ScriptRef>>{};

final RemoteDebugger remoteDebugger;
final Debugger debugger;
final Isolate isolate;
Expand Down Expand Up @@ -232,7 +235,7 @@ class AppInspector extends Domain {
String isolateId, String targetId, String expression,
{Map<String, String> scope}) async {
scope ??= {};
var library = await _getLibrary(isolateId, targetId);
var library = await getLibrary(isolateId, targetId);
if (library == null) {
throw UnsupportedError(
'Evaluate is only supported when `targetId` is a library.');
Expand Down Expand Up @@ -341,10 +344,6 @@ function($argsString) {
}

Future<Library> getLibrary(String isolateId, String objectId) async {
return await _getLibrary(isolateId, objectId);
}

Future<Library> _getLibrary(String isolateId, String objectId) async {
if (isolateId != isolate.id) return null;
var libraryRef = await libraryHelper.libraryRefFor(objectId);
if (libraryRef == null) return null;
Expand All @@ -354,7 +353,7 @@ function($argsString) {
Future<Obj> getObject(String isolateId, String objectId,
{int offset, int count}) async {
try {
var library = await _getLibrary(isolateId, objectId);
var library = await getLibrary(isolateId, objectId);
if (library != null) {
return library;
}
Expand Down Expand Up @@ -410,13 +409,16 @@ function($argsString) {

/// Returns the [ScriptRef] for the provided Dart server path [uri].
Future<ScriptRef> scriptRefFor(String uri) async {
if (_serverPathToScriptRef.isEmpty) {
// TODO(grouma) - populate the server path cache a better way.
await getScripts(isolate.id);
}
await _populateScriptCaches();
return _serverPathToScriptRef[uri];
}

/// Returns the [ScriptRef]s in the library with [libraryId].
Future<List<ScriptRef>> scriptRefsForLibrary(String libraryId) async {
await _populateScriptCaches();
return _libraryIdToScriptRefs[libraryId];
}

/// Return the VM SourceReport for the given parameters.
///
/// Currently this implements the 'PossibleBreakpoints' report kind.
Expand Down Expand Up @@ -486,8 +488,10 @@ function($argsString) {

/// Request and cache <ScriptRef>s for all the scripts in the application.
///
/// This populates [_scriptRefsById], [_scriptIdToLibraryId] and
/// [_serverPathToScriptRef]. It is a one-time operation, because if we do a
/// This populates [_scriptRefsById], [_scriptIdToLibraryId],
/// [_libraryIdToScriptRefs] and [_serverPathToScriptRef].
///
/// It is a one-time operation, because if we do a
/// reload the inspector will get re-created.
///
/// Returns the list of scripts refs cached.
Expand All @@ -507,11 +511,13 @@ function($argsString) {
for (var part in parts) ScriptRef(uri: part, id: createId())
];
var libraryRef = await libraryHelper.libraryRefFor(uri);
_libraryIdToScriptRefs.putIfAbsent(libraryRef.id, () => <ScriptRef>[]);
for (var scriptRef in scriptRefs) {
_scriptRefsById[scriptRef.id] = scriptRef;
_scriptIdToLibraryId[scriptRef.id] = libraryRef.id;
_serverPathToScriptRef[DartUri(scriptRef.uri, _root).serverPath] =
scriptRef;
_libraryIdToScriptRefs[libraryRef.id].add(scriptRef);
}
}
return _scriptRefsById.values.toList();
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/libraries.dart
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class LibraryHelper extends Domain {
uri: libraryRef.uri,
debuggable: true,
dependencies: [],
scripts: await inspector.scriptRefs,
scripts: await inspector.scriptRefsForLibrary(libraryRef.id),
variables: [],
functions: [],
classes: classRefs,
Expand Down
27 changes: 26 additions & 1 deletion dwds/test/chrome_proxy_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ void main() {

setUp(setCurrentLogWriter);

test('Libraries', () async {
test('root Library', () async {
expect(rootLibrary, isNotNull);
// TODO: library names change with kernel dart-lang/sdk#36736
expect(rootLibrary.uri, endsWith('main.dart'));
Expand All @@ -421,6 +421,31 @@ void main() {
expect(testClass.name, 'MyTestClass');
});

test('Library only contains included scripts', () async {
var library =
await service.getObject(isolate.id, rootLibrary.id) as Library;
expect(library.scripts, hasLength(2));
expect(
library.scripts,
unorderedEquals([
predicate((ScriptRef s) =>
s.uri == 'org-dartlang-app:///example/hello_world/main.dart'),
predicate((ScriptRef s) =>
s.uri == 'org-dartlang-app:///example/hello_world/part.dart'),
]));
});

test('Can get the same library in parallel', () async {
var futures = [
service.getObject(isolate.id, rootLibrary.id),
service.getObject(isolate.id, rootLibrary.id),
];
var results = await Future.wait(futures);
var library1 = results[0] as Library;
var library2 = results[1] as Library;
expect(library1, equals(library2));
});

test('Classes', () async {
var testClass = await service.getObject(
isolate.id, rootLibrary.classes.first.id) as Class;
Expand Down