support basic scope in evaluate calls (#344)

Author: jakemac53

dwds/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.3.2
+
+- Add support for `scope` in `evaluate` calls.
+
 ## 0.3.1
 
 - Improve error reporting for evals, give the full JS eval in the error message

dwds/example/hello_world/main.dart

@@ -51,7 +51,14 @@ void main() async {
 String helloString(String response) => response;
 bool helloBool(bool response) => response;
 num helloNum(num response) => response;
+MyTestClass createObject(String message) => MyTestClass(message: message);
+String messageFor(MyTestClass instance) => instance.message;
+String messagesCombined(MyTestClass a, MyTestClass b) => a.message + b.message;
 
 class MyTestClass {
-  String hello() => 'world';
+  final String message;
+
+  MyTestClass({this.message = 'world'});
+
+  String hello() => message;
 }

dwds/lib/src/chrome_proxy_service.dart

@@ -246,51 +246,80 @@ require("dart_sdk").developer.invokeExtension(
   @override
   Future evaluate(String isolateId, String targetId, String expression,
       {Map<String, String> scope, bool disableBreakpoints}) async {
+    scope ??= {};
+    disableBreakpoints ??= false;
     var library = await _getLibrary(isolateId, targetId);
     if (library == null) {
       throw UnsupportedError(
           'Evaluate is only supported when `targetId` is a library.');
     }
-    var evalExpression = '''
+    WipResponse result;
+
+    // If there is scope, we use `callFunctionOn` because that accepts the
+    // `arguments` option.
+    //
+    // If there is no scope we run a normal evaluate call.
+    if (scope.isEmpty) {
+      var evalExpression = '''
 (function() {
   ${_getLibrarySnippet(library.uri)};
   return library.$expression;
 })();
     ''';
-    var result = await tabConnection.runtime.sendCommand('Runtime.evaluate',
-        params: {'expression': evalExpression, 'returnByValue': true});
-    _handleErrorIfPresent(result,
-        evalContents: evalExpression,
-        additionalDetails: {
-          'Dart expression': expression,
-          'scope': scope,
-        });
+      result = await tabConnection.runtime.sendCommand('Runtime.evaluate',
+          params: {'expression': evalExpression});
+      _handleErrorIfPresent(result,
+          evalContents: evalExpression,
+          additionalDetails: {
+            'Dart expression': expression,
+          });
+    } else {
+      var argsString = scope.keys.join(', ');
+      var arguments = scope.values.map((id) => {'objectId': id}).toList();
+      var evalExpression = '''
+function($argsString) {
+  ${_getLibrarySnippet(library.uri)};
+  return library.$expression;
+}
+    ''';
+      result = await tabConnection.runtime
+          .sendCommand('Runtime.callFunctionOn', params: {
+        'functionDeclaration': evalExpression,
+        'arguments': arguments,
+        // TODO(jakemac): Use the executionContext instead, or possibly the
+        // library object. This will get weird if people try to use `this` in
+        // their expression.
+        'objectId': scope.values.first,
+      });
+      _handleErrorIfPresent(result,
+          evalContents: evalExpression,
+          additionalDetails: {
+            'Dart expression': expression,
+            'scope': scope,
+          });
+    }
     var remoteObject =
         RemoteObject(result.result['result'] as Map<String, dynamic>);
 
-    String kind;
-    var classRef = ClassRef()
-      ..id = 'dart:core:${remoteObject.type}'
-      ..name = remoteObject.type;
     switch (remoteObject.type) {
       case 'string':
-        kind = InstanceKind.kString;
-        break;
+        return _primitiveInstance(InstanceKind.kString, remoteObject);
       case 'number':
-        kind = InstanceKind.kDouble;
-        break;
+        return _primitiveInstance(InstanceKind.kDouble, remoteObject);
       case 'boolean':
-        kind = InstanceKind.kBool;
-        break;
+        return _primitiveInstance(InstanceKind.kBool, remoteObject);
+      case 'object':
+        return InstanceRef()
+          ..kind = InstanceKind.kPlainInstance
+          ..id = remoteObject.objectId
+          // TODO(jakemac): Create a real ClassRef, we need a way of looking
+          // up the library for a given instance to create it though.
+          // https://github.com/dart-lang/sdk/issues/36771.
+          ..classRef = ClassRef();
       default:
         throw UnsupportedError(
             'Unsupported response type ${remoteObject.type}');
     }
-
-    return InstanceRef()
-      ..valueAsString = '${remoteObject.value}'
-      ..classRef = classRef
-      ..kind = kind;
   }
 
   @override
@@ -765,8 +794,9 @@ require("dart_sdk").developer.invokeExtension(
           var inspectee = InstanceRef()
             ..kind = InstanceKind.kPlainInstance
             ..id = event.args[1].objectId
-            // TODO: A real classref? we need something here so it can properly
-            // serialize, but it isn't used by the widget inspector.
+            // TODO(jakemac): Create a real ClassRef, we need a way of looking
+            // up the library for a given instance to create it though.
+            // https://github.com/dart-lang/sdk/issues/36771.
             ..classRef = ClassRef();
           _streamNotify(
               'Debug',
@@ -870,3 +900,14 @@ const _pauseModePauseStates = {
   'all': PauseState.all,
   'unhandled': PauseState.uncaught,
 };
+
+/// Creates an [InstanceRef] for a primitive [RemoteObject].
+InstanceRef _primitiveInstance(String kind, RemoteObject remoteObject) {
+  var classRef = ClassRef()
+    ..id = 'dart:core:${remoteObject.type}'
+    ..name = kind;
+  return InstanceRef()
+    ..valueAsString = '${remoteObject.value}'
+    ..classRef = classRef
+    ..kind = kind;
+}

dwds/pubspec.yaml

@@ -1,5 +1,5 @@
 name: dwds
-version: 0.3.1
+version: 0.3.2
 author: Dart Team <misc@dartlang.org>
 homepage: https://github.com/dart-lang/webdev/tree/master/dwds
 description: >-

dwds/test/chrome_proxy_service_test.dart

@@ -150,6 +150,47 @@ void main() {
             const TypeMatcher<InstanceRef>().having(
                 (instance) => instance.valueAsString, 'valueAsString', '42.2'));
       });
+
+      test('can return objects with ids', () async {
+        expect(
+            await service.evaluate(
+                isolate.id, isolate.rootLib.id, 'createObject("cool")'),
+            const TypeMatcher<InstanceRef>()
+                .having((instance) => instance.id, 'id', isNotNull));
+        // TODO(jakemac): Add tests for the ClassRef once we create one,
+        // https://github.com/dart-lang/sdk/issues/36771.
+      });
+
+      group('with provided scope', () {
+        Future<InstanceRef> createRemoteObject(String message) async {
+          return await service.evaluate(
+                  isolate.id, isolate.rootLib.id, 'createObject("$message")')
+              as InstanceRef;
+        }
+
+        test('single scope object', () async {
+          var instance = await createRemoteObject('A');
+          var result = await service.evaluate(
+              isolate.id, isolate.rootLib.id, 'messageFor(arg1)',
+              scope: {'arg1': instance.id});
+          expect(
+              result,
+              const TypeMatcher<InstanceRef>().having(
+                  (instance) => instance.valueAsString, 'valueAsString', 'A'));
+        });
+
+        test('multiple scope objects', () async {
+          var instance1 = await createRemoteObject('A');
+          var instance2 = await createRemoteObject('B');
+          var result = await service.evaluate(
+              isolate.id, isolate.rootLib.id, 'messagesCombined(arg1, arg2)',
+              scope: {'arg1': instance1.id, 'arg2': instance2.id});
+          expect(
+              result,
+              const TypeMatcher<InstanceRef>().having(
+                  (instance) => instance.valueAsString, 'valueAsString', 'AB'));
+        });
+      });
     });
   });