Account for server root (#686)

* Account for server root

Author: grouma

dwds/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.7.2
+
+- Account for root directory path when using `package:` URIs with `DartUri`.
+
 ## 0.7.1
 
 - Fix a bug where we would try to create a new isolate even for a failed

dwds/lib/src/debugging/debugger.dart

@@ -169,7 +169,7 @@ class Debugger extends Domain {
   }
 
   Future<Null> _initialize() async {
-    sources = Sources(_assetHandler, _remoteDebugger, _logWriter);
+    sources = Sources(_assetHandler, _remoteDebugger, _logWriter, _root);
     // We must add a listener before enabling the debugger otherwise we will
     // miss events.
     // Allow a null debugger/connection for unit tests.

dwds/lib/src/debugging/sources.dart

@@ -47,7 +47,10 @@ class Sources {
 
   final RemoteDebugger _remoteDebugger;
 
-  Sources(this._assetHandler, this._remoteDebugger, this._logWriter);
+  final String _root;
+
+  Sources(
+      this._assetHandler, this._remoteDebugger, this._logWriter, this._root);
 
   /// Returns all [Location] data for a provided Dart source.
   Set<Location> locationsForDart(String serverPath) =>
@@ -77,7 +80,11 @@ class Sources {
         for (var entry in lineEntry.entries) {
           var index = entry.sourceUrlId;
           if (index == null) continue;
-          var dartUri = DartUri(mapping.urls[index], script.url);
+          // TODO(grouma) - This work seems expensive and likely should be
+          // cached.
+          var path = p.join(
+              p.dirname(Uri.parse(script.url).path), mapping.urls[index]);
+          var dartUri = DartUri(path, _root);
           var location = Location.from(
             script.scriptId,
             lineEntry,

dwds/lib/src/utilities/dart_uri.dart

@@ -4,8 +4,8 @@
 
 import 'dart:io';
 
-import 'package:path/path.dart' as p;
 import 'package:package_resolver/package_resolver.dart';
+import 'package:path/path.dart' as p;
 
 /// The URI for a particular Dart file, able to canonicalize from various
 /// different representations.
@@ -97,28 +97,34 @@ class DartUri {
   /// packages/path/src/path.dart. The optional [serverUri] is the full URI of the
   /// JS script. The dirname of that path should give us the missing prefix.
   factory DartUri(String uri, [String serverUri]) {
-    // TODO(401): Remove serverUri after D24 is stable.
-    if (uri.startsWith('package:')) return DartUri._fromPackageUri(uri);
+    if (uri.startsWith('package:')) {
+      return DartUri._fromPackageUri(uri, serverUri: serverUri);
+    }
     if (uri.startsWith('org-dartlang-app:')) return DartUri._fromAppUri(uri);
     if (uri.startsWith('google3:')) return DartUri._fromGoogleUri(uri);
     if (uri.startsWith('file:')) return DartUri._fromFileUri(uri);
-    if (uri.startsWith('/packages/')) return DartUri._fromServerPath(uri);
-    if (uri.startsWith('/')) return DartUri._fromServerPath(uri);
+    if (uri.startsWith('/packages/')) {
+      return DartUri._fromRelativePath(uri, serverUri: serverUri);
+    }
+    if (uri.startsWith('/')) return DartUri._fromRelativePath(uri);
     if (uri.startsWith('http:') || uri.startsWith('https:')) {
       return DartUri(Uri.parse(uri).path);
     }
-    // Work around short paths if we have been provided the context.
-    if (serverUri != null) {
-      var path = Uri.parse(serverUri).path;
-      var dir = p.dirname(path);
-      return DartUri._fromServerPath(p.normalize(p.join(dir, uri)));
-    }
+
     throw FormatException('Unsupported URI form', uri);
   }
 
+  /// Returns the dirname for the server URI.
+  static String _dirForServerUri(String uri) => p.dirname(Uri.parse(uri).path);
+
   /// Construct from a package: URI
-  factory DartUri._fromPackageUri(String uri) {
-    return DartUri._('packages/${uri.substring("package:".length)}');
+  factory DartUri._fromPackageUri(String uri, {String serverUri}) {
+    var packagePath = 'packages/${uri.substring("package:".length)}';
+    if (serverUri != null) {
+      return DartUri._fromRelativePath(
+          p.join(_dirForServerUri(serverUri), packagePath));
+    }
+    return DartUri._(packagePath);
   }
 
   /// Construct from a file: URI
@@ -138,17 +144,21 @@ class DartUri {
   factory DartUri._fromAppUri(String uri) {
     // We ignore the first segment of the path, which is the root
     // from which we're serving.
-    // TODO: To be able to convert to an org-dartlang-app: URI we will
-    // need to know the root - possibly keep it as a static?
-    return DartUri._(Uri.parse(uri).pathSegments.skip(1).join('/').toString());
+    var path = Uri.parse(uri).pathSegments.skip(1).join('/').toString();
+    return DartUri._(path);
   }
 
   DartUri._(this.serverPath);
 
   /// Construct from a path, relative to the directory being served.
-  factory DartUri._fromServerPath(String uri) {
+  factory DartUri._fromRelativePath(String uri, {String serverUri}) {
     uri = uri[0] == '.' ? uri.substring(1) : uri;
     uri = uri[0] == '/' ? uri.substring(1) : uri;
+
+    if (serverUri != null) {
+      return DartUri._fromRelativePath(
+          p.join(_dirForServerUri(serverUri), uri));
+    }
     return DartUri._(uri);
   }
 }

dwds/pubspec.yaml

@@ -1,5 +1,5 @@
 name: dwds
-version: 0.7.1
+version: 0.7.2
 author: Dart Team <[email protected]>
 homepage: https://github.com/dart-lang/webdev/tree/master/dwds
 description: >-

dwds/test/dart_uri_test.dart

@@ -7,16 +7,17 @@ import 'package:test/test.dart';
 
 void main() {
   group('DartUri', () {
-    test('normalizes server paths', () {
-      var uri = DartUri('../foo.dart', '/packages/blah/src/blah.dart');
-      expect(uri.serverPath, 'packages/blah/foo.dart');
-    });
-
     test('parses package : paths', () {
       var uri = DartUri('package:path/path.dart');
       expect(uri.serverPath, 'packages/path/path.dart');
     });
 
+    test('parses package : paths with root', () {
+      var uri = DartUri(
+          'package:path/path.dart', 'http://localhost:8080/foo/bar/blah');
+      expect(uri.serverPath, 'foo/bar/packages/path/path.dart');
+    });
+
     test('parses org-dartlang-app paths', () {
       var uri = DartUri('org-dartlang-app:////blah/main.dart');
       expect(uri.serverPath, 'blah/main.dart');

dwds/test/debugging/sources_test.dart

@@ -18,7 +18,7 @@ void main() {
     };
     var logs = <LogRecord>[];
     var sources = Sources(TestingAssetHandler(assets), null,
-        (level, message) => logs.add(LogRecord(level, message, '')));
+        (level, message) => logs.add(LogRecord(level, message, '')), '');
     var serverUri = 'http://localhost:1234/';
     await sources.scriptParsed(ScriptParsedEvent(WipEvent({
       'params': {

webdev/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.5.3-dev
+
+- Depend on the latest `package:dwds`.
+
 ## 2.5.2
 
 - Update SDK dependency to minimum of 2.5.0.

webdev/lib/src/version.dart

@@ -1,6 +1,6 @@
 name: webdev
 # Every time this changes you need to run `pub run build_runner build`.
-version: 2.5.2
+version: 2.5.3-dev
 author: Dart Team <[email protected]>
 homepage: https://github.com/dart-lang/webdev
 description: >-
@@ -46,3 +46,7 @@ dev_dependencies:
 
 executables:
   webdev:
+
+dependency_overrides:
+  dwds:
+    path: ../dwds