diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index bc0cdde6b..fe53aa6c2 100644 --- a/dwds/CHANGELOG.md +++ b/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`. diff --git a/dwds/lib/src/debugging/classes.dart b/dwds/lib/src/debugging/classes.dart index 112ad8994..a2c8ae927 100644 --- a/dwds/lib/src/debugging/classes.dart +++ b/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 = {}; - 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'); } diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 85bb1eb14..6b91e5e1c 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/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,6 +86,11 @@ class Debugger extends Domain { bool _isStepping = false; + void updateInspector(AppInspectorInterface appInspector) { + inspector = appInspector; + _breakpoints.inspector = appInspector; + } + Future pause() async { _isStepping = false; final result = await _remoteDebugger.pause(); @@ -93,8 +98,7 @@ class Debugger extends Domain { return Success(); } - Future setExceptionPauseMode(String isolateId, String mode) async { - checkIsolate('setExceptionPauseMode', isolateId); + Future 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 resume(String isolateId, - {String step, int frameIndex}) async { - checkIsolate('resume', isolateId); + Future 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 getStack(String isolateId, {int limit}) async { - checkIsolate('getStack', isolateId); - + Future getStack({int limit}) async { if (stackComputer == null) { throw RPCError('getStack', RPCError.kInternalError, 'Cannot compute stack when application is not paused'); @@ -170,7 +170,6 @@ class Debugger extends Domain { static Future create( RemoteDebugger remoteDebugger, StreamNotify streamNotify, - AppInspectorProvider appInspectorProvider, Locations locations, SkipLists skipLists, String root, @@ -178,7 +177,6 @@ class Debugger extends Domain { 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 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 removeBreakpoint( - String isolateId, String breakpointId) async { - checkIsolate('removeBreakpoint', isolateId); + Future 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(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 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 _createBreakpoint( String id, String scriptId, int line, int column) async { diff --git a/dwds/lib/src/debugging/inspector.dart b/dwds/lib/src/debugging/inspector.dart index a2b737d06..67c303d86 100644 --- a/dwds/lib/src/debugging/inspector.dart +++ b/dwds/lib/src/debugging/inspector.dart @@ -10,8 +10,6 @@ import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; import '../connections/app_connection.dart'; -import '../debugging/location.dart'; -import '../debugging/remote_debugger.dart'; import '../loaders/strategy.dart'; import '../readers/asset_reader.dart'; import '../utilities/conversions.dart'; @@ -24,13 +22,15 @@ import 'debugger.dart'; import 'execution_context.dart'; import 'instance.dart'; import 'libraries.dart'; +import 'location.dart'; +import 'remote_debugger.dart'; /// An inspector for a running Dart application contained in the /// [WipConnection]. /// /// Provides information about currently loaded scripts and objects and support /// for eval. -class AppInspector extends Domain { +class AppInspector implements AppInspectorInterface { final _scriptCacheMemoizer = AsyncMemoizer>(); Future> get scriptRefs => _populateScriptCaches(); @@ -49,16 +49,27 @@ class AppInspector extends Domain { /// Map of [Library] id to included [ScriptRef]s. final _libraryIdToScriptRefs = >{}; - final RemoteDebugger remoteDebugger; - final Debugger debugger; - final Isolate isolate; - final IsolateRef isolateRef; - final AppConnection appConnection; + @override + RemoteDebugger get remoteDebugger => _remoteDebugger; + final RemoteDebugger _remoteDebugger; + + @override + Isolate get isolate => _isolate; + final Isolate _isolate; + + @override + IsolateRef get isolateRef => _isolateRef; + final IsolateRef _isolateRef; + + @override + AppConnection get appConnection => _appConnection; + final AppConnection _appConnection; + final ExecutionContext _executionContext; - final LibraryHelper libraryHelper; - final ClassHelper classHelper; - final InstanceHelper instanceHelper; + LibraryHelper _libraryHelper; + ClassHelper _classHelper; + InstanceHelper _instanceHelper; final AssetReader _assetReader; final Locations _locations; @@ -74,28 +85,27 @@ class AppInspector extends Domain { static final exceptionMessageRegex = RegExp(r'^.*$', multiLine: true); AppInspector._( - this.appConnection, - this.isolate, - this.remoteDebugger, - this.debugger, - this.libraryHelper, - this.classHelper, - this.instanceHelper, + this._appConnection, + this._isolate, + this._remoteDebugger, this._assetReader, this._locations, this._root, this._executionContext, this._sdkConfiguration, - ) : isolateRef = _toIsolateRef(isolate), - super.forInspector(); + ) : _isolateRef = _toIsolateRef(_isolate); - /// We are the inspector, so this getter is trivial. - @override - AppInspector get inspector => this; + Future initialize( + LibraryHelper libraryHelper, + ClassHelper classHelper, + InstanceHelper instanceHelper, + ) async { + _libraryHelper = libraryHelper; + _classHelper = classHelper; + _instanceHelper = instanceHelper; - Future _initialize() async { - final libraries = await libraryHelper.libraryRefs; - isolate.rootLib = await libraryHelper.rootLib; + final libraries = await _libraryHelper.libraryRefs; + isolate.rootLib = await _libraryHelper.rootLib; isolate.libraries.addAll(libraries); final scripts = await scriptRefs; @@ -114,7 +124,7 @@ class AppInspector extends Domain { isSystemIsolate: isolate.isSystemIsolate, ); - static Future initialize( + static Future create( AppConnection appConnection, RemoteDebugger remoteDebugger, AssetReader assetReader, @@ -150,30 +160,33 @@ class AppInspector extends Domain { isSystemIsolate: false, isolateFlags: []) ..extensionRPCs = []; - AppInspector appInspector; - AppInspector provider() => appInspector; - final libraryHelper = LibraryHelper(provider); - final classHelper = ClassHelper(provider); - final instanceHelper = InstanceHelper(provider); - appInspector = AppInspector._( + final inspector = AppInspector._( appConnection, isolate, remoteDebugger, - debugger, - libraryHelper, - classHelper, - instanceHelper, assetReader, locations, root, executionContext, sdkConfiguration, ); - await appInspector._initialize(); - return appInspector; + + debugger.updateInspector(inspector); + + final libraryHelper = LibraryHelper(inspector); + final classHelper = ClassHelper(inspector); + final instanceHelper = InstanceHelper(inspector, debugger); + + await inspector.initialize( + libraryHelper, + classHelper, + instanceHelper, + ); + return inspector; } /// Returns the ID for the execution context or null if not found. + @override Future get contextId async { try { return await _executionContext.id; @@ -184,6 +197,7 @@ class AppInspector extends Domain { } /// Get the value of the field named [fieldName] from [receiver]. + @override Future loadField(RemoteObject receiver, String fieldName) { final load = ''' function() { @@ -195,7 +209,7 @@ class AppInspector extends Domain { /// Call a method by name on [receiver], with arguments [positionalArgs] and /// [namedArgs]. - Future invokeMethod(RemoteObject receiver, String methodName, + Future _invokeMethod(RemoteObject receiver, String methodName, [List positionalArgs = const [], Map namedArgs = const {}]) async { // TODO(alanknight): Support named arguments. @@ -217,6 +231,7 @@ class AppInspector extends Domain { /// /// [evalExpression] should be a JS function definition that can accept /// [arguments]. + @override Future jsCallFunctionOn(RemoteObject receiver, String evalExpression, List arguments, {bool returnByValue = false}) async { @@ -258,18 +273,19 @@ class AppInspector extends Domain { /// invoking a top-level function. The [arguments] are always strings that are /// Dart object Ids (which can also be Chrome RemoteObject objectIds that are /// for non-Dart JS objects.) - Future invoke(String isolateId, String targetId, - String selector, List arguments) async { - checkIsolate('invoke', isolateId); + @override + Future invoke( + String targetId, String selector, List arguments) async { final remoteArguments = arguments.cast().map(remoteObjectFor).toList(); // We special case the Dart library, where invokeMethod won't work because // it's not really a Dart object. if (isLibraryId(targetId)) { - final library = await getObject(isolateId, targetId) as Library; + final library = await getObject(targetId) as Library; return await _invokeLibraryFunction(library, selector, remoteArguments); } else { - return invokeMethod(remoteObjectFor(targetId), selector, remoteArguments); + return _invokeMethod( + remoteObjectFor(targetId), selector, remoteArguments); } } @@ -283,6 +299,7 @@ class AppInspector extends Domain { } /// Evaluate [expression] by calling Chrome's Runtime.evaluate. + @override Future jsEvaluate(String expression, {bool awaitPromise = false}) async { // TODO(alanknight): Support a version with arguments if needed. @@ -313,35 +330,47 @@ class AppInspector extends Domain { } /// Call [function] with objects referred by [argumentIds] as arguments. + @override Future callFunction( String function, Iterable argumentIds) async { final arguments = argumentIds.map(remoteObjectFor).toList(); return _jsCallFunction(function, arguments); } - Future getLibrary(String isolateId, String objectId) async { - if (isolateId != isolate.id) return null; - final libraryRef = await libraryHelper.libraryRefFor(objectId); + @override + Future instanceRefFor(Object value) => + _instanceHelper.instanceRefFor(value); + + Future instanceFor(Object value) => + _instanceHelper.instanceFor(value); + + @override + Future libraryRefFor(String objectId) => + _libraryHelper.libraryRefFor(objectId); + + @override + Future getLibrary(String objectId) async { + final libraryRef = await libraryRefFor(objectId); if (libraryRef == null) return null; - return libraryHelper.libraryFor(libraryRef); + return _libraryHelper.libraryFor(libraryRef); } - Future getObject(String isolateId, String objectId, - {int offset, int count}) async { + @override + Future getObject(String objectId, {int offset, int count}) async { try { - final library = await getLibrary(isolateId, objectId); + final library = await getLibrary(objectId); if (library != null) { return library; } - final clazz = await classHelper.forObjectId(objectId); + final clazz = await _classHelper.forObjectId(objectId); if (clazz != null) { return clazz; } final scriptRef = _scriptRefsById[objectId]; if (scriptRef != null) { - return await _getScript(isolateId, scriptRef); + return await _getScript(scriptRef); } - final instance = await instanceHelper + final instance = await _instanceHelper .instanceFor(remoteObjectFor(objectId), offset: offset, count: count); if (instance != null) { return instance; @@ -354,7 +383,7 @@ class AppInspector extends Domain { 'are supported for getObject'); } - Future