Add Service ID zones to service protocol

runtime/vm/service.cc

Index: runtime/vm/service.cc
diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
index f17949f6a51ea77d23933e04b62fc7a82e94c6f7..082c777869be54ecbe2b9c4a0552067d8cb75a7a 100644
--- a/runtime/vm/service.cc
+++ b/runtime/vm/service.cc
@@ -39,6 +39,76 @@ namespace dart {
 DECLARE_FLAG(bool, trace_service);
 DECLARE_FLAG(bool, trace_service_pause_events);
 
+ServiceIdZone::ServiceIdZone() {
+}
+
+
+ServiceIdZone::~ServiceIdZone() {
+}
+
+
+RingServiceIdZone::RingServiceIdZone(ObjectIdRing::IdPolicy policy)
+    : policy_(policy) {
+}
+
+
+RingServiceIdZone::~RingServiceIdZone() {
+}
+
+
+char* RingServiceIdZone::GetServiceId(const Object& obj) {
+  Thread* thread = Thread::Current();
+  Isolate* isolate = thread->isolate();
+  ASSERT(isolate != NULL);
+  Zone* zone = thread->zone();
+  ASSERT(zone != NULL);
+  ObjectIdRing* ring = isolate->object_id_ring();
+  const intptr_t id = ring->GetIdForObject(obj.raw(), policy_);
+  return zone->PrintToString("objects/%" Pd "", id);
+}
+
+GrowableServiceIdZone::GrowableServiceIdZone(const char* format_str,

[turnidge, 2015/05/11 19:48:45]

format_str is perhaps too general, given that we will eventually need to
parse/dispatch these ids.  Directory format, as discussed offline?

[Cutch, 2015/05/12 14:59:57]

Acknowledged.

+                                             const GrowableObjectArray& storage)
+    : format_str_(format_str),
+      array_(GrowableObjectArray::Handle(storage.raw())) {
+}
+
+
+GrowableServiceIdZone::~GrowableServiceIdZone() {
+}
+
+
+char* GrowableServiceIdZone::GetServiceId(const Object& obj) {
+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+  ASSERT(zone != NULL);
+  intptr_t id = FindIndex(obj);
+  if (id < 0) {
+    id = Append(obj);
+  }
+  ASSERT(id >= 0);
+  return zone->PrintToString(format_str_, id);
+}
+
+
+intptr_t GrowableServiceIdZone::FindIndex(const Object& obj) {
+  for (intptr_t i = 0; i < array_.Length(); i++) {
+    if (obj.raw() == array_.At(i)) {
+      return i;
+    }
+  }
+  return -1;
+}
+
+
+intptr_t GrowableServiceIdZone::Append(const Object& obj) {
+  intptr_t id = array_.Length();
+  array_.Add(obj);
+  ASSERT(id == (array_.Length() - 1));
+  return id;
+}
+
+
 // TODO(johnmccutchan): Unify embedder service handler lists and their APIs.
 EmbedderServiceHandler* Service::isolate_service_handler_head_ = NULL;
 EmbedderServiceHandler* Service::root_service_handler_head_ = NULL;
@@ -446,10 +516,36 @@ void Service::InvokeMethod(Isolate* isolate, const Array& msg) {
       FATAL("SendPort expected.");
     }
 
+    // Default service id zone is the standard object id ring which eagerly
+    // allocates a new id each time. Keep existing_ring_service_id_zone on
+    // the stack in case it is requested.
+    RingServiceIdZone
+        existing_ring_service_id_zone(ObjectIdRing::kExistingOrNewId);

[turnidge, 2015/05/11 19:48:45]

Having this on the stack feels wrong, somehow.

[Cutch, 2015/05/12 14:59:57]

Done.

+
     JSONStream js;
     js.Setup(zone.GetZone(), SendPort::Cast(reply_port).Id(),
              seq, method_name, param_keys, param_values);
 
+    // RPC came in with a custom service id zone.
+    const char* service_id_zone_param = js.LookupParam("_serviceIdZone");
+
+    if (service_id_zone_param != NULL) {
+      // Override service id.
+      if (strcmp("Ring.NewId", service_id_zone_param) == 0) {

[turnidge, 2015/05/11 19:48:45]

Name this ring "default"?

[Cutch, 2015/05/12 14:59:57]

Done.

+        // Ring with eager id allocation. This is the default ring.
+        // Nothing to do.
+      } else if (strcmp("Ring.ExistingId", service_id_zone_param) == 0) {

[turnidge, 2015/05/11 19:48:45]

Name this ring "default.reuse"?  Not sure.

[Cutch, 2015/05/12 14:59:57]

Done.

+        // Ring with existing id reuse.
+        js.set_service_id_zone(&existing_ring_service_id_zone);
+      } else {
+        // TODO(johnmccutchan): Support creating, deleting, and selecting
+        // custom service id zones (see GrowableServiceIdZone).
+        // For now, always return an error.
+        PrintInvalidParamError(&js, "_serviceIdZone");

[turnidge, 2015/05/11 20:01:04]

_idZone instead?  service is implied.  Here are elsewhere.

[Cutch, 2015/05/12 14:59:57]

Done.

+        js.PostReply();
+        return;
+      }
+    }
     const char* c_method_name = method_name.ToCString();
 
     ServiceMethodDescriptor* method = FindMethod(c_method_name);
@@ -719,7 +815,7 @@ static bool GetIsolate(Isolate* isolate, JSONStream* js) {
 
 static const MethodParameter* get_stack_params[] = {
   ISOLATE_PARAMETER,
-  new BoolParameter("full", false),
+  new BoolParameter("_full", false),

[turnidge, 2015/05/11 19:48:45]

Update the idl if necessary.

[Cutch, 2015/05/12 14:59:57]

Done.

   NULL,
 };
 
@@ -728,7 +824,7 @@ static bool GetStack(Isolate* isolate, JSONStream* js) {
   DebuggerStackTrace* stack = isolate->debugger()->StackTrace();
   // Do we want the complete script object and complete local variable objects?
   // This is true for dump requests.
-  const bool full = BoolParameter::Parse(js->LookupParam("full"), false);
+  const bool full = BoolParameter::Parse(js->LookupParam("_full"), false);
   JSONObject jsobj(js);
   jsobj.AddProperty("type", "Stack");
   {
@@ -791,6 +887,17 @@ static bool _TriggerEchoEvent(Isolate* isolate, JSONStream* js) {
 }
 
 
+static bool _DumpRingRequests(Isolate* isolate, JSONStream* js) {

[turnidge, 2015/05/11 19:48:45]

Is using a leading underscore a thing in our C++?  We should probably drop these
underscores.

[Cutch, 2015/05/12 14:59:57]

Done.

+  // Reuse object ids found in the ring.
+  RingServiceIdZone
+      existing_ring_service_id_zone(ObjectIdRing::kExistingOrNewId);
+  js->set_service_id_zone(&existing_ring_service_id_zone);
+  ObjectIdRing* ring = isolate->object_id_ring();
+  ring->PrintGetObjectRequestsToJSON(js);
+  return true;
+}
+
+
 static bool _Echo(Isolate* isolate, JSONStream* js) {
   JSONObject jsobj(js);
   return HandleCommonEcho(&jsobj, js);
@@ -2547,6 +2654,7 @@ static bool SetName(Isolate* isolate, JSONStream* js) {
 
 
 static ServiceMethodDescriptor service_methods_[] = {
+  { "_dumpRingRequests", _DumpRingRequests, NULL },

[turnidge, 2015/05/11 19:48:45]

I would instead like this request to be "_dumpZone" or "_dumpZoneContents".  It
should dump the contents of whatever zone is provided as an argument.  That way
people can eventually dump their custom zones with the same rpc.

[Cutch, 2015/05/12 14:59:57]

Done.

   { "_echo", _Echo,
     NULL },
   { "_respondWithMalformedJson", _RespondWithMalformedJson,