Update HTTP Profiling ids to use Strings

The internal Dart Ids can be 64 bit integers, and are passed to Dev Tools as integers. This is causing errors fetching requests as the ids can overflow once they get to dev tools.

Interaction between `getHttpProfile` and `getHttpProfileRequest` are already performed in https://dart-review.googlesource.com/c/sdk/+/279122/3/pkg/vm_service/test/get_http_profile_test.dart

https://github.com/flutter/devtools/issues/2860

TEST=The original tests test the surface of the RPCs that are updated here. Any tests that needed tweaking have also been fixed
CoreLibraryReviewExempt: Changes are only to http profiling
Change-Id: Ie560dde7629f91f4221c1fe18d153cd999691086
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279122
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Dan Chevalier <danchevalier@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

CHANGELOG.md

@@ -107,6 +107,11 @@
 
 - Deprecated `NetworkInterface.listSupported`. Has always returned true since
   Dart 2.3.
+- Finalize `httpEnableTimelineLogging` parameter name transition from `enable`
+  to `enabled`. See [#43638][].
+- **Breaking change** [#51035][]:
+  - Update `NetworkProfiling` to accommodate new `String` ids
+    that are introduced in vm_service:11.0.0
 
 #### `dart:js_util`
 

pkg/vm_service/CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 11.0.0
+- Change `HttpProfileRequestRef.id` type from `int` to `String`.
+- Change `SocketStatistic.id` type from `int` to `String`.
+- Change `ext.dart.io.getHttpProfileRequest` `id` parameter type from `int` to `String`.
+- Change `ext.dart.io.socketProfilingEnabled` parameter from 'enable' to 'enabled'.
+
 ## 10.0.0
 - Update to version `4.0` of the spec.
 - Update for incorrectly documented types for `WeakReference`'s `target`,

pkg/vm_service/lib/src/dart_io_extensions.dart

@@ -77,9 +77,10 @@ extension DartIOExtension on VmService {
   /// Warning: The returned [Future] will not complete if the target isolate is paused
   /// and will only complete when the isolate is resumed.
   @Deprecated('Use httpEnableTimelineLogging instead.')
-  Future<Success> setHttpEnableTimelineLogging(String isolateId, bool enable) =>
+  Future<Success> setHttpEnableTimelineLogging(
+          String isolateId, bool enabled) =>
       _callHelper('ext.dart.io.setHttpEnableTimelineLogging', isolateId, args: {
-        'enable': enable,
+        'enabled': enabled,
       });
 
   /// The `httpEnableTimelineLogging` RPC is used to set and inspect the value of
@@ -119,7 +120,8 @@ extension DartIOExtension on VmService {
 
   /// The `getHttpProfileRequest` RPC is used to retrieve an instance of
   /// [HttpProfileRequest], which includes request and response body data.
-  Future<HttpProfileRequest> getHttpProfileRequest(String isolateId, int id) =>
+  Future<HttpProfileRequest> getHttpProfileRequest(
+          String isolateId, String id) =>
       _callHelper('ext.dart.io.getHttpProfileRequest', isolateId, args: {
         'id': id,
       });
@@ -205,7 +207,7 @@ class SocketStatistic {
       json == null ? null : SocketStatistic._fromJson(json);
 
   /// The unique ID associated with this socket.
-  final int id;
+  final String id;
 
   /// The time, in microseconds, that this socket was created.
   final int startTime;
@@ -364,7 +366,7 @@ class HttpProfileRequestRef {
   /// The ID associated with this request.
   ///
   /// This ID corresponds to the ID of the timeline event for this request.
-  final int id;
+  final String id;
 
   /// The HTTP request method associated with this request.
   final String method;
@@ -411,7 +413,7 @@ class HttpProfileRequest extends HttpProfileRequestRef {
         super._fromJson(json);
 
   HttpProfileRequest({
-    required int id,
+    required String id,
     required String isolateId,
     required String method,
     required Uri uri,

pkg/vm_service/pubspec.yaml

@@ -1,5 +1,5 @@
 name: vm_service
-version: 10.0.0
+version: 11.0.0
 description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.

runtime/observatory/tests/service/http_enable_timeline_logging_service_test.dart

@@ -32,7 +32,7 @@ var tests = <IsolateTest>[
     expect(response['enabled'], false);
 
     response = await isolate
-        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': true});
+        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': true});
     expect(response['type'], 'Success');
 
     response =
@@ -41,7 +41,7 @@ var tests = <IsolateTest>[
     expect(response['enabled'], true);
 
     response = await isolate
-        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': false});
+        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': false});
     expect(response['type'], 'Success');
 
     response =
@@ -52,8 +52,8 @@ var tests = <IsolateTest>[
   (Isolate isolate) async {
     // Bad argument.
     try {
-      await isolate
-          .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': 'foo'});
+      await isolate.invokeRpcNoUpgrade(
+          kSetHttpEnableTimelineLogging, {'enabled': 'foo'});
     } catch (e) {/* expected */}
     // Missing argument.
     try {

runtime/observatory/tests/service/network_profiling_test.dart

@@ -130,7 +130,7 @@ var tests = <IsolateTest>[
     stats.forEach((socket) {
       expect(socket['address'], contains(localhost));
       Expect.type<int>(socket['startTime']);
-      Expect.type<int>(socket['id']);
+      Expect.type<String>(socket['id']);
       Expect.type<int>(socket['port']);
       if (socket['socketType'] == 'tcp') {
         expect(socket['writeBytes'], content.length);

runtime/observatory_2/tests/service_2/http_enable_timeline_logging_service_test.dart

@@ -32,7 +32,7 @@ var tests = <IsolateTest>[
     expect(response['enabled'], false);
 
     response = await isolate
-        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': true});
+        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': true});
     expect(response['type'], 'Success');
 
     response =
@@ -41,7 +41,7 @@ var tests = <IsolateTest>[
     expect(response['enabled'], true);
 
     response = await isolate
-        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': false});
+        .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': false});
     expect(response['type'], 'Success');
 
     response =
@@ -52,8 +52,8 @@ var tests = <IsolateTest>[
   (Isolate isolate) async {
     // Bad argument.
     try {
-      await isolate
-          .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': 'foo'});
+      await isolate.invokeRpcNoUpgrade(
+          kSetHttpEnableTimelineLogging, {'enabled': 'foo'});
     } catch (e) {/* expected */}
     // Missing argument.
     try {

runtime/observatory_2/tests/service_2/network_profiling_test.dart

@@ -130,7 +130,7 @@ var tests = <IsolateTest>[
     stats.forEach((socket) {
       expect(socket['address'], contains(localhost));
       Expect.type<int>(socket['startTime']);
-      Expect.type<int>(socket['id']);
+      Expect.type<String>(socket['id']);
       Expect.type<int>(socket['port']);
       if (socket['socketType'] == 'tcp') {
         expect(socket['writeBytes'], content.length);

sdk/lib/_http/http_impl.dart

@@ -7,7 +7,7 @@ part of dart._http;
 abstract class HttpProfiler {
   static const _kType = 'HttpProfile';
 
-  static final Map<int, _HttpProfileData> _profile = {};
+  static final Map<String, _HttpProfileData> _profile = {};
 
   static _HttpProfileData startRequest(
     String method,
@@ -19,7 +19,7 @@ abstract class HttpProfiler {
     return data;
   }
 
-  static _HttpProfileData? getHttpProfileRequest(int id) => _profile[id];
+  static _HttpProfileData? getHttpProfileRequest(String id) => _profile[id];
 
   static void clear() => _profile.clear();
 
@@ -63,7 +63,7 @@ class _HttpProfileData {
         ) {
     // Grab the ID from the timeline event so HTTP profile IDs can be matched
     // to the timeline.
-    id = _timeline.pass();
+    id = _timeline.pass().toString();
     requestInProgress = true;
     requestStartTimestamp = Timeline.now;
     _timeline.start('HTTP CLIENT $method', arguments: {
@@ -261,7 +261,7 @@ class _HttpProfileData {
   bool requestInProgress = true;
   bool? responseInProgress;
 
-  late final int id;
+  late final String id;
   final String method;
   final Uri uri;
 

sdk/lib/io/network_profiling.dart

@@ -5,8 +5,8 @@
 part of dart.io;
 
 // TODO(bkonyi): refactor into io_resource_info.dart
-const int _versionMajor = 1;
-const int _versionMinor = 6;
+const int _versionMajor = 2;
+const int _versionMinor = 0;
 
 const String _tcpSocket = 'tcp';
 const String _udpSocket = 'udp';
@@ -68,15 +68,7 @@ abstract class _NetworkProfiling {
           responseJson = _setHttpEnableTimelineLogging(parameters);
           break;
         case _kHttpEnableTimelineLogging:
-          if (parameters.containsKey('enabled') ||
-              parameters.containsKey('enable')) {
-            // TODO(bkonyi): Backwards compatibility.
-            // See https://github.com/dart-lang/sdk/issues/43638.
-            assert(_versionMajor == 1,
-                "'enable' is deprecated and should be removed (See #43638)");
-            if (parameters.containsKey('enabled')) {
-              parameters['enable'] = parameters['enabled']!;
-            }
+          if (parameters.containsKey('enabled')) {
             _setHttpEnableTimelineLogging(parameters);
           }
           responseJson = _getHttpEnableTimelineLogging();
@@ -156,13 +148,13 @@ String _getHttpEnableTimelineLogging() => json.encode({
     });
 
 String _setHttpEnableTimelineLogging(Map<String, String> parameters) {
-  const String kEnable = 'enable';
-  if (!parameters.containsKey(kEnable)) {
-    throw _missingArgument(kEnable);
+  const String kEnabled = 'enabled';
+  if (!parameters.containsKey(kEnabled)) {
+    throw _missingArgument(kEnabled);
   }
-  final enable = parameters[kEnable]!.toLowerCase();
+  final enable = parameters[kEnabled]!.toLowerCase();
   if (enable != 'true' && enable != 'false') {
-    throw _invalidArgument(kEnable, enable);
+    throw _invalidArgument(kEnabled, enable);
   }
   HttpClient.enableTimelineLogging = enable == 'true';
   return _success();
@@ -172,10 +164,8 @@ String _getHttpProfileRequest(Map<String, String> parameters) {
   if (!parameters.containsKey('id')) {
     throw _missingArgument('id');
   }
-  final id = int.tryParse(parameters['id']!);
-  if (id == null) {
-    throw _invalidArgument('id', parameters['id']!);
-  }
+  String id = parameters['id']!;
+
   final request = HttpProfiler.getHttpProfileRequest(id);
   if (request == null) {
     throw "Unable to find request with id: '$id'";
@@ -215,7 +205,7 @@ abstract class _SocketProfile {
   static bool get enableSocketProfiling => _enableSocketProfiling;
 
   static bool _enableSocketProfiling = false;
-  static Map<int, _SocketStatistic> _idToSocketStatistic = {};
+  static Map<String, _SocketStatistic> _idToSocketStatistic = {};
 
   static String toJson() => json.encode({
         'type': _kType,
@@ -232,13 +222,15 @@ abstract class _SocketProfile {
 
   static void collectStatistic(int id, _SocketProfileType type,
       [dynamic object]) {
+    final idKey = id.toString();
     if (!_enableSocketProfiling) {
       return;
     }
     // Skip socket that started before _enableSocketProfiling turned on.
-    if (!_idToSocketStatistic.containsKey(id) &&
+    if (!_idToSocketStatistic.containsKey(idKey) &&
         type != _SocketProfileType.startTime) return;
-    _SocketStatistic stats = _idToSocketStatistic[id] ??= _SocketStatistic(id);
+    _SocketStatistic stats =
+        _idToSocketStatistic[idKey] ??= _SocketStatistic(idKey);
     switch (type) {
       case _SocketProfileType.startTime:
         stats.startTime = Timeline.now;
@@ -304,7 +296,7 @@ enum _SocketProfileType {
 
 /// Socket statistic
 class _SocketStatistic {
-  final int id;
+  final String id;
   int? startTime;
   int? endTime;
   String? address;