Support VMService.evaluate with scope (#1340)

* Support scope in ChromeProxyService.evaluate()

Support evaluate with scope by
- wrapping the expression in a function with scope keys as parameters
- compiling the wrapper
- and using chrome's Runtime.callFunctionOn to evaluate the compiled
  call with arguments

Will be followed up with a cleanup of try/catch additions and removals.

Closes:  #1336

* Build

* Minor fixes and rebuild

* Addressed CR comments

Author: Anna Gringauze

dwds/CHANGELOG.md

@@ -6,6 +6,7 @@
 - Show lowered final fields using their original dart names.
 - Limit simultaneous connections to asset server to prevent broken sockets.
 - Fix hangs in hot restart.
+- Initial support for passing scope to `ChromeProxyService.evaluate`.
 
 ## 11.1.1
 

dwds/lib/src/debugging/inspector.dart

@@ -208,6 +208,25 @@ class AppInspector extends Domain {
     return RemoteObject(result.result['result'] as Map<String, Object>);
   }
 
+  /// Calls Chrome's Runtime.callFunctionOn method with a global function.
+  ///
+  /// [evalExpression] should be a JS function definition that can accept
+  /// [arguments].
+  Future<RemoteObject> _jsCallFunction(
+      String evalExpression, List<RemoteObject> arguments,
+      {bool returnByValue = false}) async {
+    var jsArguments = arguments.map(callArgumentFor).toList();
+    var result =
+        await remoteDebugger.sendCommand('Runtime.callFunctionOn', params: {
+      'functionDeclaration': evalExpression,
+      'arguments': jsArguments,
+      'executionContextId': await contextId,
+      'returnByValue': returnByValue,
+    });
+    handleErrorIfPresent(result, evalContents: evalExpression);
+    return RemoteObject(result.result['result'] as Map<String, Object>);
+  }
+
   Future<RemoteObject> evaluate(
       String isolateId, String targetId, String expression,
       {Map<String, String> scope}) async {
@@ -313,6 +332,13 @@ function($argsString) {
     return _evaluateInLibrary(library, evalExpression, arguments);
   }
 
+  /// Call [function] with objects referred by [argumentIds] as arguments.
+  Future<RemoteObject> callFunction(
+      String function, Iterable<String> argumentIds) async {
+    var arguments = argumentIds.map(remoteObjectFor).toList();
+    return _jsCallFunction(function, arguments);
+  }
+
   Future<Library> getLibrary(String isolateId, String objectId) async {
     return await _getLibrary(isolateId, objectId);
   }

dwds/lib/src/services/chrome_proxy_service.dart

@@ -116,9 +116,6 @@ class ChromeProxyService implements VmServiceInterface {
       _skipLists,
       uri,
     );
-    _expressionEvaluator = _compiler == null
-        ? null
-        : ExpressionEvaluator(debugger, _locations, _modules, _compiler);
     _debuggerCompleter.complete(debugger);
   }
 
@@ -228,6 +225,10 @@ class ChromeProxyService implements VmServiceInterface {
       executionContext,
     );
 
+    _expressionEvaluator = _compiler == null
+        ? null
+        : ExpressionEvaluator(_inspector, _locations, _modules, _compiler);
+
     await debugger.reestablishBreakpoints(
         _previousBreakpoints, _disabledBreakpoints);
     _disabledBreakpoints.clear();
@@ -434,7 +435,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
         var library = await _inspector?.getLibrary(isolateId, targetId);
         var result = await _getEvaluationResult(
             () => _expressionEvaluator.evaluateExpression(
-                isolateId, library.uri, expression),
+                isolateId, library.uri, expression, scope),
             expression);
         if (result is ErrorRef) {
           error = result;
@@ -473,9 +474,19 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
         await isCompilerInitialized;
         _validateIsolateId(isolateId);
 
+        if (scope != null) {
+          // TODO(annagrin): Implement scope support.
+          // Issue: https://github.com/dart-lang/webdev/issues/1344
+          throw RPCError(
+              'evaluateInFrame',
+              RPCError.kInvalidRequest,
+              'Expression evaluation with scope is not supported '
+                  'for this configuration.');
+        }
+
         var result = await _getEvaluationResult(
             () => _expressionEvaluator.evaluateExpressionInFrame(
-                isolateId, frameIndex, expression),
+                isolateId, frameIndex, expression, scope),
             expression);
 
         if (result is ErrorRef) {

dwds/lib/src/services/expression_evaluator.dart

@@ -8,7 +8,7 @@ import 'package:logging/logging.dart';
 import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 
 import '../debugging/dart_scope.dart';
-import '../debugging/debugger.dart';
+import '../debugging/inspector.dart';
 import '../debugging/location.dart';
 import '../debugging/modules.dart';
 import '../utilities/objects.dart' as chrome;
@@ -33,7 +33,7 @@ class ErrorKind {
 /// collect context for evaluation (scope, types, modules), and using
 /// ExpressionCompilerInterface to compile dart expressions to JavaScript.
 class ExpressionEvaluator {
-  final Future<Debugger> _debugger;
+  final AppInspector _inspector;
   final Locations _locations;
   final Modules _modules;
   final ExpressionCompiler _compiler;
@@ -43,7 +43,7 @@ class ExpressionEvaluator {
       RegExp('org-dartlang-debug:synthetic_debug_expression:.*:.*Error: ');
 
   ExpressionEvaluator(
-      this._debugger, this._locations, this._modules, this._compiler);
+      this._inspector, this._locations, this._modules, this._compiler);
 
   RemoteObject _createError(ErrorKind severity, String message) {
     return RemoteObject(
@@ -62,7 +62,11 @@ class ExpressionEvaluator {
   /// [libraryUri] dart library to evaluate the expression in.
   /// [expression] dart expression to evaluate.
   Future<RemoteObject> evaluateExpression(
-      String isolateId, String libraryUri, String expression) async {
+    String isolateId,
+    String libraryUri,
+    String expression,
+    Map<String, String> scope,
+  ) async {
     if (_compiler == null) {
       return _createError(ErrorKind.internal,
           'ExpressionEvaluator needs an ExpressionCompiler');
@@ -74,6 +78,10 @@ class ExpressionEvaluator {
 
     var module = await _modules.moduleForlibrary(libraryUri);
 
+    if (scope != null && scope.isNotEmpty) {
+      var params = scope.keys.join(', ');
+      expression = '($params) => $expression';
+    }
     _logger.finest('Evaluating "$expression" at $module');
 
     // Compile expression using an expression compiler, such as
@@ -88,8 +96,22 @@ class ExpressionEvaluator {
     }
 
     // Send JS expression to chrome to evaluate.
-    var result = await (await _debugger).evaluate(jsResult);
-    result = _formatEvaluationError(result);
+    RemoteObject result;
+    if (scope != null && scope.isNotEmpty) {
+      // Strip try/catch.
+      // TODO: remove adding try/catch block in expression compiler.
+      // https://github.com/dart-lang/webdev/issues/1341
+      var lines = jsResult.split('\n');
+      var inner = lines.getRange(2, lines.length - 3).join('\n');
+      var function = 'function(t) {'
+          '  return $inner(t);'
+          '}';
+      result = await _inspector.callFunction(function, scope.values);
+      result = _formatEvaluationError(result);
+    } else {
+      result = await _inspector.debugger.evaluate(jsResult);
+      result = _formatEvaluationError(result);
+    }
 
     _logger.finest('Evaluated "$expression" to "$result"');
     return result;
@@ -107,8 +129,8 @@ class ExpressionEvaluator {
   /// [isolateId] current isolate ID.
   /// [frameIndex] JavaScript frame to evaluate the expression in.
   /// [expression] dart expression to evaluate.
-  Future<RemoteObject> evaluateExpressionInFrame(
-      String isolateId, int frameIndex, String expression) async {
+  Future<RemoteObject> evaluateExpressionInFrame(String isolateId,
+      int frameIndex, String expression, Map<String, String> scope) async {
     if (_compiler == null) {
       return _createError(ErrorKind.internal,
           'ExpressionEvaluator needs an ExpressionCompiler');
@@ -119,7 +141,7 @@ class ExpressionEvaluator {
     }
 
     // Get JS scope and current JS location.
-    var jsFrame = (await _debugger).jsFrameForIndex(frameIndex);
+    var jsFrame = _inspector.debugger.jsFrameForIndex(frameIndex);
     if (jsFrame == null) {
       return _createError(
           ErrorKind.internal, 'No frame with index $frameIndex');
@@ -175,7 +197,7 @@ class ExpressionEvaluator {
     }
 
     // Send JS expression to chrome to evaluate.
-    var result = await (await _debugger)
+    var result = await _inspector.debugger
         .evaluateJsOnCallFrameIndex(frameIndex, jsResult);
     result = _formatEvaluationError(result);
 
@@ -271,7 +293,7 @@ class ExpressionEvaluator {
     var scopeChain = filterScopes(frame).reversed;
     for (var scope in scopeChain) {
       var scopeProperties =
-          await (await _debugger).getProperties(scope.object.objectId);
+          await _inspector.debugger.getProperties(scope.object.objectId);
 
       collectVariables(scope.scope, scopeProperties);
     }

dwds/test/build_daemon_evaluate_test.dart

@@ -126,6 +126,27 @@ void main() async {
             await setup.service.resume(isolate.id);
           });
 
+          test('with scope override is not supported yet', () async {
+            await onBreakPoint(isolate.id, mainScript, 'printLocal', () async {
+              var event = await stream.firstWhere(
+                  (event) => event.kind == EventKind.kPauseBreakpoint);
+
+              var object = await setup.service.evaluateInFrame(
+                  isolate.id, event.topFrame.index, 'MainClass(0)');
+
+              var param = object as InstanceRef;
+
+              expect(
+                  () => setup.service.evaluateInFrame(
+                        isolate.id,
+                        event.topFrame.index,
+                        't.toString()',
+                        scope: {'t': param.id},
+                      ),
+                  throwsRPCError);
+            });
+          });
+
           test('local', () async {
             await onBreakPoint(isolate.id, mainScript, 'printLocal', () async {
               var event = await stream.firstWhere(
@@ -477,6 +498,24 @@ void main() async {
 
           tearDown(() async {});
 
+          test('with scope override', () async {
+            var library = isolate.rootLib;
+            var object = await setup.service
+                .evaluate(isolate.id, library.id, 'MainClass(0)');
+
+            var param = object as InstanceRef;
+            var result = await setup.service.evaluate(
+                isolate.id, library.id, 't.toString()',
+                scope: {'t': param.id});
+
+            expect(
+                result,
+                const TypeMatcher<InstanceRef>().having(
+                    (instance) => instance.valueAsString,
+                    'valueAsString',
+                    '0'));
+          });
+
           test('uses symbol from the same library', () async {
             var library = isolate.rootLib;
             var result = await setup.service