Reland "Update HTTP Profiling ids to use Strings"

This is a reland of commit e7d8261f9a




Original change's description:
> 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. https://github.com/flutter/devtools/pull/5127 is being submitted to Devtools and G3, and the regressions have been tested against vm_service:10.0.0 and vm_service:11.0.0
> 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>

TEST=The original tests test the surface of the RPCs that are updated here. Any tests that needed tweaking have also been fixed. https://github.com/flutter/devtools/pull/5127 is being submitted to Devtools and G3, and the regressions have been tested against vm_service:10.0.0 and vm_service:11.0.0
CoreLibraryReviewExempt: Changes are only to http profiling
Change-Id: I132f30111ca9c4c53dec377f6038453cacfc6243
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280020
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Dan Chevalier <danchevalier@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,10 @@
 # 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.1.2
 - Fix bug where code would try to call `.toJson()` on `String`s.
 

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.1.2
+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;