From 5d1fb118467025b232f74ca541c4bb4c511606c0 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Thu, 7 Oct 2021 15:13:03 -0700 Subject: [PATCH 1/2] Return only scripts included in a library for library object --- dwds/lib/src/debugging/inspector.dart | 26 ++++++++++++++++-------- dwds/lib/src/debugging/libraries.dart | 2 +- dwds/test/chrome_proxy_service_test.dart | 25 ++++++++++++++++++++++- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/dwds/lib/src/debugging/inspector.dart b/dwds/lib/src/debugging/inspector.dart index 0130f341d..2ea88ad01 100644 --- a/dwds/lib/src/debugging/inspector.dart +++ b/dwds/lib/src/debugging/inspector.dart @@ -45,6 +45,9 @@ class AppInspector extends Domain { /// Map of [ScriptRef] id to containing [LibraryRef] id. final _scriptIdToLibraryId = {}; + /// Map of [Library] id to included [ScriptRef]s. + final _libraryIdToScriptRefs = >{}; + final RemoteDebugger remoteDebugger; final Debugger debugger; final Isolate isolate; @@ -340,8 +343,8 @@ function($argsString) { return _jsCallFunction(function, arguments); } - Future getLibrary(String isolateId, String objectId) async { - return await _getLibrary(isolateId, objectId); + Future getLibrary(String isolateId, String objectId) { + return _getLibrary(isolateId, objectId); } Future _getLibrary(String isolateId, String objectId) async { @@ -410,13 +413,16 @@ function($argsString) { /// Returns the [ScriptRef] for the provided Dart server path [uri]. Future 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> scriptRefsForLibrary(String libraryId) async { + await _populateScriptCaches(); + return _libraryIdToScriptRefs[libraryId]; + } + /// Return the VM SourceReport for the given parameters. /// /// Currently this implements the 'PossibleBreakpoints' report kind. @@ -486,8 +492,10 @@ function($argsString) { /// Request and cache 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. @@ -507,11 +515,13 @@ function($argsString) { for (var part in parts) ScriptRef(uri: part, id: createId()) ]; var libraryRef = await libraryHelper.libraryRefFor(uri); + _libraryIdToScriptRefs.putIfAbsent(libraryRef.id, () => []); 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(); diff --git a/dwds/lib/src/debugging/libraries.dart b/dwds/lib/src/debugging/libraries.dart index a6b3bae84..6719cbbc7 100644 --- a/dwds/lib/src/debugging/libraries.dart +++ b/dwds/lib/src/debugging/libraries.dart @@ -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, diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart index df550d799..c783e0d90 100644 --- a/dwds/test/chrome_proxy_service_test.dart +++ b/dwds/test/chrome_proxy_service_test.dart @@ -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')); @@ -421,6 +421,29 @@ 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, [ + 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; From 3896cfce186d342149ec2ee5281e24e2da8f9005 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Thu, 7 Oct 2021 16:16:24 -0700 Subject: [PATCH 2/2] Addressed CR comments --- dwds/CHANGELOG.md | 1 + dwds/lib/src/debugging/inspector.dart | 10 +++------- dwds/test/chrome_proxy_service_test.dart | 14 ++++++++------ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index f960e1bff..5a2668f25 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -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: diff --git a/dwds/lib/src/debugging/inspector.dart b/dwds/lib/src/debugging/inspector.dart index 2ea88ad01..a3e657ce4 100644 --- a/dwds/lib/src/debugging/inspector.dart +++ b/dwds/lib/src/debugging/inspector.dart @@ -235,7 +235,7 @@ class AppInspector extends Domain { String isolateId, String targetId, String expression, {Map 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.'); @@ -343,11 +343,7 @@ function($argsString) { return _jsCallFunction(function, arguments); } - Future getLibrary(String isolateId, String objectId) { - return _getLibrary(isolateId, objectId); - } - - Future _getLibrary(String isolateId, String objectId) async { + Future getLibrary(String isolateId, String objectId) async { if (isolateId != isolate.id) return null; var libraryRef = await libraryHelper.libraryRefFor(objectId); if (libraryRef == null) return null; @@ -357,7 +353,7 @@ function($argsString) { Future 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; } diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart index c783e0d90..33a17f9d9 100644 --- a/dwds/test/chrome_proxy_service_test.dart +++ b/dwds/test/chrome_proxy_service_test.dart @@ -425,12 +425,14 @@ void main() { var library = await service.getObject(isolate.id, rootLibrary.id) as Library; expect(library.scripts, hasLength(2)); - expect(library.scripts, [ - 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'), - ]); + 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 {