Fix failing breakpoints in library part files and data race

- Record library part paths in debugger tables properly.
  Make sure that we have module and library recorded for
  part scripts, in `MetadataProvider` and `Modules`,
- Calculate locations for module atomically in `Locations`.
  This fixes intermittent 'Oh, snap!' in chrome due to a
  data race in location computation if several breakpoints
  are set in the same dart library.

- Update test fixtures to include part files.
- Add tests for frontend server and build runner scenarios.

Closes: dart-lang#1271

dwds/CHANGELOG.md

@@ -3,6 +3,8 @@
 - Support `vm_service` version `6.2.0`.
 - Fix missing sdk libraries in `getObject()` calls.
 - Fix incorrect `rootLib` returned by `ChromeProxyService`.
+- Fix not working breakpoints in library part files.
+- Fix data race in calculating locations for a module.
 
 ## 10.0.1
 

dwds/lib/src/debugging/inspector.dart

@@ -5,7 +5,6 @@
 // @dart = 2.9
 
 import 'package:logging/logging.dart';
-import 'package:path/path.dart' as p;
 import 'package:vm_service/vm_service.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
@@ -88,6 +87,9 @@ class AppInspector extends Domain {
     isolate.libraries.addAll(libraries);
     await DartUri.recordAbsoluteUris(libraries.map((lib) => lib.uri));
 
+    var scripts = await _getScripts();
+    await DartUri.recordAbsoluteUris(scripts.map((script) => script.uri));
+
     isolate.extensionRPCs.addAll(await _getExtensionRpcs());
   }
 
@@ -470,12 +472,10 @@ function($argsString) {
     // for them.
     var userLibraries = libraryUris.where((uri) => !uri.startsWith('dart:'));
     for (var uri in userLibraries) {
-      var parent = uri.substring(0, uri.lastIndexOf('/'));
       var parts = scripts[uri];
       var scriptRefs = [
         ScriptRef(uri: uri, id: createId()),
-        for (var part in parts)
-          ScriptRef(uri: p.url.join(parent, part), id: createId())
+        for (var part in parts) ScriptRef(uri: part, id: createId())
       ];
       var libraryRef = await libraryHelper.libraryRefFor(uri);
       for (var scriptRef in scriptRefs) {

dwds/lib/src/debugging/location.dart

@@ -4,6 +4,7 @@
 
 // @dart = 2.9
 
+import 'package:async/async.dart';
 import 'package:path/path.dart' as p;
 import 'package:source_maps/parser.dart';
 import 'package:source_maps/source_maps.dart';
@@ -105,16 +106,17 @@ class JsLocation {
 
 /// Contains meta data for known [Location]s.
 class Locations {
-  /// Map from Dart server path to all corresponding [Location] data.
-  final _sourceToLocation = <String, Set<Location>>{};
+  /// [Location] data for Dart server path.
+  final Map<String, Set<Location>> _sourceToLocation = {};
+  final Map<String, AsyncMemoizer<Set<Location>>> _locationMemoizer = {};
 
-  /// Map from Dart server path to tokenPosTable as defined in the
+  /// `tokenPosTable` for Dart server path, as defined in the
   /// Dart VM Service Protocol:
   /// https://github.com/dart-lang/sdk/blob/master/runtime/vm/service/service.md#script
-  final _sourceToTokenPosTable = <String, List<List<int>>>{};
+  final Map<String, List<List<int>>> _sourceToTokenPosTable = {};
 
   /// The set of all known [Location]s for a module.
-  final _moduleToLocations = <String, Set<Location>>{};
+  final Map<String, Set<Location>> _moduleToLocations = {};
 
   final AssetReader _assetReader;
   final Modules _modules;
@@ -129,15 +131,14 @@ class Locations {
   void initialize(String entrypoint) {
     _sourceToTokenPosTable.clear();
     _sourceToLocation.clear();
+    _locationMemoizer.clear();
     _moduleToLocations.clear();
     _entrypoint = entrypoint;
   }
 
   /// Returns all [Location] data for a provided Dart source.
   Future<Set<Location>> locationsForDart(String serverPath) async {
     var module = await _modules.moduleForSource(serverPath);
-    var cache = _sourceToLocation[serverPath];
-    if (cache != null) return cache;
     await _locationsForModule(module);
     return _sourceToLocation[serverPath] ?? {};
   }
@@ -202,50 +203,56 @@ class Locations {
   ///
   /// This will populate the [_sourceToLocation] and [_moduleToLocations] maps.
   Future<Set<Location>> _locationsForModule(String module) async {
-    if (module == null) return {};
-    if (_moduleToLocations[module] != null) return _moduleToLocations[module];
-    var result = <Location>{};
-    if (module?.isEmpty ?? true) return _moduleToLocations[module] = result;
-    if (module.endsWith('dart_sdk') || module.endsWith('dart_library')) {
-      return result;
-    }
-    var modulePath =
-        await globalLoadStrategy.serverPathForModule(_entrypoint, module);
-    var sourceMapPath =
-        await globalLoadStrategy.sourceMapPathForModule(_entrypoint, module);
-    var sourceMapContents = await _assetReader.sourceMapContents(sourceMapPath);
-    var scriptLocation = p.url.dirname('/$modulePath');
-    if (sourceMapContents == null) return result;
-    // This happens to be a [SingleMapping] today in DDC.
-    var mapping = parse(sourceMapContents);
-    if (mapping is SingleMapping) {
-      // Create TokenPos for each entry in the source map.
-      for (var lineEntry in mapping.lines) {
-        for (var entry in lineEntry.entries) {
-          var index = entry.sourceUrlId;
-          if (index == null) continue;
-          // Source map URLS are relative to the script. They may have platform separators
-          // or they may use URL semantics. To be sure, we split and re-join them.
-          // This works on Windows because path treats both / and \ as separators.
-          // It will fail if the path has both separators in it.
-          var relativeSegments = p.split(mapping.urls[index]);
-          var path = p.url
-              .normalize(p.url.joinAll([scriptLocation, ...relativeSegments]));
-          var dartUri = DartUri(path, _root);
-          result.add(Location.from(
-            modulePath,
-            lineEntry,
-            entry,
-            dartUri,
-          ));
+    _locationMemoizer.putIfAbsent(module, () => AsyncMemoizer());
+
+    return await _locationMemoizer[module].runOnce(() async {
+      if (module == null) return {};
+      if (_moduleToLocations[module] != null) return _moduleToLocations[module];
+      var result = <Location>{};
+      if (module?.isEmpty ?? true) return _moduleToLocations[module] = result;
+      if (module.endsWith('dart_sdk') || module.endsWith('dart_library')) {
+        return result;
+      }
+      var modulePath =
+          await globalLoadStrategy.serverPathForModule(_entrypoint, module);
+      var sourceMapPath =
+          await globalLoadStrategy.sourceMapPathForModule(_entrypoint, module);
+      var sourceMapContents =
+          await _assetReader.sourceMapContents(sourceMapPath);
+      var scriptLocation = p.url.dirname('/$modulePath');
+      if (sourceMapContents == null) return result;
+      // This happens to be a [SingleMapping] today in DDC.
+      var mapping = parse(sourceMapContents);
+      if (mapping is SingleMapping) {
+        // Create TokenPos for each entry in the source map.
+        for (var lineEntry in mapping.lines) {
+          for (var entry in lineEntry.entries) {
+            var index = entry.sourceUrlId;
+            if (index == null) continue;
+            // Source map URLS are relative to the script. They may have platform separators
+            // or they may use URL semantics. To be sure, we split and re-join them.
+            // This works on Windows because path treats both / and \ as separators.
+            // It will fail if the path has both separators in it.
+            var relativeSegments = p.split(mapping.urls[index]);
+            var path = p.url.normalize(
+                p.url.joinAll([scriptLocation, ...relativeSegments]));
+            var dartUri = DartUri(path, _root);
+            result.add(Location.from(
+              modulePath,
+              lineEntry,
+              entry,
+              dartUri,
+            ));
+          }
         }
       }
-    }
-    for (var location in result) {
-      _sourceToLocation
-          .putIfAbsent(location.dartLocation.uri.serverPath, () => <Location>{})
-          .add(location);
-    }
-    return _moduleToLocations[module] = result;
+      for (var location in result) {
+        _sourceToLocation
+            .putIfAbsent(
+                location.dartLocation.uri.serverPath, () => <Location>{})
+            .add(location);
+      }
+      return _moduleToLocations[module] = result;
+    });
   }
 }

dwds/lib/src/debugging/metadata/provider.dart

@@ -9,6 +9,7 @@ import 'dart:convert';
 
 import 'package:async/async.dart';
 import 'package:logging/logging.dart';
+import 'package:path/path.dart' as p;
 
 import '../../readers/asset_reader.dart';
 import 'module_metadata.dart';
@@ -233,7 +234,10 @@ class MetadataProvider {
 
       _scriptToModule[library.importUri] = metadata.name;
       for (var path in library.partUris) {
-        _scripts[library.importUri].add(path);
+        // parts in metadata are relative to the library Uri directory
+        var partPath = p.url.join(p.dirname(library.importUri), path);
+        _scripts[library.importUri].add(partPath);
+        _scriptToModule[partPath] = metadata.name;
       }
     }
   }

dwds/lib/src/debugging/modules.dart

@@ -73,15 +73,26 @@ class Modules {
   Future<void> _initializeMapping() async {
     var provider = globalLoadStrategy.metadataProviderFor(_entrypoint);
 
+    var libraryToScripts = await provider.scripts;
     var scriptToModule = await provider.scriptToModule;
-    for (var script in scriptToModule.keys) {
-      var serverPath = script.startsWith('dart:')
-          ? script
-          : DartUri(script, _root).serverPath;
-
-      _sourceToModule[serverPath] = scriptToModule[script];
-      _sourceToLibrary[serverPath] = Uri.parse(script);
-      _libraryToModule[script] = scriptToModule[script];
+
+    for (var library in libraryToScripts.keys) {
+      var libraryServerPath = library.startsWith('dart:')
+          ? library
+          : DartUri(library, _root).serverPath;
+
+      _sourceToModule[libraryServerPath] = scriptToModule[library];
+      _sourceToLibrary[libraryServerPath] = Uri.parse(library);
+      _libraryToModule[library] = scriptToModule[library];
+
+      for (var script in libraryToScripts[library]) {
+        var scriptServerPath = script.startsWith('dart:')
+            ? script
+            : DartUri(script, _root).serverPath;
+
+        _sourceToModule[scriptServerPath] = scriptToModule[library];
+        _sourceToLibrary[scriptServerPath] = Uri.parse(library);
+      }
     }
   }
 }

dwds/test/build_daemon_evaluate_test.dart

@@ -85,6 +85,7 @@ void main() async {
           ScriptRef mainScript;
           ScriptRef libraryScript;
           ScriptRef testLibraryScript;
+          ScriptRef testLibraryPartScript;
           Stream<Event> stream;
 
           setUp(() async {
@@ -99,6 +100,8 @@ void main() async {
                 .firstWhere((each) => each.uri.contains('main.dart'));
             testLibraryScript = scripts.scripts.firstWhere((each) =>
                 each.uri.contains('package:_testPackage/test_library.dart'));
+            testLibraryPartScript = scripts.scripts.firstWhere((each) =>
+                each.uri.contains('package:_testPackage/src/test_part.dart'));
             libraryScript = scripts.scripts.firstWhere(
                 (each) => each.uri.contains('package:_test/library.dart'));
           });
@@ -125,7 +128,8 @@ void main() async {
           });
 
           test('field', () async {
-            await onBreakPoint(isolate.id, mainScript, 'printField', () async {
+            await onBreakPoint(
+                isolate.id, mainScript, 'printFieldFromLibraryClass', () async {
               var event = await stream.firstWhere(
                   (event) => event.kind == EventKind.kPauseBreakpoint);
 
@@ -142,7 +146,8 @@ void main() async {
           });
 
           test('private field from another library', () async {
-            await onBreakPoint(isolate.id, mainScript, 'printField', () async {
+            await onBreakPoint(
+                isolate.id, mainScript, 'printFieldFromLibraryClass', () async {
               var event = await stream.firstWhere(
                   (event) => event.kind == EventKind.kPauseBreakpoint);
 
@@ -177,7 +182,8 @@ void main() async {
           });
 
           test('access instance fields after evaluation', () async {
-            await onBreakPoint(isolate.id, mainScript, 'printField', () async {
+            await onBreakPoint(
+                isolate.id, mainScript, 'printFieldFromLibraryClass', () async {
               var event = await stream.firstWhere(
                   (event) => event.kind == EventKind.kPauseBreakpoint);
 
@@ -267,6 +273,40 @@ void main() async {
             });
           });
 
+          test('call library part function with const param', () async {
+            await onBreakPoint(isolate.id, mainScript, 'printLocal', () async {
+              var event = await stream.firstWhere(
+                  (event) => event.kind == EventKind.kPauseBreakpoint);
+
+              var result = await setup.service.evaluateInFrame(isolate.id,
+                  event.topFrame.index, 'testLibraryPartFunction(42)');
+
+              expect(
+                  result,
+                  isA<InstanceRef>().having(
+                      (instance) => instance.valueAsString,
+                      'valueAsString',
+                      '42'));
+            });
+          });
+
+          test('call library part function with local param', () async {
+            await onBreakPoint(isolate.id, mainScript, 'printLocal', () async {
+              var event = await stream.firstWhere(
+                  (event) => event.kind == EventKind.kPauseBreakpoint);
+
+              var result = await setup.service.evaluateInFrame(isolate.id,
+                  event.topFrame.index, 'testLibraryPartFunction(local)');
+
+              expect(
+                  result,
+                  isA<InstanceRef>().having(
+                      (instance) => instance.valueAsString,
+                      'valueAsString',
+                      '42'));
+            });
+          });
+
           test('loop variable', () async {
             await onBreakPoint(isolate.id, mainScript, 'printLoopVariable',
                 () async {
@@ -303,7 +343,7 @@ void main() async {
             });
           });
 
-          test('evaluate expression in a class constructor in another library',
+          test('evaluate expression in a class constructor in a library',
               () async {
             await onBreakPoint(
                 isolate.id, testLibraryScript, 'testLibraryClassConstructor',
@@ -323,6 +363,25 @@ void main() async {
             });
           });
 
+          test('evaluate expression in a class constructor in a library part',
+              () async {
+            await onBreakPoint(isolate.id, testLibraryPartScript,
+                'testLibraryPartClassConstructor', () async {
+              var event = await stream.firstWhere(
+                  (event) => event.kind == EventKind.kPauseBreakpoint);
+
+              var result = await setup.service.evaluateInFrame(
+                  isolate.id, event.topFrame.index, 'this.field');
+
+              expect(
+                  result,
+                  isA<InstanceRef>().having(
+                      (instance) => instance.valueAsString,
+                      'valueAsString',
+                      '1'));
+            });
+          });
+
           test('evaluate expression in caller frame', () async {
             await onBreakPoint(
                 isolate.id, testLibraryScript, 'testLibraryFunction', () async {
@@ -341,7 +400,7 @@ void main() async {
             });
           });
 
-          test('evaluate expression in  another library', () async {
+          test('evaluate expression in a library', () async {
             await onBreakPoint(isolate.id, libraryScript, 'Concatenate',
                 () async {
               var event = await stream.firstWhere(