vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

runtime/vm/service.cc

Index: runtime/vm/service.cc
diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
index f4241bb996cbbb7a1a0eabe689732815c8eeda42..91b52af8adc199d58a8660f077c100785071a8d9 100644
--- a/runtime/vm/service.cc
+++ b/runtime/vm/service.cc
@@ -15,12 +15,13 @@
 #include "vm/dart_entry.h"
 #include "vm/debugger.h"
 #include "vm/isolate.h"
+#include "vm/kernel_isolate.h"
 #include "vm/lockers.h"
 #include "vm/malloc_hooks.h"
 #include "vm/message.h"
 #include "vm/message_handler.h"
-#include "vm/native_entry.h"
 #include "vm/native_arguments.h"
+#include "vm/native_entry.h"
 #include "vm/native_symbol.h"
 #include "vm/object.h"
 #include "vm/object_graph.h"
@@ -40,7 +41,6 @@
 #include "vm/type_table.h"
 #include "vm/unicode.h"
 #include "vm/version.h"
-#include "vm/kernel_isolate.h"
 
 namespace dart {
 
@@ -2221,6 +2221,112 @@ static const MethodParameter* evaluate_params[] = {
 };
 
 
+static bool IsAlpha(char c) {
+  return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+}
+static bool IsAlphaNum(char c) {
+  return (c >= '0' && c <= '9') || IsAlpha(c);
+}
+static bool IsWhitespace(char c) {
+  return c <= ' ';

[siva, 2017/05/18 20:56:48]

should this also include tab?

[rmacnak, 2017/05/19 00:56:44]

This includes tab.

+}
+static bool IsObjectIdChar(char c) {
+  return IsAlphaNum(c) || c == '/' || c == '-' || c == '@' || c == '%';
+}
+
+
+// TODO(vm-service): Consider whether we should pass structured objects in
+// service messages instead of always flattening them to C strings.
+static bool ParseScope(const char* scope,
+                       const GrowableObjectArray& names,
+                       const GrowableObjectArray& values) {
+  const char* c = scope;
+  if (*c++ != '{') return false;
+
+  String& str = String::Handle();
+  for (;;) {
+    while (IsWhitespace(*c)) {
+      c++;
+    }
+
+    if (*c == '}') return true;
+
+    const char* name = c;
+    if (!IsAlpha(*c)) return false;
+    while (IsAlphaNum(*c)) {
+      c++;
+    }
+    str = String::FromUTF8(reinterpret_cast<const uint8_t*>(name), c - name);

[siva, 2017/05/18 20:56:48]

Is this really a UTF8 array or can you just use OneByteString::New(...) here or
maybe String::FromLatin1(...)

[rmacnak, 2017/05/19 00:56:44]

The input came from a String::ToCString(). But switched to keeping C strings as
suggested below.

+    names.Add(str);
+
+    while (IsWhitespace(*c)) {
+      c++;
+    }
+
+    if (*c++ != ':') return false;
+
+    while (IsWhitespace(*c)) {
+      c++;
+    }
+
+    const char* id = c;
+    if (!IsObjectIdChar(*c)) return false;
+    while (IsObjectIdChar(*c)) {
+      c++;
+    }
+    str = String::FromUTF8(reinterpret_cast<const uint8_t*>(id), c - id);

[siva, 2017/05/18 20:56:49]

Ditto comment here.

Also why do we need to create a GrowableObjectArray for the values here, convert
the C string to a Dart string and again convert it back to a C string when doing
the lookup? Why not just store a GrowableArray<const char*> here and create the
GrowableObjectArray in BuildScope after the lookup is done.

+    values.Add(str);
+
+    while (IsWhitespace(*c)) {
+      c++;
+    }
+    if (*c == ',') c++;
+  }
+
+  return false;
+}
+
+
+static bool BuildScope(Thread* thread,
+                       JSONStream* js,
+                       const GrowableObjectArray& names,
+                       const GrowableObjectArray& values) {
+  const char* scope = js->LookupParam("scope");
+  if (scope != NULL) {
+    if (!ParseScope(scope, names, values)) {
+      PrintInvalidParamError(js, "scope");
+      return true;
+    }
+    String& id = String::Handle();
+    Object& obj = Object::Handle();
+    for (intptr_t i = 0; i < values.Length(); i++) {
+      id ^= values.At(i);
+      ObjectIdRing::LookupResult lookup_result;
+      obj = LookupHeapObject(thread, id.ToCString(), &lookup_result);
+      if (obj.raw() == Object::sentinel().raw()) {
+        if (lookup_result == ObjectIdRing::kCollected) {
+          PrintSentinel(js, kCollectedSentinel);
+        } else if (lookup_result == ObjectIdRing::kExpired) {
+          PrintSentinel(js, kExpiredSentinel);
+        } else {
+          PrintInvalidParamError(js, "targetId");
+        }
+        return true;
+      }
+      if ((!obj.IsInstance() && !obj.IsNull()) || ContainsNonInstance(obj)) {
+        js->PrintError(kInvalidParams,
+                       "%s: invalid scope 'targetId' parameter: "
+                       "Cannot evaluate against a VM-internal object",
+                       js->method());
+        return true;
+      }
+      values.SetAt(i, obj);
+    }
+  }
+  return false;
+}
+
+
 static bool Evaluate(Thread* thread, JSONStream* js) {
   if (!thread->isolate()->compilation_allowed()) {
     js->PrintError(kFeatureDisabled,
@@ -2237,7 +2343,18 @@ static bool Evaluate(Thread* thread, JSONStream* js) {
     PrintMissingParamError(js, "expression");
     return true;
   }
+
   Zone* zone = thread->zone();
+  const GrowableObjectArray& names =
+      GrowableObjectArray::Handle(zone, GrowableObjectArray::New());
+  const GrowableObjectArray& values =
+      GrowableObjectArray::Handle(zone, GrowableObjectArray::New());
+  if (BuildScope(thread, js, names, values)) {
+    return true;
+  }
+  const Array& names_array = Array::Handle(zone, Array::MakeArray(names));
+  const Array& values_array = Array::Handle(zone, Array::MakeArray(values));
+
   const String& expr_str = String::Handle(zone, String::New(expr));
   ObjectIdRing::LookupResult lookup_result;
   Object& obj =
@@ -2254,17 +2371,15 @@ static bool Evaluate(Thread* thread, JSONStream* js) {
   }
   if (obj.IsLibrary()) {
     const Library& lib = Library::Cast(obj);
-    const Object& result = Object::Handle(
-        zone,
-        lib.Evaluate(expr_str, Array::empty_array(), Array::empty_array()));
+    const Object& result =
+        Object::Handle(zone, lib.Evaluate(expr_str, names_array, values_array));
     result.PrintJSON(js, true);
     return true;
   }
   if (obj.IsClass()) {
     const Class& cls = Class::Cast(obj);
-    const Object& result = Object::Handle(
-        zone,
-        cls.Evaluate(expr_str, Array::empty_array(), Array::empty_array()));
+    const Object& result =
+        Object::Handle(zone, cls.Evaluate(expr_str, names_array, values_array));
     result.PrintJSON(js, true);
     return true;
   }
@@ -2274,8 +2389,8 @@ static bool Evaluate(Thread* thread, JSONStream* js) {
     instance ^= obj.raw();
     const Class& receiver_cls = Class::Handle(zone, instance.clazz());
     const Object& result = Object::Handle(
-        zone, instance.Evaluate(receiver_cls, expr_str, Array::empty_array(),
-                                Array::empty_array()));
+        zone,
+        instance.Evaluate(receiver_cls, expr_str, names_array, values_array));
     result.PrintJSON(js, true);
     return true;
   }
@@ -2309,10 +2424,19 @@ static bool EvaluateInFrame(Thread* thread, JSONStream* js) {
   ActivationFrame* frame = stack->FrameAt(framePos);
 
   Zone* zone = thread->zone();
+  const GrowableObjectArray& names =
+      GrowableObjectArray::Handle(zone, GrowableObjectArray::New());
+  const GrowableObjectArray& values =
+      GrowableObjectArray::Handle(zone, GrowableObjectArray::New());
+  if (BuildScope(thread, js, names, values)) {
+    return true;
+  }
+
   const char* expr = js->LookupParam("expression");
   const String& expr_str = String::Handle(zone, String::New(expr));
 
-  const Object& result = Object::Handle(zone, frame->Evaluate(expr_str));
+  const Object& result =
+      Object::Handle(zone, frame->Evaluate(expr_str, names, values));
   result.PrintJSON(js, true);
   return true;
 }
@@ -2929,9 +3053,10 @@ static const char* const timeline_streams_enum_names[] = {
         NULL};
 
 static const MethodParameter* set_vm_timeline_flags_params[] = {
-    NO_ISOLATE_PARAMETER, new EnumListParameter("recordedStreams",
-                                                false,
-                                                timeline_streams_enum_names),
+    NO_ISOLATE_PARAMETER,
+    new EnumListParameter("recordedStreams",
+                          false,
+                          timeline_streams_enum_names),
     NULL,
 };
 
@@ -3609,8 +3734,9 @@ class PersistentHandleVisitor : public HandleVisitor {
     obj.AddProperty("type", "_WeakPersistentHandle");
     const Object& object = Object::Handle(weak_persistent_handle->raw());
     obj.AddProperty("object", object);
-    obj.AddPropertyF("peer", "0x%" Px "", reinterpret_cast<uintptr_t>(
-                                              weak_persistent_handle->peer()));
+    obj.AddPropertyF(
+        "peer", "0x%" Px "",
+        reinterpret_cast<uintptr_t>(weak_persistent_handle->peer()));
     obj.AddPropertyF(
         "callbackAddress", "0x%" Px "",
         reinterpret_cast<uintptr_t>(weak_persistent_handle->callback()));