[ VM / Service ] Add getInstances and InstanceSet to the service protocol.

Change-Id: Ia5aab0f1bbdb99a7690a107eeef08de6a1f33656
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106561
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/observatory/lib/src/elements/strongly_reachable_instances.dart b/runtime/observatory/lib/src/elements/strongly_reachable_instances.dart
index 73c0f9e..a772497 100644
--- a/runtime/observatory/lib/src/elements/strongly_reachable_instances.dart
+++ b/runtime/observatory/lib/src/elements/strongly_reachable_instances.dart
@@ -92,7 +92,7 @@
     if (_result == null) {
       return [new SpanElement()..text = 'Loading...'];
     }
-    final content = _result.samples
+    final content = _result.instances
         .map<Element>((sample) => new DivElement()
           ..children = <Element>[
             anyRef(_isolate, sample, _objects, queue: _r.queue)
@@ -106,7 +106,7 @@
   }
 
   List<Element> _createShowMoreButton() {
-    final samples = _result.samples.toList();
+    final samples = _result.instances.toList();
     if (samples.length == _result.count) {
       return [];
     }
diff --git a/runtime/observatory/lib/src/models/objects/class.dart b/runtime/observatory/lib/src/models/objects/class.dart
index 464956c..9d1a7d1 100644
--- a/runtime/observatory/lib/src/models/objects/class.dart
+++ b/runtime/observatory/lib/src/models/objects/class.dart
@@ -69,7 +69,7 @@
 
 abstract class InstanceSet {
   int get count;
-  Iterable<ObjectRef> get samples;
+  Iterable<ObjectRef> get instances;
 }
 
 abstract class TopRetainedInstances {}
diff --git a/runtime/observatory/lib/src/service/object.dart b/runtime/observatory/lib/src/service/object.dart
index 13da3e9..124cbc2 100644
--- a/runtime/observatory/lib/src/service/object.dart
+++ b/runtime/observatory/lib/src/service/object.dart
@@ -1904,7 +1904,7 @@
       'classId': cls.id,
       'limit': limit.toString(),
     };
-    return invokeRpc('_getInstances', params);
+    return invokeRpc('getInstances', params);
   }
 
   Future<ServiceObject /*HeapObject*/ > getObjectByAddress(String address,
@@ -4049,7 +4049,7 @@
 class InstanceSet extends HeapObject implements M.InstanceSet {
   HeapObject dartOwner;
   int count;
-  Iterable<HeapObject> samples;
+  Iterable<HeapObject> instances;
 
   InstanceSet._empty(ServiceObjectOwner owner) : super._empty(owner);
 
@@ -4061,7 +4061,7 @@
       return;
     }
     count = map['totalCount'];
-    samples = new List<HeapObject>.from(map['samples']);
+    instances = new List<HeapObject>.from(map['instances']);
   }
 }
 
diff --git a/runtime/observatory/tests/service/get_instances_rpc_test.dart b/runtime/observatory/tests/service/get_instances_rpc_test.dart
index 7f1bc22..814405c 100644
--- a/runtime/observatory/tests/service/get_instances_rpc_test.dart
+++ b/runtime/observatory/tests/service/get_instances_rpc_test.dart
@@ -31,25 +31,38 @@
   (Isolate isolate) async {
     var obj = await eval(isolate, 'global');
     var params = {
-      'classId': obj['class']['id'],
+      'objectId': obj['class']['id'],
       'limit': 4,
     };
-    var result = await isolate.invokeRpcNoUpgrade('_getInstances', params);
+    var result = await isolate.invokeRpcNoUpgrade('getInstances', params);
     expect(result['type'], equals('InstanceSet'));
     expect(result['totalCount'], equals(2));
-    expect(result['samples'].length, equals(2));
-    expect(result['samples'][0]['type'], equals('@Instance'));
+    expect(result['instances'].length, equals(2));
+    expect(result['instances'][0]['type'], equals('@Instance'));
 
     // Limit is respected.
     params = {
-      'classId': obj['class']['id'],
+      'objectId': obj['class']['id'],
       'limit': 1,
     };
-    result = await isolate.invokeRpcNoUpgrade('_getInstances', params);
+    result = await isolate.invokeRpcNoUpgrade('getInstances', params);
     expect(result['type'], equals('InstanceSet'));
     expect(result['totalCount'], equals(2));
-    expect(result['samples'].length, equals(1));
-    expect(result['samples'][0]['type'], equals('@Instance'));
+    expect(result['instances'].length, equals(1));
+    expect(result['instances'][0]['type'], equals('@Instance'));
+
+    // Try an object ID that isn't a class ID
+    params = {
+      'objectId': isolate.rootLibrary.id,
+      'limit': 1,
+    };
+    try {
+      await isolate.invokeRpcNoUpgrade('getInstances', params);
+    } on ServerRpcException catch (_) {
+      // Success.
+    } catch (e) {
+      fail('Failed with exception: $e');
+    }
   },
 ];
 
diff --git a/runtime/observatory/tests/service/get_version_rpc_test.dart b/runtime/observatory/tests/service/get_version_rpc_test.dart
index 287d311..a5f9a4f 100644
--- a/runtime/observatory/tests/service/get_version_rpc_test.dart
+++ b/runtime/observatory/tests/service/get_version_rpc_test.dart
@@ -12,7 +12,7 @@
     var result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], equals('Version'));
     expect(result['major'], equals(3));
-    expect(result['minor'], equals(19));
+    expect(result['minor'], equals(20));
     expect(result['_privateMajor'], equals(0));
     expect(result['_privateMinor'], equals(0));
   },
diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc
index 80ba87f..a5f12e0 100644
--- a/runtime/vm/service.cc
+++ b/runtime/vm/service.cc
@@ -2989,8 +2989,10 @@
 
 class GetInstancesVisitor : public ObjectGraph::Visitor {
  public:
-  GetInstancesVisitor(const Class& cls, const Array& storage)
-      : cls_(cls), storage_(storage), count_(0) {}
+  GetInstancesVisitor(const Class& cls,
+                      ZoneGrowableHandlePtrArray<Object>* storage,
+                      intptr_t limit)
+      : cls_(cls), storage_(storage), limit_(limit), count_(0) {}
 
   virtual Direction VisitObject(ObjectGraph::StackIterator* it) {
     RawObject* raw_obj = it->Get();
@@ -3002,8 +3004,8 @@
     Object& obj = thread->ObjectHandle();
     obj = raw_obj;
     if (obj.GetClassId() == cls_.id()) {
-      if (!storage_.IsNull() && count_ < storage_.Length()) {
-        storage_.SetAt(count_, obj);
+      if (count_ < limit_) {
+        storage_->Add(Object::Handle(raw_obj));
       }
       ++count_;
     }
@@ -3014,7 +3016,8 @@
 
  private:
   const Class& cls_;
-  const Array& storage_;
+  ZoneGrowableHandlePtrArray<Object>* storage_;
+  const intptr_t limit_;
   intptr_t count_;
 };
 
@@ -3023,9 +3026,9 @@
 };
 
 static bool GetInstances(Thread* thread, JSONStream* js) {
-  const char* target_id = js->LookupParam("classId");
-  if (target_id == NULL) {
-    PrintMissingParamError(js, "classId");
+  const char* object_id = js->LookupParam("objectId");
+  if (object_id == NULL) {
+    PrintMissingParamError(js, "objectId");
     return true;
   }
   const char* limit_cstr = js->LookupParam("limit");
@@ -3038,14 +3041,20 @@
     PrintInvalidParamError(js, "limit");
     return true;
   }
-  const Object& obj = Object::Handle(LookupHeapObject(thread, target_id, NULL));
+
+  const Object& obj = Object::Handle(LookupHeapObject(thread, object_id, NULL));
   if (obj.raw() == Object::sentinel().raw() || !obj.IsClass()) {
-    PrintInvalidParamError(js, "classId");
+    PrintInvalidParamError(js, "objectId");
     return true;
   }
   const Class& cls = Class::Cast(obj);
-  Array& storage = Array::Handle(Array::New(limit));
-  GetInstancesVisitor visitor(cls, storage);
+
+  // Ensure the array and handles created below are promptly destroyed.
+  StackZone zone(thread);
+  HANDLESCOPE(thread);
+
+  ZoneGrowableHandlePtrArray<Object> storage(thread->zone(), limit);
+  GetInstancesVisitor visitor(cls, &storage, limit);
   ObjectGraph graph(thread);
   HeapIterationScope iteration_scope(Thread::Current(), true);
   graph.IterateObjects(&visitor);
@@ -3054,20 +3063,11 @@
   jsobj.AddProperty("type", "InstanceSet");
   jsobj.AddProperty("totalCount", count);
   {
-    JSONArray samples(&jsobj, "samples");
-    for (int i = 0; (i < storage.Length()) && (i < count); i++) {
-      const Object& sample = Object::Handle(storage.At(i));
-      samples.AddValue(sample);
+    JSONArray samples(&jsobj, "instances");
+    for (int i = 0; (i < limit) && (i < count); i++) {
+      samples.AddValue(storage.At(i));
     }
   }
-
-  // We nil out the array after generating the response to prevent
-  // reporting spurious references when looking for inbound references
-  // after looking at allInstances.
-  for (intptr_t i = 0; i < storage.Length(); i++) {
-    storage.SetAt(i, Object::null_object());
-  }
-
   return true;
 }
 
@@ -4926,7 +4926,7 @@
     get_heap_map_params },
   { "_getInboundReferences", GetInboundReferences,
     get_inbound_references_params },
-  { "_getInstances", GetInstances,
+  { "getInstances", GetInstances,
     get_instances_params },
   { "getIsolate", GetIsolate,
     get_isolate_params },
diff --git a/runtime/vm/service.h b/runtime/vm/service.h
index 6e99782..15d30ff 100644
--- a/runtime/vm/service.h
+++ b/runtime/vm/service.h
@@ -15,7 +15,7 @@
 namespace dart {
 
 #define SERVICE_PROTOCOL_MAJOR_VERSION 3
-#define SERVICE_PROTOCOL_MINOR_VERSION 19
+#define SERVICE_PROTOCOL_MINOR_VERSION 20
 
 class Array;
 class EmbedderServiceHandler;
diff --git a/runtime/vm/service/service.md b/runtime/vm/service/service.md
index d01f0ef..2c5731c 100644
--- a/runtime/vm/service/service.md
+++ b/runtime/vm/service/service.md
@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 3.19
+# Dart VM Service Protocol 3.20
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 3.19_ of the Dart VM Service Protocol. This
+This document describes of _version 3.20_ of the Dart VM Service Protocol. This
 protocol is used to communicate with a running Dart Virtual Machine.
 
 To use the Service Protocol, start the VM with the *--observe* flag.
@@ -32,6 +32,7 @@
   - [evaluateInFrame](#evaluateinframe)
   - [getAllocationProfile](#getallocationprofile)
   - [getFlagList](#getflaglist)
+  - [getInstances](#getinstances)
   - [getIsolate](#getisolate)
   - [getMemoryUsage](#getmemoryusage)
   - [getScripts](#getscripts)
@@ -79,6 +80,7 @@
   - [Frame](#frame)
   - [Function](#function)
   - [Instance](#instance)
+  - [InstanceSet](#instanceset)
   - [Isolate](#isolate)
   - [Library](#library)
   - [LibraryDependency](#librarydependency)
@@ -638,6 +640,21 @@
 
 See [FlagList](#flaglist).
 
+### getInstances
+
+```
+InstanceSet getInstances(string objectId,
+                         int limit)
+```
+
+The _getInstances_ RPC is used to retrieve a set of instances which are of a specific type.
+
+_objectId_ is the ID of the `Class` to retrieve instances for. _objectId_ must be the ID of a `Class`, otherwise an error is returned.
+
+_limit_ is the maximum number of instances to be returned.
+
+See [InstanceSet](#instanceset).
+
 ### getIsolate
 
 ```
@@ -2293,6 +2310,20 @@
 
 An _Isolate_ object provides information about one isolate in the VM.
 
+### InstanceSet
+
+```
+class InstanceSet extends Response {
+  // The number of instances of the requested type currently allocated.
+  int totalCount;
+
+  // An array of instances of the requested type.
+  @Instance[] instances;
+}
+```
+
+See [getInstances](#getinstances).
+
 ### Library
 
 ```
@@ -3002,7 +3033,7 @@
 
 version | comments
 ------- | --------
-1.0 | initial revision
+1.0 | Initial revision
 2.0 | Describe protocol version 2.0.
 3.0 | Describe protocol version 3.0.  Added UnresolvedSourceLocation.  Added Sentinel return to getIsolate.  Add AddedBreakpointWithScriptUri.  Removed Isolate.entry. The type of VM.pid was changed from string to int.  Added VMUpdate events.  Add offset and count parameters to getObject() and offset and count fields to Instance. Added ServiceExtensionAdded event.
 3.1 | Add the getSourceReport RPC.  The getObject RPC now accepts offset and count for string objects.  String objects now contain length, offset, and count properties.
@@ -3024,5 +3055,6 @@
 3.17 | Add 'Logging' event kind and the LogRecord class.
 3.18 | Add 'getAllocationProfile' RPC and 'AllocationProfile' and 'ClassHeapStats' objects.
 3.19 | Add 'clearVMTimeline', 'getVMTimeline', 'getVMTimelineFlags', 'setVMTimelineFlags', 'Timeline', and 'TimelineFlags'.
+3.20 | Add 'getInstances' RPC and 'InstanceSet' object.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss
diff --git a/runtime/vm/service/service_dev.md b/runtime/vm/service/service_dev.md
index bfc5fa9..14293eb 100644
--- a/runtime/vm/service/service_dev.md
+++ b/runtime/vm/service/service_dev.md
@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 3.20-dev
+# Dart VM Service Protocol 3.21-dev
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 3.20-dev_ of the Dart VM Service Protocol. This
+This document describes of _version 3.21-dev_ of the Dart VM Service Protocol. This
 protocol is used to communicate with a running Dart Virtual Machine.
 
 To use the Service Protocol, start the VM with the *--observe* flag.
@@ -32,6 +32,7 @@
   - [evaluateInFrame](#evaluateinframe)
   - [getAllocationProfile](#getallocationprofile)
   - [getFlagList](#getflaglist)
+  - [getInstances](#getinstances)
   - [getIsolate](#getisolate)
   - [getMemoryUsage](#getmemoryusage)
   - [getScripts](#getscripts)
@@ -79,6 +80,7 @@
   - [Frame](#frame)
   - [Function](#function)
   - [Instance](#instance)
+  - [InstanceSet](#instanceset)
   - [Isolate](#isolate)
   - [Library](#library)
   - [LibraryDependency](#librarydependency)
@@ -638,6 +640,21 @@
 
 See [FlagList](#flaglist).
 
+### getInstances
+
+```
+InstanceSet getInstances(string objectId,
+                         int limit)
+```
+
+The _getInstances_ RPC is used to retrieve a set of instances which are of a specific type.
+
+_objectId_ is the ID of the `Class` to retrieve instances for. _objectId_ must be the ID of a `Class`, otherwise an error is returned.
+
+_limit_ is the maximum number of instances to be returned.
+
+See [InstanceSet](#instanceset).
+
 ### getIsolate
 
 ```
@@ -2293,6 +2310,20 @@
 
 An _Isolate_ object provides information about one isolate in the VM.
 
+### InstanceSet
+
+```
+class InstanceSet extends Response {
+  // The number of instances of the requested type currently allocated.
+  int totalCount;
+
+  // An array of instances of the requested type.
+  @Instance[] instances;
+}
+```
+
+See [getInstances](#getinstances).
+
 ### Library
 
 ```
@@ -3002,7 +3033,7 @@
 
 version | comments
 ------- | --------
-1.0 | initial revision
+1.0 | Initial revision
 2.0 | Describe protocol version 2.0.
 3.0 | Describe protocol version 3.0.  Added UnresolvedSourceLocation.  Added Sentinel return to getIsolate.  Add AddedBreakpointWithScriptUri.  Removed Isolate.entry. The type of VM.pid was changed from string to int.  Added VMUpdate events.  Add offset and count parameters to getObject() and offset and count fields to Instance. Added ServiceExtensionAdded event.
 3.1 | Add the getSourceReport RPC.  The getObject RPC now accepts offset and count for string objects.  String objects now contain length, offset, and count properties.
@@ -3024,5 +3055,6 @@
 3.17 | Add 'Logging' event kind and the LogRecord class.
 3.18 | Add 'getAllocationProfile' RPC and 'AllocationProfile' and 'ClassHeapStats' objects.
 3.19 | Add 'clearVMTimeline', 'getVMTimeline', 'getVMTimelineFlags', 'setVMTimelineFlags', 'Timeline', and 'TimelineFlags'.
+3.20 | Add 'getInstances' RPC and 'InstanceSet' object.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss