Revert "Call the HttpClient constructor when connecting a WebSocket to allow HttpOverrides to work."

This reverts commit 92e7989fbc.

Reason for revert: Causes VmService WebSocket creating to fail because HttpClient is mocked during Flutter Widget tests. See https://github.com/flutter/flutter/issues/138413

Original change's description:
> Call the HttpClient constructor when connecting a WebSocket to allow HttpOverrides to work.
>
> One consequence of this is that there will be one HttpClient per WebSocket connection rather than one for all WebSocket connections. This does *not* affect connection pooling because the sockets are detached from the HttpClient after the WebSocket connection is made.
>
> Closes https://github.com/dart-lang/sdk/pull/52509
>
> GitOrigin-RevId: 28ca642a56e3bbc6112df844b07026b9e0ae8ae6
> Change-Id: I8d9c6f2a883b0f79bafca30abd1dfd98291cf0e0
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305620
> Reviewed-by: Alexander Aprelev <aam@google.com>
> Commit-Queue: Brian Quinlan <bquinlan@google.com>

Change-Id: Ie1d029e7acf477503cd601f6a03e121cd17d1695
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336100
Auto-Submit: Brian Quinlan <bquinlan@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Quinlan <bquinlan@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>

sdk/lib/_http/websocket.dart

@@ -311,9 +311,6 @@ abstract class WebSocket
   ///
   /// If the `url` contains user information this will be passed as basic
   /// authentication when setting up the connection.
-  ///
-  /// If [customClient] is provided, then it will be used to make the
-  /// WebSocket connection and the [userAgent] field is ignored.
   static Future<WebSocket> connect(String url,
           {Iterable<String>? protocols,
           Map<String, dynamic>? headers,
@@ -399,15 +396,10 @@ abstract class WebSocket
   /// not, the receiving end will close the connection.
   void addUtf8Text(List<int> bytes);
 
-  /// The `User-Agent` header used for WebSocket connections.
-  ///
-  /// If the userAgent is set to `null`, no "User-Agent" header will be added
-  /// to requests.
-  ///
-  /// If [connect] is called with a `customClient`, then the WebSocket will
-  /// not use [userAgent].
+  /// Gets the user agent used for WebSocket connections.
   static String? get userAgent => _WebSocketImpl.userAgent;
 
+  /// Sets the user agent to use for WebSocket connections.
   static set userAgent(String? userAgent) {
     _WebSocketImpl.userAgent = userAgent;
   }

sdk/lib/_http/websocket_impl.dart

@@ -989,7 +989,7 @@ class _WebSocketImpl extends Stream with _ServiceObject implements WebSocket {
   Timer? _closeTimer;
   _WebSocketPerMessageDeflate? _deflate;
 
-  static String? _userAgent = _getHttpVersion();
+  static final HttpClient _httpClient = HttpClient();
 
   static Future<WebSocket> connect(
       String url, Iterable<String>? protocols, Map<String, dynamic>? headers,
@@ -1018,16 +1018,7 @@ class _WebSocketImpl extends Stream with _ServiceObject implements WebSocket {
         path: uri.path,
         query: uri.query,
         fragment: uri.fragment);
-
-    // The default userAgent is only overridden if no custom HttpClient is set.
-    late HttpClient client;
-    if (customClient != null) {
-      client = customClient;
-    } else {
-      client = HttpClient();
-      client.userAgent = _userAgent;
-    }
-    return client.openUrl("GET", uri).then((request) {
+    return (customClient ?? _httpClient).openUrl("GET", uri).then((request) {
       if (uri.userInfo != null && uri.userInfo.isNotEmpty) {
         // If the URL contains user information use that for basic
         // authorization.
@@ -1273,10 +1264,10 @@ class _WebSocketImpl extends Stream with _ServiceObject implements WebSocket {
     return _sink.close();
   }
 
-  static String? get userAgent => _userAgent;
+  static String? get userAgent => _httpClient.userAgent;
 
   static set userAgent(String? userAgent) {
-    _userAgent = userAgent;
+    _httpClient.userAgent = userAgent;
   }
 
   void _close([int? code, String? reason]) {

tests/standalone/io/http_override_websocket_test.dart

@@ -1,82 +0,0 @@
-// Copyright (c) 2023, 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:io';
-
-import "package:async_helper/async_helper.dart";
-import "package:expect/expect.dart";
-
-class TestQueryParamAddingHttpClient implements HttpClient {
-  Map<String, String> _queryParams;
-  final _client = HttpClient();
-
-  TestQueryParamAddingHttpClient(this._queryParams);
-
-  set userAgent(String? agent) => _client.userAgent = agent;
-  String? get userAgent => _client.userAgent;
-
-  Future<HttpClientRequest> openUrl(String method, Uri url) {
-    return _client.openUrl(method, url.replace(queryParameters: _queryParams));
-  }
-
-  void close({bool force = false}) => _client.close(force: force);
-
-  dynamic noSuchMethod(Invocation invocation) {
-    throw UnsupportedError(invocation.memberName.toString());
-  }
-}
-
-Future<void> testZoneHttpClientUsedInWebSocket() async {
-  final server = await HttpServer.bind("localhost", 0);
-  String? recordedUserAgent;
-  Map<String, String>? recordedQueryParameters;
-
-  server.forEach((request) {
-    recordedUserAgent = request.headers.value(HttpHeaders.userAgentHeader);
-    recordedQueryParameters = request.uri.queryParameters;
-    WebSocketTransformer.upgrade(request)
-        .then((webSocket) => webSocket.close());
-  });
-
-  WebSocket.userAgent = 'Agent Smith';
-
-  final client1 = TestQueryParamAddingHttpClient({"test": "1"});
-  client1.userAgent = "ZoneAgent1";
-  await HttpOverrides.runZoned(
-    () async {
-      final webSocket =
-          await WebSocket.connect("ws://localhost:${server.port}");
-      await webSocket.close();
-    },
-    createHttpClient: (c) => client1,
-  );
-  Expect.equals("Agent Smith", recordedUserAgent);
-  Expect.mapEquals({"test": "1"}, recordedQueryParameters!);
-
-  // The `HttpClient` used by `WebSocket` used to be static, so it is worth
-  // testing that the value used in the first execution is not reused.
-  final client2 = TestQueryParamAddingHttpClient({"test": "2"});
-  WebSocket.userAgent = 'Agent X';
-  client2.userAgent = "ZoneAgent2";
-  await HttpOverrides.runZoned(
-    () async {
-      final webSocket =
-          await WebSocket.connect("ws://localhost:${server.port}");
-      await webSocket.close();
-    },
-    createHttpClient: (c) => client2,
-  );
-  Expect.equals("Agent X", recordedUserAgent);
-  Expect.mapEquals({"test": "2"}, recordedQueryParameters!);
-
-  client1.close();
-  client2.close();
-  server.close();
-}
-
-main() async {
-  asyncStart();
-  await testZoneHttpClientUsedInWebSocket();
-  asyncEnd();
-}

tests/standalone/io/web_socket_test.dart

@@ -542,78 +542,42 @@ class SecurityConfiguration {
     });
   }
 
-  Future<void> testUserAgentSetToString() async {
+  void testShouldSetUserAgent() {
     asyncStart();
+    createServer().then((server) {
+      server.transform(new WebSocketTransformer()).listen((webSocket) {
+        Expect.equals('Custom User Agent', WebSocket.userAgent);
+        server.close();
+        webSocket.close();
+        asyncEnd();
+      });
 
-    final server = await HttpServer.bind("localhost", 0);
-    String? recordedUserAgent;
-    server.forEach((request) {
-      recordedUserAgent = request.headers.value(HttpHeaders.userAgentHeader);
-      WebSocketTransformer.upgrade(request)
-          .then((webSocket) => webSocket.close());
+      WebSocket.userAgent = 'Custom User Agent';
+      createClient(server.port).then((webSocket) {
+        webSocket.close();
+      });
     });
-
-    WebSocket.userAgent = "Agent Smith";
-    final webSocket = await WebSocket.connect("ws://localhost:${server.port}");
-
-    Expect.equals("Agent Smith", recordedUserAgent);
-
-    await webSocket.close();
-    await server.close();
-
-    asyncEnd();
   }
 
-  Future<void> testUserAgentSetToNull() async {
+  void testStaticClientUserAgentStaysTheSame() {
     asyncStart();
-
-    final server = await HttpServer.bind("localhost", 0);
-    String? recordedUserAgent;
-    server.forEach((request) {
-      recordedUserAgent = request.headers.value(HttpHeaders.userAgentHeader);
-      WebSocketTransformer.upgrade(request)
-          .then((webSocket) => webSocket.close());
+    createServer().then((server) {
+      server.transform(new WebSocketTransformer()).listen((webSocket) {
+        Expect.equals('Custom User Agent', WebSocket.userAgent);
+        server.close();
+        webSocket.close();
+        asyncEnd();
+      });
+      // Next line should take no effect on custom user agent value provided
+      WebSocket.userAgent = 'Custom User Agent';
+      createClient(server.port, customUserAgent: 'New User Agent')
+          .then((webSocket) {
+        webSocket.close();
+      });
     });
-
-    WebSocket.userAgent = null;
-    final webSocket = await WebSocket.connect("ws://localhost:${server.port}");
-
-    Expect.equals(null, recordedUserAgent);
-
-    await webSocket.close();
-    await server.close();
-
-    asyncEnd();
   }
 
-  Future<void> testUserAgentSetToStringAndCustomClient() async {
-    asyncStart();
-
-    final server = await HttpServer.bind("localhost", 0);
-    String? recordedUserAgent;
-    server.forEach((request) {
-      recordedUserAgent = request.headers.value(HttpHeaders.userAgentHeader);
-      WebSocketTransformer.upgrade(request)
-          .then((webSocket) => webSocket.close());
-    });
-
-    final client = HttpClient();
-    client.userAgent = "Agent Jones";
-    WebSocket.userAgent = "Agent X";
-    final webSocket = await WebSocket.connect("ws://localhost:${server.port}",
-        customClient: client);
-
-    Expect.equals("Agent Jones", recordedUserAgent);
-    Expect.equals("Agent Jones", client.userAgent);
-    Expect.equals("Agent X", WebSocket.userAgent);
-
-    await webSocket.close();
-    await server.close();
-
-    asyncEnd();
-  }
-
-  Future<void> runTests() async {
+  void runTests() {
     testRequestResponseClientCloses(2, null, null, 1);
     testRequestResponseClientCloses(2, 3001, null, 2);
     testRequestResponseClientCloses(2, 3002, "Got tired", 3);
@@ -640,15 +604,12 @@ class SecurityConfiguration {
     testFromUpgradedSocket();
     testAdditionalHeaders();
     testBasicAuthentication();
-    await testUserAgentSetToString();
-    await testUserAgentSetToNull();
-    await testUserAgentSetToStringAndCustomClient();
+    testShouldSetUserAgent();
+    testStaticClientUserAgentStaysTheSame();
   }
 }
 
-main() async {
-  asyncStart();
-  await new SecurityConfiguration(secure: false).runTests();
-  await new SecurityConfiguration(secure: true).runTests();
-  asyncEnd();
+main() {
+  new SecurityConfiguration(secure: false).runTests();
+  new SecurityConfiguration(secure: true).runTests();
 }