[ VM / Service ] Add libraryFilters to getSourceReport RPC

Bug: https://github.com/dart-lang/sdk/issues/48754
Change-Id: I42519d7dd751aa024860f940cbf51d2e38f7ea69
Tested: CI and new integration test
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241180
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>

pkg/vm_service/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 8.3.0
+- Update to version `3.57` of the spec.
+- Added optional `libraryFilters` parameter to `getSourceReport` RPC.
+
 ## 8.2.2+1
 - Documentation update for `FieldRef` and `FuncRef`.
 
@@ -11,7 +15,7 @@
 
 ## 8.2.1
 - Changed type of `UriList.uris` from `dynamic` to `List<String?>?`.
-- Remove `example/vm_service_asserts.dart'
+- Remove `example/vm_service_asserts.dart'`
 
 ## 8.2.0
 - Update to version `3.56` of the spec.

pkg/vm_service/java/version.properties

@@ -1 +1 @@
-version=3.56
+version=3.57

pkg/vm_service/lib/src/vm_service.dart

@@ -26,7 +26,7 @@ export 'snapshot_graph.dart'
         HeapSnapshotObjectNoData,
         HeapSnapshotObjectNullData;
 
-const String vmServiceVersion = '3.56.0';
+const String vmServiceVersion = '3.57.0';
 
 /// @optional
 const String optional = 'optional';
@@ -830,6 +830,12 @@ abstract class VmServiceInterface {
   /// numbers. If this parameter is not provided, it is considered to have the
   /// value `false`.
   ///
+  /// The `libraryFilters` parameter is intended to be used when gathering
+  /// coverage for the whole isolate. If it is provided, the `SourceReport` will
+  /// only contain results from scripts with URIs that start with one of the
+  /// filter strings. For example, pass `["package:foo/"]` to only include
+  /// scripts from the foo package.
+  ///
   /// If `isolateId` refers to an isolate which has exited, then the `Collected`
   /// [Sentinel] is returned.
   ///
@@ -845,6 +851,7 @@ abstract class VmServiceInterface {
     int? endTokenPos,
     bool? forceCompile,
     bool? reportLines,
+    List<String>? libraryFilters,
   });
 
   /// The `getVersion` RPC is used to determine what version of the Service
@@ -1510,6 +1517,7 @@ class VmServerConnection {
             endTokenPos: params['endTokenPos'],
             forceCompile: params['forceCompile'],
             reportLines: params['reportLines'],
+            libraryFilters: params['libraryFilters'],
           );
           break;
         case 'getVersion':
@@ -2036,6 +2044,7 @@ class VmService implements VmServiceInterface {
     int? endTokenPos,
     bool? forceCompile,
     bool? reportLines,
+    List<String>? libraryFilters,
   }) =>
       _call('getSourceReport', {
         'isolateId': isolateId,
@@ -2045,6 +2054,7 @@ class VmService implements VmServiceInterface {
         if (endTokenPos != null) 'endTokenPos': endTokenPos,
         if (forceCompile != null) 'forceCompile': forceCompile,
         if (reportLines != null) 'reportLines': reportLines,
+        if (libraryFilters != null) 'libraryFilters': libraryFilters,
       });
 
   @override

pkg/vm_service/pubspec.yaml

@@ -1,8 +1,9 @@
 name: vm_service
-version: 8.2.2+1
+version: 8.3.0
 description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.
+
 repository: https://github.com/dart-lang/sdk/tree/main/pkg/vm_service
 
 environment:

pkg/vm_service/test/source_report_package_filters_test.dart

@@ -0,0 +1,88 @@
+// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:developer';
+import 'dart:math';
+import 'package:test/test.dart';
+import 'package:test_package/has_part.dart';
+import 'package:vm_service/vm_service.dart';
+import 'common/service_test_common.dart';
+import 'common/test_helper.dart';
+
+void testFunction() {
+  // Use functions from various packages, so we can get coverage for them.
+  print(sqrt(123)); // dart:math
+  print(anything); // package:test/test.dart
+  print(decodeBase64("SGkh")); // package:vm_service/vm_service.dart
+  print(removeAdjacentDuplicates([])); // common/service_test_common.dart
+  foo(); // package:test_package/has_part.dart
+
+  debugger();
+}
+
+IsolateTest filterTestImpl(List<String> filters, Function(Set<String>) check) {
+  return (VmService service, IsolateRef isolateRef) async {
+    final isolateId = isolateRef.id!;
+
+    final report = await service.getSourceReport(
+      isolateId,
+      [SourceReportKind.kCoverage],
+      forceCompile: true,
+      libraryFilters: filters,
+    );
+    check(Set.of(report.scripts!.map((s) => s.uri!)));
+  };
+}
+
+IsolateTest filterTestExactlyMatches(
+        List<String> filters, List<String> expectedScripts) =>
+    filterTestImpl(filters, (Set<String> scripts) {
+      expect(scripts, unorderedEquals(expectedScripts));
+    });
+
+IsolateTest filterTestContains(
+        List<String> filters, List<String> expectedScripts) =>
+    filterTestImpl(filters, (Set<String> scripts) {
+      expect(scripts, containsAll(expectedScripts));
+    });
+
+var tests = <IsolateTest>[
+  hasStoppedAtBreakpoint,
+  filterTestExactlyMatches(
+    ['package:test_pack'],
+    [
+      'package:test_package/has_part.dart',
+      'package:test_package/the_part.dart',
+      'package:test_package/the_part_2.dart',
+    ],
+  ),
+  filterTestExactlyMatches(
+    ['package:test_package/'],
+    [
+      'package:test_package/has_part.dart',
+      'package:test_package/the_part.dart',
+      'package:test_package/the_part_2.dart',
+    ],
+  ),
+  filterTestExactlyMatches(
+    ['zzzzzzzzzzz'],
+    [],
+  ),
+  filterTestContains(
+    ['dart:math'],
+    ['dart:math'],
+  ),
+  filterTestContains(
+    ['package:test/', 'package:vm'],
+    ['package:test/test.dart', 'package:vm_service/vm_service.dart'],
+  ),
+  resumeIsolate,
+];
+
+main([args = const <String>[]]) => runIsolateTests(
+      args,
+      tests,
+      'source_report_package_filters_test.dart',
+      testeeConcurrent: testFunction,
+    );

runtime/observatory/tests/service/get_version_rpc_test.dart

@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], 'Version');
     expect(result['major'], 3);
-    expect(result['minor'], 56);
+    expect(result['minor'], 57);
     expect(result['_privateMajor'], 0);
     expect(result['_privateMinor'], 0);
   },

runtime/observatory_2/tests/service_2/get_version_rpc_test.dart

@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], equals('Version'));
     expect(result['major'], equals(3));
-    expect(result['minor'], equals(56));
+    expect(result['minor'], equals(57));
     expect(result['_privateMajor'], equals(0));
     expect(result['_privateMinor'], equals(0));
   },

runtime/vm/service.cc

@@ -3568,6 +3568,35 @@ static void GetInstancesAsArray(Thread* thread, JSONStream* js) {
   instances.PrintJSON(js, /* as_ref */ true);
 }
 
+static intptr_t ParseJSONArray(Thread* thread,
+                               const char* str,
+                               const GrowableObjectArray& elements) {
+  ASSERT(str != nullptr);
+  ASSERT(thread != nullptr);
+  Zone* zone = thread->zone();
+  intptr_t n = strlen(str);
+  if (n < 2) {
+    return -1;
+  }
+  intptr_t start = 1;
+  while (start < n) {
+    intptr_t end = start;
+    while ((str[end + 1] != ',') && (str[end + 1] != ']')) {
+      end++;
+    }
+    if (end == start) {
+      // Empty element
+      break;
+    }
+    String& element = String::Handle(
+        zone, String::FromUTF8(reinterpret_cast<const uint8_t*>(&str[start]),
+                               end - start + 1));
+    elements.Add(element);
+    start = end + 3;
+  }
+  return 0;
+}
+
 static const MethodParameter* const get_ports_params[] = {
     RUNNABLE_ISOLATE_PARAMETER,
     NULL,
@@ -3684,7 +3713,20 @@ static void GetSourceReport(Thread* thread, JSONStream* js) {
       return;
     }
   }
-  SourceReport report(report_set, compile_mode, report_lines);
+
+  const char* library_filters_param = js->LookupParam("libraryFilters");
+  GrowableObjectArray& library_filters = GrowableObjectArray::Handle();
+  if (library_filters_param != nullptr) {
+    library_filters = GrowableObjectArray::New();
+    intptr_t library_filters_length =
+        ParseJSONArray(thread, library_filters_param, library_filters);
+    if (library_filters_length < 0) {
+      PrintInvalidParamError(js, "library_filters");
+      return;
+    }
+  }
+
+  SourceReport report(report_set, library_filters, compile_mode, report_lines);
   report.PrintJSON(js, script, TokenPosition::Deserialize(start_pos),
                    TokenPosition::Deserialize(end_pos));
 #endif  // !DART_PRECOMPILED_RUNTIME
@@ -5185,35 +5227,6 @@ static void GetVM(Thread* thread, JSONStream* js) {
   Service::PrintJSONForVM(js, false);
 }
 
-static intptr_t ParseJSONArray(Thread* thread,
-                               const char* str,
-                               const GrowableObjectArray& elements) {
-  ASSERT(str != nullptr);
-  ASSERT(thread != nullptr);
-  Zone* zone = thread->zone();
-  intptr_t n = strlen(str);
-  if (n < 2) {
-    return -1;
-  }
-  intptr_t start = 1;
-  while (start < n) {
-    intptr_t end = start;
-    while ((str[end + 1] != ',') && (str[end + 1] != ']')) {
-      end++;
-    }
-    if (end == start) {
-      // Empty element
-      break;
-    }
-    String& element = String::Handle(
-        zone, String::FromUTF8(reinterpret_cast<const uint8_t*>(&str[start]),
-                               end - start + 1));
-    elements.Add(element);
-    start = end + 3;
-  }
-  return 0;
-}
-
 class UriMappingTraits {
  public:
   static const char* Name() { return "UriMappingTraits"; }

runtime/vm/service.h

@@ -17,7 +17,7 @@
 namespace dart {
 
 #define SERVICE_PROTOCOL_MAJOR_VERSION 3
-#define SERVICE_PROTOCOL_MINOR_VERSION 56
+#define SERVICE_PROTOCOL_MINOR_VERSION 57
 
 class Array;
 class EmbedderServiceHandler;
@@ -71,12 +71,12 @@ class RingServiceIdZone : public ServiceIdZone {
 class StreamInfo {
  public:
   explicit StreamInfo(const char* id)
-      : id_(id), enabled_(false), include_private_members_(false) {}
+      : id_(id), enabled_(0), include_private_members_(false) {}
 
   const char* id() const { return id_; }
 
   void set_enabled(bool value) { enabled_ = value ? 1 : 0; }
-  bool enabled() const { return !!enabled_; }
+  bool enabled() const { return enabled_ != 0; }
 
   void set_include_private_members(bool value) {
     include_private_members_ = value;

runtime/vm/service/service.md

@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 3.56
+# Dart VM Service Protocol 3.57
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 3.56_ of the Dart VM Service Protocol. This
+This document describes of _version 3.57_ 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.
@@ -1065,7 +1065,8 @@ SourceReport|Sentinel getSourceReport(string isolateId,
                                       int tokenPos [optional],
                                       int endTokenPos [optional],
                                       bool forceCompile [optional],
-                                      bool reportLines [optional])
+                                      bool reportLines [optional],
+                                      string[] libraryFilters [optional])
 ```
 
 The _getSourceReport_ RPC is used to generate a set of reports tied to
@@ -1107,6 +1108,11 @@ numbers. This is designed to reduce the number of RPCs that need to be performed
 in the case that the client is only interested in line numbers. If this
 parameter is not provided, it is considered to have the value _false_.
 
+The _libraryFilters_ parameter is intended to be used when gathering coverage
+for the whole isolate. If it is provided, the _SourceReport_ will only contain
+results from scripts with URIs that start with one of the filter strings. For
+example, pass `["package:foo/"]` to only include scripts from the foo package.
+
 If _isolateId_ refers to an isolate which has exited, then the
 _Collected_ [Sentinel](#sentinel) is returned.
 
@@ -4362,5 +4368,6 @@ version | comments
 3.54 | Added `CpuSamplesEvent`, updated `cpuSamples` property on `Event` to have type `CpuSamplesEvent`.
 3.55 | Added `streamCpuSamplesWithUserTag` RPC.
 3.56 | Added optional `line` and `column` properties to `SourceLocation`. Added a new `SourceReportKind`, `BranchCoverage`, which reports branch level coverage information.
+3.57 | Added optional `libraryFilters` parameter to `getSourceReport` RPC.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss

runtime/vm/source_report.cc

@@ -29,6 +29,21 @@ SourceReport::SourceReport(intptr_t report_set,
     : report_set_(report_set),
       compile_mode_(compile_mode),
       report_lines_(report_lines),
+      library_filters_(GrowableObjectArray::Handle()),
+      thread_(NULL),
+      script_(NULL),
+      start_pos_(TokenPosition::kMinSource),
+      end_pos_(TokenPosition::kMaxSource),
+      next_script_index_(0) {}
+
+SourceReport::SourceReport(intptr_t report_set,
+                           const GrowableObjectArray& library_filters,
+                           CompileMode compile_mode,
+                           bool report_lines)
+    : report_set_(report_set),
+      compile_mode_(compile_mode),
+      report_lines_(report_lines),
+      library_filters_(library_filters),
       thread_(NULL),
       script_(NULL),
       start_pos_(TokenPosition::kMinSource),
@@ -169,7 +184,8 @@ bool SourceReport::ShouldSkipField(const Field& field) {
   return false;
 }
 
-intptr_t SourceReport::GetScriptIndex(const Script& script) {
+intptr_t SourceReport::GetScriptIndex(const Script& script,
+                                      bool bypass_filters) {
   ScriptTableEntry wrapper;
   const String& url = String::Handle(zone(), script.url());
   wrapper.key = &url;
@@ -178,6 +194,9 @@ intptr_t SourceReport::GetScriptIndex(const Script& script) {
   if (pair != NULL) {
     return pair->index;
   }
+  if (!library_filters_.IsNull() && !bypass_filters) {
+    return -1;
+  }
   ScriptTableEntry* tmp = new ScriptTableEntry();
   tmp->key = &url;
   tmp->index = next_script_index_++;
@@ -493,6 +512,11 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
   const TokenPosition begin_pos = func.token_pos();
   const TokenPosition end_pos = func.end_token_pos();
 
+  const intptr_t script_index = GetScriptIndex(script);
+  if (script_index < 0) {
+    return;
+  }
+
   Code& code = Code::Handle(zone(), func.unoptimized_code());
   if (code.IsNull()) {
     if (func.HasCode() || (compile_mode_ == kForceCompile)) {
@@ -501,7 +525,7 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
       if (!err.IsNull()) {
         // Emit an uncompiled range for this function with error information.
         JSONObject range(jsarr);
-        range.AddProperty("scriptIndex", GetScriptIndex(script));
+        range.AddProperty("scriptIndex", script_index);
         range.AddProperty("startPos", begin_pos);
         range.AddProperty("endPos", end_pos);
         range.AddProperty("compiled", false);
@@ -512,7 +536,7 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
     } else {
       // This function has not been compiled yet.
       JSONObject range(jsarr);
-      range.AddProperty("scriptIndex", GetScriptIndex(script));
+      range.AddProperty("scriptIndex", script_index);
       range.AddProperty("startPos", begin_pos);
       range.AddProperty("endPos", end_pos);
       range.AddProperty("compiled", false);
@@ -527,7 +551,7 @@ void SourceReport::VisitFunction(JSONArray* jsarr, const Function& func) {
   if (!func.IsAsyncFunction() && !func.IsAsyncGenerator() &&
       !func.IsSyncGenerator()) {
     JSONObject range(jsarr);
-    range.AddProperty("scriptIndex", GetScriptIndex(script));
+    range.AddProperty("scriptIndex", script_index);
     range.AddProperty("startPos", begin_pos);
     range.AddProperty("endPos", end_pos);
     range.AddProperty("compiled", true);
@@ -577,7 +601,11 @@ void SourceReport::VisitLibrary(JSONArray* jsarr, const Library& lib) {
           // Emit an uncompiled range for this class with error information.
           JSONObject range(jsarr);
           script = cls.script();
-          range.AddProperty("scriptIndex", GetScriptIndex(script));
+          const intptr_t script_index = GetScriptIndex(script);
+          if (script_index < 0) {
+            continue;
+          }
+          range.AddProperty("scriptIndex", script_index);
           range.AddProperty("startPos", cls.token_pos());
           range.AddProperty("endPos", cls.end_token_pos());
           range.AddProperty("compiled", false);
@@ -590,7 +618,11 @@ void SourceReport::VisitLibrary(JSONArray* jsarr, const Library& lib) {
         // Emit one range for the whole uncompiled class.
         JSONObject range(jsarr);
         script = cls.script();
-        range.AddProperty("scriptIndex", GetScriptIndex(script));
+        const intptr_t script_index = GetScriptIndex(script);
+        if (script_index < 0) {
+          continue;
+        }
+        range.AddProperty("scriptIndex", script_index);
         range.AddProperty("startPos", cls.token_pos());
         range.AddProperty("endPos", cls.end_token_pos());
         range.AddProperty("compiled", false);
@@ -626,6 +658,19 @@ void SourceReport::VisitClosures(JSONArray* jsarr) {
   });
 }
 
+bool SourceReport::LibraryMatchesFilters(const Library& lib) {
+  const String& url = String::Handle(zone(), lib.url());
+  String& filter = String::Handle(zone());
+  const intptr_t num_filters = library_filters_.Length();
+  for (intptr_t i = 0; i < num_filters; ++i) {
+    filter ^= library_filters_.At(i);
+    if (url.StartsWith(filter)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void SourceReport::PrintJSON(JSONStream* js,
                              const Script& script,
                              TokenPosition start_pos,
@@ -640,9 +685,25 @@ void SourceReport::PrintJSON(JSONStream* js,
     const GrowableObjectArray& libs = GrowableObjectArray::Handle(
         zone(), thread()->isolate_group()->object_store()->libraries());
 
-    // We only visit the libraries which actually load the specified script.
     Library& lib = Library::Handle(zone());
-    for (int i = 0; i < libs.Length(); i++) {
+    if (!library_filters_.IsNull()) {
+      // If we have library filters, pre-fill GetScriptIndex with all the
+      // scripts from the libraries that pass the filters. Later calls to
+      // GetScriptIndex will ignore any scripts that are missing.
+      for (intptr_t i = 0; i < libs.Length(); i++) {
+        lib ^= libs.At(i);
+        if (LibraryMatchesFilters(lib)) {
+          Script& script = Script::Handle(zone());
+          const Array& scripts = Array::Handle(zone(), lib.LoadedScripts());
+          for (intptr_t j = 0; j < scripts.Length(); j++) {
+            script ^= scripts.At(j);
+            GetScriptIndex(script, true /* bypass_filters */);
+          }
+        }
+      }
+    }
+    // We only visit the libraries which actually load the specified script.
+    for (intptr_t i = 0; i < libs.Length(); i++) {
       lib ^= libs.At(i);
       if (script.IsNull() || ScriptIsLoadedByLibrary(script, lib)) {
         VisitLibrary(&ranges, lib);
@@ -737,11 +798,15 @@ void SourceReport::CollectConstConstructorCoverageFromScripts(
           continue;
         }
         scriptRef ^= constructor.script();
+        const intptr_t script_index = GetScriptIndex(scriptRef);
+        if (script_index < 0) {
+          continue;
+        }
         code ^= constructor.unoptimized_code();
         const TokenPosition begin_pos = constructor.token_pos();
         const TokenPosition end_pos = constructor.end_token_pos();
         JSONObject range(ranges);
-        range.AddProperty("scriptIndex", GetScriptIndex(scriptRef));
+        range.AddProperty("scriptIndex", script_index);
         range.AddProperty("compiled",
                           !code.IsNull());  // Does this make a difference?
         range.AddProperty("startPos", begin_pos);

runtime/vm/source_report.h

@@ -40,9 +40,16 @@ class SourceReport {
 
   // report_set is a bitvector indicating which reports to generate
   // (e.g. kCallSites | kCoverage).
+  //
+  // If library_filters is not null, then the report will only include libraries
+  // whose URIs start with one of the filter strings.
   explicit SourceReport(intptr_t report_set,
                         CompileMode compile = kNoCompile,
                         bool report_lines = false);
+  explicit SourceReport(intptr_t report_set,
+                        const GrowableObjectArray& library_filters,
+                        CompileMode compile = kNoCompile,
+                        bool report_lines = false);
   ~SourceReport();
 
   // Generate a source report for (some subrange of) a script.
@@ -69,10 +76,11 @@ class SourceReport {
   bool ShouldSkipFunction(const Function& func);
   bool ShouldSkipField(const Field& field);
   bool ShouldCoverageSkipCallSite(const ICData* ic_data);
-  intptr_t GetScriptIndex(const Script& script);
+  intptr_t GetScriptIndex(const Script& script, bool bypass_filters = false);
   bool ScriptIsLoadedByLibrary(const Script& script, const Library& lib);
   intptr_t GetTokenPosOrLine(const Script& script,
                              const TokenPosition& token_pos);
+  bool LibraryMatchesFilters(const Library& lib);
 
   void PrintCallSitesData(JSONObject* jsobj,
                           const Function& func,
@@ -135,6 +143,7 @@ class SourceReport {
   intptr_t report_set_;
   CompileMode compile_mode_;
   bool report_lines_;
+  const GrowableObjectArray& library_filters_;
   Thread* thread_;
   const Script* script_;
   TokenPosition start_pos_;