Remove circular dependencies and unused code in preparation for further null safety migration

dwds/CHANGELOG.md

@@ -6,6 +6,7 @@
 - Fix a bug where evaluation would fail with more than one parameter in
   the scope.
 - Remove showing uncaptured values from the stack during evaluation.
+- Refactor code to break most circular dependencies between files.
 
 **Breaking changes**
 - Remove no longer used `ExpressionCompilerService.handler`.

dwds/lib/src/debugging/classes.dart

@@ -11,30 +11,15 @@ import '../../src/services/chrome_debug_exception.dart';
 import '../loaders/strategy.dart';
 import '../utilities/domain.dart';
 import '../utilities/shared.dart';
-import 'inspector.dart';
 import 'metadata/class.dart';
 
-/// A hard-coded ClassRef for the Closure class.
-final classRefForClosure = classRefFor('dart:core', 'Closure');
-
-/// A hard-coded ClassRef for the String class.
-final classRefForString = classRefFor('dart:core', InstanceKind.kString);
-
-/// A hard-coded ClassRef for a (non-existent) class called Unknown.
-final classRefForUnknown = classRefFor('dart:core', 'Unknown');
-
-/// Returns a [ClassRef] for the provided library ID and class name.
-ClassRef classRefFor(String libraryId, String name) => ClassRef(
-    id: 'classes|$libraryId|$name',
-    name: name,
-    library: LibraryRef(id: libraryId, name: libraryId, uri: libraryId));
-
 /// Keeps track of Dart classes available in the running application.
 class ClassHelper extends Domain {
   /// Map of class ID to [Class].
   final _classes = <String, Class>{};
 
-  ClassHelper(AppInspector Function() provider) : super(provider) {
+  ClassHelper(AppInspectorInterface appInspector) {
+    inspector = appInspector;
     final staticClasses = [
       classRefForClosure,
       classRefForString,
@@ -67,7 +52,7 @@ class ClassHelper extends Domain {
     if (libraryId == 'null') {
       throw UnsupportedError('unknown library: $libraryId');
     }
-    final libraryRef = await inspector.libraryHelper.libraryRefFor(libraryId);
+    final libraryRef = await inspector.libraryRefFor(libraryId);
     if (libraryRef == null) {
       throw Exception('Could not find library: $libraryId');
     }

dwds/lib/src/debugging/debugger.dart

@@ -15,7 +15,6 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
     hide StackTrace;
 
 import '../loaders/strategy.dart';
-import '../services/chrome_proxy_service.dart';
 import '../services/chrome_debug_exception.dart';
 import '../utilities/conversions.dart';
 import '../utilities/dart_uri.dart';
@@ -28,6 +27,10 @@ import 'location.dart';
 import 'remote_debugger.dart';
 import 'skip_list.dart';
 
+/// Adds [event] to the stream with [streamId] if there is anybody listening
+/// on that stream.
+typedef StreamNotify = void Function(String streamId, Event event);
+
 /// Converts from ExceptionPauseMode strings to [PauseState] enums.
 ///
 /// Values defined in:
@@ -51,16 +54,13 @@ class Debugger extends Domain {
   Debugger._(
     this._remoteDebugger,
     this._streamNotify,
-    AppInspectorProvider provider,
     this._locations,
     this._skipLists,
     this._root,
-  )   : _breakpoints = _Breakpoints(
+  ) : _breakpoints = _Breakpoints(
             locations: _locations,
-            provider: provider,
             remoteDebugger: _remoteDebugger,
-            root: _root),
-        super(provider);
+            root: _root);
 
   /// The breakpoints we have set so far, indexable by either
   /// Dart or JS ID.
@@ -86,15 +86,19 @@ class Debugger extends Domain {
 
   bool _isStepping = false;
 
+  void updateInspector(AppInspectorInterface appInspector) {
+    inspector = appInspector;
+    _breakpoints.inspector = appInspector;
+  }
+
   Future<Success> pause() async {
     _isStepping = false;
     final result = await _remoteDebugger.pause();
     handleErrorIfPresent(result);
     return Success();
   }
 
-  Future<Success> setExceptionPauseMode(String isolateId, String mode) async {
-    checkIsolate('setExceptionPauseMode', isolateId);
+  Future<Success> setExceptionPauseMode(String mode) async {
     mode = mode?.toLowerCase();
     if (!_pauseModePauseStates.containsKey(mode)) {
       throwInvalidParam('setExceptionPauseMode', 'Unsupported mode: $mode');
@@ -117,9 +121,7 @@ class Debugger extends Domain {
   ///
   /// Note that stepping will automatically continue until Chrome is paused at
   /// a location for which we have source information.
-  Future<Success> resume(String isolateId,
-      {String step, int frameIndex}) async {
-    checkIsolate('resume', isolateId);
+  Future<Success> resume({String step, int frameIndex}) async {
     if (frameIndex != null) {
       throw ArgumentError('FrameIndex is currently unsupported.');
     }
@@ -152,9 +154,7 @@ class Debugger extends Domain {
   /// Returns null if the debugger is not paused.
   ///
   /// The returned stack will contain up to [limit] frames if provided.
-  Future<Stack> getStack(String isolateId, {int limit}) async {
-    checkIsolate('getStack', isolateId);
-
+  Future<Stack> getStack({int limit}) async {
     if (stackComputer == null) {
       throw RPCError('getStack', RPCError.kInternalError,
           'Cannot compute stack when application is not paused');
@@ -170,15 +170,13 @@ class Debugger extends Domain {
   static Future<Debugger> create(
     RemoteDebugger remoteDebugger,
     StreamNotify streamNotify,
-    AppInspectorProvider appInspectorProvider,
     Locations locations,
     SkipLists skipLists,
     String root,
   ) async {
     final debugger = Debugger._(
       remoteDebugger,
       streamNotify,
-      appInspectorProvider,
       locations,
       skipLists,
       root,
@@ -224,13 +222,11 @@ class Debugger extends Domain {
   ///
   /// Note that line and column are Dart source locations and are one-based.
   Future<Breakpoint> addBreakpoint(
-    String isolateId,
     String scriptId,
     int line, {
     int column,
   }) async {
     column ??= 0;
-    checkIsolate('addBreakpoint', isolateId);
     final breakpoint = await _breakpoints.add(scriptId, line, column);
     _notifyBreakpoint(breakpoint);
     return breakpoint;
@@ -264,9 +260,7 @@ class Debugger extends Domain {
     // Disabled breakpoints were actually removed from Chrome so simply add
     // them back.
     for (var breakpoint in disabledBreakpoints) {
-      await addBreakpoint(
-          inspector.isolate.id,
-          (await _updatedScriptRefFor(breakpoint)).id,
+      await addBreakpoint((await _updatedScriptRefFor(breakpoint)).id,
           _lineNumberFor(breakpoint),
           column: _columnNumberFor(breakpoint));
     }
@@ -283,9 +277,7 @@ class Debugger extends Domain {
   }
 
   /// Remove a Dart breakpoint.
-  Future<Success> removeBreakpoint(
-      String isolateId, String breakpointId) async {
-    checkIsolate('removeBreakpoint', isolateId);
+  Future<Success> removeBreakpoint(String breakpointId) async {
     if (!_breakpoints._bpByDartId.containsKey(breakpointId)) {
       throwInvalidParam(
           'removeBreakpoint', 'invalid breakpoint id $breakpointId');
@@ -361,8 +353,7 @@ class Debugger extends Domain {
   Future<BoundVariable> _boundVariable(Property property) async {
     // We return one level of properties from this object. Sub-properties are
     // another round trip.
-    final instanceRef =
-        await inspector.instanceHelper.instanceRefFor(property.value);
+    final instanceRef = await inspector.instanceRefFor(property.value);
     // Skip null instance refs, which we get for weird objects, e.g.
     // properties that are getter/setter pairs.
     // TODO(alanknight): Handle these properly.
@@ -552,7 +543,7 @@ class Debugger extends Domain {
         if (map['type'] == 'object') {
           // The className here is generally 'DartError'.
           final obj = RemoteObject(map);
-          exception = await inspector.instanceHelper.instanceRefFor(obj);
+          exception = await inspector.instanceRefFor(obj);
 
           // TODO: The exception object generally doesn't get converted to a
           // Dart object (and instead has a classRef name of 'NativeJavaScriptObject').
@@ -561,8 +552,7 @@ class Debugger extends Domain {
               // Create a string exception object.
               final description =
                   await inspector.mapExceptionStackTrace(obj.description);
-              exception =
-                  await inspector.instanceHelper.instanceRefFor(description);
+              exception = await inspector.instanceRefFor(description);
             } else {
               exception = null;
             }
@@ -671,21 +661,6 @@ class Debugger extends Domain {
     logger.severe('Target crashed!');
   }
 
-  /// Evaluate [expression] by calling Chrome's Runtime.evaluate
-  Future<RemoteObject> evaluate(String expression) async {
-    try {
-      return await _remoteDebugger.evaluate(expression);
-    } on ExceptionDetails catch (e) {
-      throw ChromeDebugException(
-        e.json,
-        evalContents: expression,
-        additionalDetails: {
-          'Dart expression': expression,
-        },
-      );
-    }
-  }
-
   WipCallFrame jsFrameForIndex(int frameIndex) {
     if (stackComputer == null) {
       throw RPCError('evaluateInFrame', 106,
@@ -766,10 +741,9 @@ class _Breakpoints extends Domain {
 
   _Breakpoints({
     @required this.locations,
-    @required AppInspectorProvider provider,
     @required this.remoteDebugger,
     @required this.root,
-  }) : super(provider);
+  });
 
   Future<Breakpoint> _createBreakpoint(
       String id, String scriptId, int line, int column) async {