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

* Return only scripts included in a library for library object

* Addressed CR comments

Author: Anna Gringauze

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:

dwds/lib/src/debugging/inspector.dart

@@ -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;
@@ -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.');
@@ -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;
@@ -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;
       }
@@ -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.
@@ -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.
@@ -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();

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,

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,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;