Fix issues discovered when using web server device in flutter tools (#1588)

* Fix breakpoints in web server

* Updated version and built

* Addressed CR comments

* Addressed CR comments, added checks for assumptions and tests

* Disable testing without DevTools and with debug extension on windows

* Addressed CR comments

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -1,6 +1,13 @@
 ## 13.1.1-dev
 - Add column information to breakpoints to allow precise breakpoint placement.
 - Split SDK validation methods to allow validation of separate components.
+- Remove dependency on `package:_fe_analyzer_shared`.
+  Note: this removes current incomplete support for resolving `dart:` uris.
+- Fix issues discovered when using flutter tools with web server device:
+  - Remove `dart:web_sql` from the list of SDK libraries as it is no longer
+    used.
+  - Fix crash when using flutter tools with web server device.
+  - Remove clearing all scripts on page load for extension debugger.
 
 ## 13.1.0
 - Update _fe_analyzer_shared to version ^38.0.0.

dwds/lib/src/debugging/debugger.dart

@@ -471,6 +471,7 @@ class Debugger extends Domain {
     if (url == null) {
       logger.severe('Failed to create dart frame for ${frame.functionName}: '
           'cannot find location for script ${location.scriptId}');
+      return null;
     }
 
     // TODO(sdk/issues/37240) - ideally we look for an exact location instead
@@ -740,7 +741,7 @@ String breakpointIdFor(String scriptId, int line, int column) =>
 
 /// Keeps track of the Dart and JS breakpoint Ids that correspond.
 class _Breakpoints extends Domain {
-  final logger = Logger('Breakpoints');
+  final _logger = Logger('Breakpoints');
   final _dartIdByJsId = <String, String>{};
   final _jsIdByDartId = <String, String>{};
 
@@ -768,10 +769,9 @@ class _Breakpoints extends Domain {
     var location = await locations.locationForDart(dartUri, line, column);
     // TODO: Handle cases where a breakpoint can't be set exactly at that line.
     if (location == null) {
-      logger
-          .fine('Failed to set breakpoint at ${dartScript.uri}:$line:$column: '
-              'Dart location not found for scriptId: $scriptId, '
-              'server path: ${dartUri.serverPath}, root:$root');
+      _logger.fine('Failed to set breakpoint $id '
+          '(${dartUri.serverPath}:$line:$column): '
+          'cannot find Dart location.');
       throw RPCError(
           'addBreakpoint',
           102,

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

@@ -65,7 +65,6 @@ class MetadataProvider {
         'dart:svg',
         'dart:web_audio',
         'dart:web_gl',
-        'dart:web_sql',
         'dart:ui',
       ];
 

dwds/lib/src/handlers/dev_handler.dart

@@ -93,6 +93,7 @@ class DevHandler {
     this._launchDevToolsInNewWindow,
     this._sdkConfigurationProvider,
   ) {
+    _validateDevToolsOptions();
     _subs.add(buildResults.listen(_emitBuildResults));
     _listen();
     if (_extensionBackend != null) {
@@ -376,10 +377,12 @@ class DevHandler {
       debuggerStart: debuggerStart,
       devToolsStart: DateTime.now(),
     );
-    await _launchDevTools(
-        appServices.chromeProxyService.remoteDebugger,
-        _constructDevToolsUri(appServices.debugService.uri,
-            ideQueryParam: 'Dwds'));
+    if (_serveDevTools) {
+      await _launchDevTools(
+          appServices.chromeProxyService.remoteDebugger,
+          _constructDevToolsUri(appServices.debugService.uri,
+              ideQueryParam: 'Dwds'));
+    }
   }
 
   Future<AppConnection> _handleConnectRequest(
@@ -533,29 +536,45 @@ class DevHandler {
       appServices.dwdsStats.updateLoadTime(
           debuggerStart: debuggerStart, devToolsStart: DateTime.now());
 
-      // If we only want the URI, this means we are embedding Dart DevTools in
-      // Chrome DevTools. Therefore return early.
-      if (devToolsRequest.uriOnly != null && devToolsRequest.uriOnly) {
+      if (_serveDevTools) {
+        // If we only want the URI, this means we are embedding Dart DevTools in
+        // Chrome DevTools. Therefore return early.
+        if (devToolsRequest.uriOnly != null && devToolsRequest.uriOnly) {
+          final devToolsUri = _constructDevToolsUri(
+            encodedUri,
+            ideQueryParam: 'ChromeDevTools',
+          );
+          extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri);
+          return;
+        }
         final devToolsUri = _constructDevToolsUri(
           encodedUri,
-          ideQueryParam: 'ChromeDevTools',
+          ideQueryParam: 'DebugExtension',
         );
-        extensionDebugger.sendEvent('dwds.devtoolsUri', devToolsUri);
-        return;
+        await _launchDevTools(extensionDebugger, devToolsUri);
       }
-      final devToolsUri = _constructDevToolsUri(
-        encodedUri,
-        ideQueryParam: 'DebugExtension',
-      );
-      await _launchDevTools(extensionDebugger, devToolsUri);
     });
   }
 
+  void _ensureServeDevtools() {
+    if (!_serveDevTools) {
+      _logger.severe('Expected _serveDevTools');
+      throw StateError('Expected _serveDevTools');
+    }
+  }
+
+  void _validateDevToolsOptions() {
+    if (_serveDevTools && _devTools == null) {
+      _logger.severe('DevHandler: invalid DevTools options');
+      throw StateError('DevHandler: invalid DevTools options');
+    }
+  }
+
   Future<void> _launchDevTools(
       RemoteDebugger remoteDebugger, String devToolsUri) async {
+    _ensureServeDevtools();
     // TODO(grouma) - We may want to log the debugServiceUri if we don't launch
     // DevTools so that users can manually connect.
-    if (!_serveDevTools) return;
     emitEvent(DwdsEvent.devtoolsLaunch());
     await remoteDebugger.sendCommand('Target.createTarget', params: {
       'newWindow': _launchDevToolsInNewWindow,
@@ -567,6 +586,7 @@ class DevHandler {
     String debugServiceUri, {
     String ideQueryParam = '',
   }) {
+    _ensureServeDevtools();
     return Uri(
         scheme: 'http',
         host: _devTools.hostname,

dwds/lib/src/injected/client.js

@@ -263,7 +263,7 @@ class ExtensionDebugger implements RemoteDebugger {
 
   @override
   Stream<GlobalObjectClearedEvent> get onGlobalObjectCleared => eventStream(
-      'Page.frameStartedLoading',
+      'Debugger.globalObjectCleared',
       (WipEvent event) => GlobalObjectClearedEvent(event.json));
 
   @override

dwds/lib/src/servers/extension_debugger.dart

@@ -337,6 +337,17 @@ class ChromeProxyService implements VmServiceInterface {
       String isolateId, String scriptUri, int line,
       {int column}) async {
     await isInitialized;
+    if (Uri.parse(scriptUri).scheme == 'dart') {
+      _logger.finest('Cannot set breakpoint at $scriptUri:$line:$column: '
+          'breakpoints in dart SDK locations are not supported yet.');
+      // TODO(annagrin): Support setting breakpoints in dart SDK locations.
+      // Issue: https://github.com/dart-lang/webdev/issues/1584
+      throw RPCError(
+          'addBreakpoint',
+          102,
+          'The VM is unable to add a breakpoint '
+              'at the specified line or function');
+    }
     var dartUri = DartUri(scriptUri, uri);
     var ref = await _inspector.scriptRefFor(dartUri.serverPath);
     return (await _debugger)

dwds/lib/src/services/chrome_proxy_service.dart

@@ -4,10 +4,6 @@
 
 // @dart = 2.9
 
-import 'dart:io';
-
-// ignore: implementation_imports
-import 'package:_fe_analyzer_shared/src/util/libraries_specification.dart';
 import 'package:logging/logging.dart';
 import 'package:package_config/package_config.dart';
 import 'package:path/path.dart' as p;
@@ -41,6 +37,8 @@ class DartUri {
   factory DartUri(String uri, [String serverUri]) {
     var serverPath = globalLoadStrategy.serverPathForAppUri(uri);
     if (serverPath != null) return DartUri._(serverPath);
+    // TODO(annagrin): Support creating DartUris from `dart:` uris.
+    // Issue: https://github.com/dart-lang/webdev/issues/1584
     if (uri.startsWith('package:')) {
       return DartUri._fromPackageUri(uri, serverUri: serverUri);
     }
@@ -98,9 +96,6 @@ class DartUri {
   /// The way we resolve file: URLs into package: URLs
   static PackageConfig _packageConfig;
 
-  /// The way we resolve dart: URLs into org-dartland-sdk: URLs
-  static TargetLibrariesSpecification _librariesSpec;
-
   /// SDK installation directory.
   ///
   /// Directory where the SDK client code built with is installed,
@@ -166,16 +161,12 @@ class DartUri {
     if (_sdkConfiguration.sdkDirectory != null) {
       _sdkConfiguration.validateSdkDir();
     }
-    if (_sdkConfiguration.librariesUri != null) {
-      await _loadLibrariesConfig(_sdkConfiguration.librariesUri);
-    }
 
     await _loadPackageConfig(packagesUri);
   }
 
   /// Clear the uri resolution tables.
   static void clear() {
-    _librariesSpec = null;
     _packageConfig = null;
     _resolvedUriToUri.clear();
     _uriToResolvedUri.clear();
@@ -199,20 +190,6 @@ class DartUri {
     });
   }
 
-  /// Load and parse libraries.json spec file.
-  /// Used for resolving `dart:` libraries uris.
-  static Future<void> _loadLibrariesConfig(Uri uri) async {
-    try {
-      var spec = await LibrariesSpecification.load(
-          uri, (uri) => File.fromUri(uri).readAsString());
-      _librariesSpec = spec.specificationFor('dartdevc');
-    } on LibrariesSpecificationException catch (e) {
-      _logger.warning('Cannot parse libraries spec: $uri', e);
-    } on FileSystemException catch (e) {
-      _logger.warning('Cannot read libraries spec: $uri', e);
-    }
-  }
-
   /// Record the library represented by package: or org-dartlang-app: uris
   /// indexed by absolute file: URI.
   static void _recordAbsoluteUri(String libraryUri) {
@@ -225,12 +202,9 @@ class DartUri {
     String libraryPath;
     switch (uri.scheme) {
       case 'dart':
-        var libSpec = _librariesSpec?.libraryInfoFor(uri.path);
-        libraryPath = libSpec?.uri?.path;
-        var sdkDir = _sdkConfiguration.sdkDirectoryUri;
-        libraryPath =
-            libraryPath?.replaceAll(sdkDir.path, 'org-dartlang-sdk:///sdk');
-        break;
+        // TODO(annagrin): Support resolving `dart:` uris.
+        // Issue: https://github.com/dart-lang/webdev/issues/1584
+        return;
       case 'org-dartlang-app':
       case 'google3':
         // Both currentDirectoryUri and the libraryUri path should have '/'
@@ -249,7 +223,7 @@ class DartUri {
       _uriToResolvedUri[libraryUri] = libraryPath;
       _resolvedUriToUri[libraryPath] = libraryUri;
     } else {
-      _logger.warning('Unresolved uri: $uri');
+      _logger.fine('Unresolved uri: $uri');
     }
   }
 }

dwds/lib/src/utilities/dart_uri.dart

@@ -10,7 +10,6 @@ environment:
   sdk: ">=2.16.0 <3.0.0"
 
 dependencies:
-  _fe_analyzer_shared: ^38.0.0
   async: ^2.3.0
   built_collection: ^5.0.0
   built_value: '>=6.7.0 <9.0.0'

dwds/pubspec.yaml

@@ -1356,7 +1356,7 @@ void main() {
         'org-dartlang-sdk:///sdk/lib/html/dart2js/html_dart2js.dart',
         'org-dartlang-sdk:///sdk/lib/async/async.dart',
       ]);
-    });
+    }, skip: 'https://github.com/dart-lang/webdev/issues/1584');
 
     test('lookupPackageUris finds package and org-dartlang-app paths',
         () async {
@@ -1404,7 +1404,7 @@ void main() {
         'dart:html',
         'dart:async',
       ]);
-    });
+    }, skip: 'https://github.com/dart-lang/webdev/issues/1584');
 
     test('registerService', () async {
       await expectLater(