Migrate debugging class and library helpers to null safety (#1669)

* Migrate modules and locations to null safety

* Remove circular dependencies

* Remove circular dependencies and unused code

* Simplified expression evaluation code

* Cleanup

* Fix failing tests

* Move more helpers from Domain to ChromeProxyService

* Build

* Migrate some files in debugger directory to null safety

* Minor fixes

* Migrate debugging class and library helpers to null safety

* Update AppInspectorInterface

* Fix crash in inferring library root

Author: Anna Gringauze

dwds/lib/src/debugging/classes.dart

@@ -2,8 +2,6 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.import 'dart:async';
 
-// @dart = 2.9
-
 import 'package:vm_service/vm_service.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
@@ -26,7 +24,9 @@ class ClassHelper extends Domain {
       classRefForUnknown
     ];
     for (var classRef in staticClasses) {
-      _classes[classRef.id] = Class(
+      final classId = classRef.id;
+      if (classId != null) {
+        _classes[classId] = Class(
           name: classRef.name,
           isAbstract: false,
           isConst: false,
@@ -35,15 +35,17 @@ class ClassHelper extends Domain {
           fields: [],
           functions: [],
           subclasses: [],
-          id: classRef.id,
-          traceAllocations: false);
+          id: classId,
+          traceAllocations: false,
+        );
+      }
     }
   }
 
   /// Returns the [Class] that corresponds to the provided [objectId].
   ///
   /// If a corresponding class does not exist it will return null.
-  Future<Class> forObjectId(String objectId) async {
+  Future<Class?> forObjectId(String objectId) async {
     if (!objectId.startsWith('classes|')) return null;
     var clazz = _classes[objectId];
     if (clazz != null) return clazz;
@@ -66,12 +68,18 @@ class ClassHelper extends Domain {
 
   /// Constructs a [Class] instance for the provided [LibraryRef] and
   /// [ClassRef].
-  Future<Class> _constructClass(
+  Future<Class?> _constructClass(
       LibraryRef libraryRef, ClassRef classRef) async {
-    final rawName = classRef.name.split('<').first;
+    final libraryUri = libraryRef.uri;
+    final className = classRef.name;
+    final classId = classRef.id;
+
+    if (libraryUri == null || classId == null || className == null) return null;
+
+    final rawName = className.split('<').first;
     final expression = '''
     (function() {
-      ${globalLoadStrategy.loadLibrarySnippet(libraryRef.uri)}
+      ${globalLoadStrategy.loadLibrarySnippet(libraryUri)}
       var result = {};
       var clazz = library["$rawName"];
       var descriptor = {
@@ -171,7 +179,7 @@ class ClassHelper extends Domain {
         classDescriptor['staticMethods'] as Map<String, dynamic>;
     methodDescriptors.addAll(staticMethodDescriptors);
     methodDescriptors.forEach((name, descriptor) {
-      final methodId = 'methods|${classRef.id}|$name';
+      final methodId = 'methods|$classId|$name';
       methodRefs.add(FuncRef(
           id: methodId,
           name: name,
@@ -237,7 +245,7 @@ class ClassHelper extends Domain {
         fields: fieldRefs,
         functions: methodRefs,
         subclasses: [],
-        id: classRef.id,
+        id: classId,
         traceAllocations: false);
   }
 }

dwds/lib/src/debugging/inspector.dart

@@ -301,11 +301,12 @@ class AppInspector implements AppInspectorInterface {
   /// Evaluate [expression] by calling Chrome's Runtime.evaluate.
   @override
   Future<RemoteObject> jsEvaluate(String expression,
-      {bool awaitPromise = false}) async {
+      {bool returnByValue = false, bool awaitPromise = false}) async {
     // TODO(alanknight): Support a version with arguments if needed.
     WipResponse result;
     result = await remoteDebugger.sendCommand('Runtime.evaluate', params: {
       'expression': expression,
+      'returnByValue': returnByValue,
       'awaitPromise': awaitPromise,
       'contextId': await contextId,
     });

dwds/lib/src/debugging/libraries.dart

@@ -2,13 +2,14 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.import 'dart:async';
 
-// @dart = 2.9
-
+import 'package:collection/collection.dart';
 import 'package:logging/logging.dart';
 import 'package:vm_service/vm_service.dart';
+import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
 import '../loaders/strategy.dart';
 import '../utilities/domain.dart';
+import '../services/chrome_debug_exception.dart';
 import 'metadata/class.dart';
 
 /// Keeps track of Dart libraries available in the running application.
@@ -21,24 +22,24 @@ class LibraryHelper extends Domain {
   /// Map of libraryRef ID to [LibraryRef].
   final _libraryRefsById = <String, LibraryRef>{};
 
-  LibraryRef _rootLib;
+  LibraryRef? _rootLib;
 
   LibraryHelper(AppInspectorInterface appInspector) {
     inspector = appInspector;
   }
 
   Future<LibraryRef> get rootLib async {
-    if (_rootLib != null) return _rootLib;
+    if (_rootLib != null) return _rootLib!;
     // TODO: read entrypoint from app metadata.
     // Issue: https://github.com/dart-lang/webdev/issues/1290
     final libraries = await libraryRefs;
-    _rootLib = libraries.firstWhere((lib) => lib.name.contains('org-dartlang'),
-        orElse: () => null);
+    _rootLib = libraries
+        .firstWhereOrNull((lib) => lib.name?.contains('org-dartlang') ?? false);
     _rootLib = _rootLib ??
-        libraries.firstWhere((lib) => lib.name.contains('main'),
-            orElse: () => null);
+        libraries
+            .firstWhereOrNull((lib) => lib.name?.contains('main') ?? false);
     _rootLib = _rootLib ?? (libraries.isNotEmpty ? libraries.last : null);
-    return _rootLib;
+    return _rootLib!;
   }
 
   /// Returns all libraryRefs in the app.
@@ -56,22 +57,29 @@ class LibraryHelper extends Domain {
     return _libraryRefsById.values.toList();
   }
 
-  Future<Library> libraryFor(LibraryRef libraryRef) async {
-    final library = _librariesById[libraryRef.id];
-    if (library != null) return library;
-    return _librariesById[libraryRef.id] = await _constructLibrary(libraryRef);
+  Future<Library?> libraryFor(LibraryRef libraryRef) async {
+    final libraryId = libraryRef.id;
+    if (libraryId == null) return null;
+    final library =
+        _librariesById[libraryId] ?? await _constructLibrary(libraryRef);
+    if (library == null) return null;
+    return _librariesById[libraryId] = library;
   }
 
-  Future<LibraryRef> libraryRefFor(String objectId) async {
+  Future<LibraryRef?> libraryRefFor(String objectId) async {
     if (_libraryRefsById.isEmpty) await libraryRefs;
     return _libraryRefsById[objectId];
   }
 
-  Future<Library> _constructLibrary(LibraryRef libraryRef) async {
+  Future<Library?> _constructLibrary(LibraryRef libraryRef) async {
+    final libraryId = libraryRef.id;
+    final libraryUri = libraryRef.uri;
+    if (libraryId == null || libraryUri == null) return null;
+
     // Fetch information about all the classes in this library.
     final expression = '''
     (function() {
-      ${globalLoadStrategy.loadLibrarySnippet(libraryRef.uri)}
+      ${globalLoadStrategy.loadLibrarySnippet(libraryUri)}
       var result = {};
       var classes = Object.values(Object.getOwnPropertyDescriptors(library))
         .filter((p) => 'value' in p)
@@ -88,41 +96,41 @@ class LibraryHelper extends Domain {
       return result;
     })()
     ''';
-    final result =
-        await inspector.remoteDebugger.sendCommand('Runtime.evaluate', params: {
-      'expression': expression,
-      'returnByValue': true,
-      'contextId': await inspector.contextId,
-    });
-    List<ClassRef> classRefs;
-    if (result.result.containsKey('exceptionDetails')) {
+    RemoteObject? result;
+    try {
+      result = await inspector.jsEvaluate(expression, returnByValue: true);
+    } on ChromeDebugException catch (_) {
       // Unreferenced libraries are not loaded at runtime,
       // return empty library object for consistency among
       // VM Service implementations.
       // TODO: Collect library and class information from debug symbols.
       _logger.warning('Library ${libraryRef.uri} is not loaded. '
           'This can happen for unreferenced libraries.');
-    } else {
+    }
+    List<ClassRef>? classRefs;
+    if (result != null) {
       final classDescriptors =
-          (result.result['result']['value']['classes'] as List)
-              .cast<Map<String, Object>>();
-      classRefs = classDescriptors.map<ClassRef>((classDescriptor) {
+          ((result.value as Map<String, dynamic>)['classes'] as List?)
+              ?.cast<Map<String, Object>>();
+      classRefs = classDescriptors?.map<ClassRef>((classDescriptor) {
         final classMetaData = ClassMetaData(
-            jsName: classDescriptor['name'],
-            libraryId: libraryRef.id,
-            dartName: classDescriptor['dartName']);
+          jsName: classDescriptor['name'],
+          libraryId: libraryRef.id,
+          dartName: classDescriptor['dartName'],
+        );
         return classMetaData.classRef;
       }).toList();
     }
     return Library(
-        name: libraryRef.name,
-        uri: libraryRef.uri,
-        debuggable: true,
-        dependencies: [],
-        scripts: await inspector.scriptRefsForLibrary(libraryRef.id),
-        variables: [],
-        functions: [],
-        classes: classRefs,
-        id: libraryRef.id);
+      name: libraryRef.name,
+      uri: libraryRef.uri,
+      debuggable: true,
+      dependencies: [],
+      scripts: await inspector.scriptRefsForLibrary(libraryId),
+      variables: [],
+      functions: [],
+      classes: classRefs,
+      id: libraryId,
+    );
   }
 }

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

@@ -2,12 +2,10 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.import 'dart:async';
 
-// @dart = 2.9
-
+import 'package:dwds/src/utilities/domain.dart';
 import 'package:vm_service/vm_service.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
-import '../../debugging/inspector.dart';
 import '../../debugging/remote_debugger.dart';
 import '../../loaders/strategy.dart';
 import '../../services/chrome_debug_exception.dart';
@@ -22,34 +20,36 @@ final classRefForString = classRefFor('dart:core', InstanceKind.kString);
 final classRefForUnknown = classRefFor('dart:core', 'Unknown');
 
 /// Returns a [ClassRef] for the provided library ID and class name.
-ClassRef classRefFor(String libraryId, String name) => ClassRef(
+ClassRef classRefFor(String? libraryId, String? name) => ClassRef(
     id: 'classes|$libraryId|$name',
     name: name,
-    library: LibraryRef(id: libraryId, name: libraryId, uri: libraryId));
+    library: libraryId == null
+        ? null
+        : LibraryRef(id: libraryId, name: libraryId, uri: libraryId));
 
 /// Meta data for a remote Dart class in Chrome.
 class ClassMetaData {
   /// The name of the JS constructor for the object.
   ///
   /// This may be a constructor for a Dart, but it's still a JS name. For
   /// example, 'Number', 'JSArray', 'Object'.
-  final String jsName;
+  final String? jsName;
 
   /// The length of the object, if applicable.
-  final int length;
+  final int? length;
 
   /// The dart type name for the object.
   ///
   /// For example, 'int', 'List<String>', 'Null'
-  final String dartName;
+  final String? dartName;
 
   /// The library identifier, which is the URI of the library.
-  final String libraryId;
+  final String? libraryId;
 
   factory ClassMetaData(
-      {Object jsName, Object libraryId, Object dartName, Object length}) {
-    return ClassMetaData._(jsName as String, libraryId as String,
-        dartName as String, int.tryParse('$length'));
+      {Object? jsName, Object? libraryId, Object? dartName, Object? length}) {
+    return ClassMetaData._(jsName as String?, libraryId as String?,
+        dartName as String?, int.tryParse('$length'));
   }
 
   ClassMetaData._(this.jsName, this.libraryId, this.dartName, this.length);
@@ -62,8 +62,8 @@ class ClassMetaData {
   /// Returns the [ClassMetaData] for the Chrome [remoteObject].
   ///
   /// Returns null if the [remoteObject] is not a Dart class.
-  static Future<ClassMetaData> metaDataFor(RemoteDebugger remoteDebugger,
-      RemoteObject remoteObject, AppInspector inspector) async {
+  static Future<ClassMetaData?> metaDataFor(RemoteDebugger remoteDebugger,
+      RemoteObject remoteObject, AppInspectorInterface inspector) async {
     try {
       final evalExpression = '''
       function(arg) {

dwds/lib/src/utilities/domain.dart

@@ -72,7 +72,8 @@ abstract class AppInspectorInterface {
       String targetId, String selector, List<dynamic> arguments);
 
   /// Evaluate [expression] by calling Chrome's `Runtime.evaluate`.
-  Future<RemoteObject> jsEvaluate(String expression);
+  Future<RemoteObject> jsEvaluate(String expression,
+      {bool returnByValue = false, bool awaitPromise = false});
 
   /// Lookup an `object` from some isolate by its [objectId].
   Future<Obj> getObject(String objectId, {int offset, int count});

dwds/pubspec.yaml

@@ -13,6 +13,7 @@ dependencies:
   async: ^2.9.0
   built_collection: ^5.1.1
   built_value: ^8.3.0
+  collection: ^1.15.0
   crypto: ^3.0.2
   dds: ^2.2.5
   file: ^6.1.2