Skip to content

Implement lookupResolvedPackageUris and lookupPackageUris #1458

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
Show file tree
Hide file tree
Changes from 4 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 @@ -14,6 +14,7 @@
- Fix chrome detection in iPhone emulation mode in chrome or edge browsers.
- Reliably find unused port for extension backend http service.
- Ignore offset / count parameters in getObject if the object has no length
- Implement `lookupResolvedPackageUris` and `lookupPackageUris` vm service API.

## 11.4.0

Expand Down
26 changes: 13 additions & 13 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,19 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
return (await _debugger).pause();
}

@override
Future<UriList> lookupResolvedPackageUris(
String isolateId, List<String> uris) async {
await isInitialized;
return UriList(uris: <String>[...uris.map(DartUri.toResolvedUri)]);
}

@override
Future<UriList> lookupPackageUris(String isolateId, List<String> uris) async {
await isInitialized;
return UriList(uris: <String>[...uris.map(DartUri.toPackageUri)]);
}

@override
Future<Success> registerService(String service, String alias) async {
return _rpcNotSupportedFuture('registerService');
Expand Down Expand Up @@ -1037,19 +1050,6 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
String isolateId, String breakpointId, bool enable) =>
throw UnimplementedError();

@override
Future<UriList> lookupPackageUris(String isolateId, List<String> uris) {
// TODO(https://github.com/dart-lang/webdev/issues/1446): implement.
throw UnimplementedError();
}

@override
Future<UriList> lookupResolvedPackageUris(
String isolateId, List<String> uris) {
// TODO(https://github.com/dart-lang/webdev/issues/1446): implement.
throw UnimplementedError();
}

/// Prevent DWDS from blocking Dart SDK rolls if changes in package:vm_service
/// are unimplemented in DWDS.
@override
Expand Down
53 changes: 42 additions & 11 deletions dwds/lib/src/utilities/dart_uri.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,39 +50,70 @@ class DartUri {
/// The way we resolve file: URLs into package: URLs
static PackageConfig _packageConfig;

/// All of the known absolute library paths, indexed by their library URL.
///
/// Examples:
///
/// We are assuming that all library uris are coming from
/// https://github.com/dart-lang/sdk/blob/main/runtime/vm/service/service.md#getscripts)
/// and can be translated to their absolute paths and back.
///
/// dart:html <->
/// org-dartlang-sdk:///sdk/lib/html/html.dart
/// (not supported, issue: https://github.com/dart-lang/webdev/issues/1457)
///
/// org-dartlang-app:///example/hello_world/main.dart <->
/// file:///source/webdev/fixtures/_test/example/hello_world/main.dart,
///
/// org-dartlang-app:///example/hello_world/part.dart <->
/// file:///source/webdev/fixtures/_test/example/hello_world/part.dart,
///
/// package:path/path.dart <->
/// file:///.pub-cache/hosted/pub.dartlang.org/path-1.8.0/lib/path.dart,
///
/// package:path/src/path_set.dart <->
/// file:///.pub-cache/hosted/pub.dartlang.org/path-1.8.0/lib/src/path_set.dart,
static final Map<String, String> _uriToResolvedUri = {};

/// All of the known libraries, indexed by their absolute file URL.
static final Map<String, String> _libraryNamesByPath = {};
static final Map<String, String> _resolvedUriToUri = {};

/// Record all of the libraries, indexed by their absolute file: URI.
static Future<void> recordAbsoluteUris(Iterable<String> libraryUris) async {
if (_shouldRecord) {
await _loadPackageConfig(_packagesUri);
_libraryNamesByPath.clear();
_resolvedUriToUri.clear();
_uriToResolvedUri.clear();
for (var uri in libraryUris) {
_recordAbsoluteUri(uri);
}
}
}

static String toPackageUri(String uri) => _resolvedUriToUri[uri];

static String toResolvedUri(String uri) => _uriToResolvedUri[uri];

/// Record the library represented by package: or org-dartlang-app: uris
/// indexed by absolute file: URI.
static void _recordAbsoluteUri(String libraryUri) {
var uri = Uri.parse(libraryUri);
if (uri.scheme == 'dart' ||
(uri.scheme == '' && !uri.path.endsWith('.dart'))) {
// We ignore dart: libraries, and non-Dart libraries referenced by path.
// e.g. main.dart.bootstrap
// TODO(alanknight): These should not be showing up in the library list,
// fix _getLibraryRefs and then remove this check.
if (uri.scheme == '' && !uri.path.endsWith('.dart')) {
// ignore non-dart files
} else if (uri.scheme == 'dart') {
// TODO: implement to match the VM service API
// Issue: https://github.com/dart-lang/webdev/issues/1457
} else if (uri.scheme == 'org-dartlang-app' || uri.scheme == 'google3') {
// Both currentDirectoryUri and the libraryUri path should have '/'
// separators, so we can join them as url paths to get the absolute file
// url.
var libraryPath = p.url.join(currentDirectoryUri, uri.path.substring(1));
_libraryNamesByPath[libraryPath] = libraryUri;
_uriToResolvedUri[libraryUri] = libraryPath;
_resolvedUriToUri[libraryPath] = libraryUri;
} else if (uri.scheme == 'package') {
var libraryPath = _packageConfig.resolve(uri);
_libraryNamesByPath['$libraryPath'] = libraryUri;
_uriToResolvedUri[libraryUri] = '$libraryPath';
_resolvedUriToUri['$libraryPath'] = libraryUri;
} else {
throw ArgumentError.value(libraryUri, 'URI scheme not allowed');
}
Expand Down Expand Up @@ -139,7 +170,7 @@ class DartUri {

/// Construct from a file: URI
factory DartUri._fromFileUri(String uri) {
var libraryName = _libraryNamesByPath[uri];
var libraryName = _resolvedUriToUri[uri];
if (libraryName != null) return DartUri(libraryName);
// This is not one of our recorded libraries.
throw ArgumentError.value(uri, 'uri', 'Unknown library');
Expand Down
93 changes: 93 additions & 0 deletions dwds/test/chrome_proxy_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1261,6 +1261,99 @@ void main() {
service.getRetainingPath(null, null, null), throwsRPCError);
});

test('lookupResolvedPackageUris converts package and org-dartlang-app uris',
() async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;
var scriptList = await service.getScripts(isolateId);

var uris = [...scriptList.scripts.map((e) => e.uri)];
var resolvedUris =
await service.lookupResolvedPackageUris(isolateId, uris);

expect(
resolvedUris.uris,
containsAll([
contains('/_test/example/hello_world/main.dart'),
contains('/lib/path.dart'),
contains('/lib/src/path_set.dart'),
]));
Comment on lines +1310 to +1314
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused why expectation needs containsAll() with nested contains(). Is that on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

containsAll matches the array, and the nested contains match the elements. I like that our matches allow that!

});

test('lookupResolvedPackageUris does not translate non-exsitent paths',
() async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;

var resolvedUris = await service.lookupResolvedPackageUris(isolateId, [
'dart:io', // dart:io is not imported
'does_not_exist.dart',
]);
expect(resolvedUris.uris, [null, null]);
});

test('lookupResolvedPackageUris translates dart uris', () async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;

var resolvedUris = await service.lookupResolvedPackageUris(isolateId, [
'dart:html',
'dart:async',
]);

expect(resolvedUris.uris, [
'org-dartlang-sdk:///sdk/lib/html/html.dart',
'org-dartlang-sdk:///sdk/lib/async/async.dart',
]);
}, skip: 'https://github.com/dart-lang/webdev/issues/1457');

test('lookupPackageUris finds package and org-dartlang-app paths',
() async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;
var scriptList = await service.getScripts(isolateId);

var uris = [...scriptList.scripts.map((e) => e.uri)];
var resolvedUris =
await service.lookupResolvedPackageUris(isolateId, uris);

var packageUris = await service.lookupPackageUris(
isolateId, resolvedUris.uris as List<String>);
expect(
packageUris.uris,
containsAll([
'org-dartlang-app:///example/hello_world/main.dart',
'package:path/path.dart',
'package:path/src/path_set.dart',
]));
});

test('lookupPackageUris does not translate non-exsitent paths', () async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;

var resolvedUris = await service.lookupPackageUris(isolateId, [
'org-dartlang-sdk:///sdk/lib/io/io.dart', // dart:io is not imported
'does_not_exist.dart',
]);
expect(resolvedUris.uris, [null, null]);
});

test('lookupPackageUris translates dart uris', () async {
var vm = await service.getVM();
var isolateId = vm.isolates.first.id;

var resolvedUris = await service.lookupPackageUris(isolateId, [
'org-dartlang-sdk:///sdk/lib/html/html.dart',
'org-dartlang-sdk:///sdk/lib/async/async.dart',
]);

expect(resolvedUris.uris, [
'dart:html',
'dart:async',
]);
}, skip: 'https://github.com/dart-lang/webdev/issues/1457');

test('registerService', () async {
await expectLater(
service.registerService('ext.foo.bar', null), throwsRPCError);
Expand Down