Fix issue where socket profile data was being displayed as web socket connections (#8061)

* Fix issue where socket profile data was being displayed as web socket connections

Fixes #3033

Author: bkonyi

packages/devtools_app/lib/src/screens/network/network_controller.dart

@@ -146,7 +146,7 @@ class NetworkController extends DisposableController
       timelineMicrosOffset: timelineMicrosOffset,
     );
 
-    // If we have updated data for the selected web socket, we need to update
+    // If we have updated data for the selected socket, we need to update
     // the value.
     final currentSelectedRequestId = selectedRequest.value?.id;
     if (currentSelectedRequestId != null) {
@@ -362,7 +362,7 @@ class NetworkController extends DisposableController
   }
 }
 
-/// Class for managing the set of all current websocket requests, and
+/// Class for managing the set of all current sockets, and
 /// http profile requests.
 class CurrentNetworkRequests extends ValueNotifier<List<NetworkRequest>> {
   CurrentNetworkRequests() : super([]);
@@ -384,7 +384,7 @@ class CurrentNetworkRequests extends ValueNotifier<List<NetworkRequest>> {
     required int timelineMicrosOffset,
   }) {
     _updateOrAddRequests(requests);
-    _updateWebSocketRequests(sockets, timelineMicrosOffset);
+    _updateSocketProfiles(sockets, timelineMicrosOffset);
     notifyListeners();
   }
 
@@ -415,29 +415,29 @@ class CurrentNetworkRequests extends ValueNotifier<List<NetworkRequest>> {
     }
   }
 
-  void _updateWebSocketRequests(
+  void _updateSocketProfiles(
     List<SocketStatistic> sockets,
     int timelineMicrosOffset,
   ) {
-    for (final socket in sockets) {
-      final webSocket = WebSocket(socket, timelineMicrosOffset);
+    for (final socketStats in sockets) {
+      final socket = Socket(socketStats, timelineMicrosOffset);
 
-      if (_requestsById.containsKey(webSocket.id)) {
-        final existingRequest = _requestsById[webSocket.id];
-        if (existingRequest is WebSocket) {
-          existingRequest.update(webSocket);
+      if (_requestsById.containsKey(socket.id)) {
+        final existingRequest = _requestsById[socket.id];
+        if (existingRequest is Socket) {
+          existingRequest.update(socket);
         } else {
-          // If we override an entry that is not a Websocket then that means
+          // If we override an entry that is not a Socket then that means
           // the ids of the requestMapping entries may collide with other types
           // of requests.
-          assert(existingRequest is WebSocket);
+          assert(existingRequest is Socket);
         }
       } else {
-        value.add(webSocket);
-        // The new [sockets] may contain web sockets with the same ids as ones we
-        // already have, so we remove the current web sockets and replace them with
+        value.add(socket);
+        // The new [sockets] may contain sockets with the same ids as ones we
+        // already have, so we remove the current sockets and replace them with
         // updated data.
-        _requestsById[webSocket.id] = webSocket;
+        _requestsById[socket.id] = socket;
       }
     }
   }

packages/devtools_app/lib/src/screens/network/network_model.dart

@@ -82,8 +82,8 @@ abstract class NetworkRequest with ChangeNotifier, SearchableDataMixin {
       );
 }
 
-class WebSocket extends NetworkRequest {
-  WebSocket(this._socket, this._timelineMicrosBase);
+class Socket extends NetworkRequest {
+  Socket(this._socket, this._timelineMicrosBase);
 
   int _timelineMicrosBase;
 
@@ -93,7 +93,7 @@ class WebSocket extends NetworkRequest {
     return _timelineMicrosBase + micros;
   }
 
-  void update(WebSocket other) {
+  void update(Socket other) {
     _socket = other._socket;
     _timelineMicrosBase = other._timelineMicrosBase;
     notifyListeners();
@@ -145,15 +145,15 @@ class WebSocket extends NetworkRequest {
   }
 
   @override
-  String get contentType => 'websocket';
+  String get contentType => 'socket';
 
   @override
-  String get type => 'ws';
+  String get type => _socket.socketType;
 
   String get socketType => _socket.socketType;
 
   @override
-  String get uri => _socket.address;
+  String get uri => '${_socket.address}:$port';
 
   @override
   int get port => _socket.port;
@@ -166,21 +166,17 @@ class WebSocket extends NetworkRequest {
 
   int get writeBytes => _socket.writeBytes;
 
-  // TODO(kenz): is this always GET? Chrome DevTools shows GET in the response
-  // headers for web socket traffic.
   @override
-  String get method => 'GET';
+  String get method => 'SOCKET';
 
-  // TODO(kenz): is this always 101? Chrome DevTools lists "101" for WS status
-  // codes with a tooltip of "101 Web Socket Protocol Handshake"
   @override
-  String get status => '101';
+  String get status => _socket.endTime == null ? 'Open' : 'Closed';
 
   @override
   bool get inProgress => false;
 
   @override
-  bool operator ==(Object other) => other is WebSocket && id == other.id;
+  bool operator ==(Object other) => other is Socket && id == other.id;
 
   @override
   int get hashCode => id.hashCode;

packages/devtools_app/lib/src/screens/network/network_request_inspector_views.dart

@@ -639,7 +639,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
         padding: const EdgeInsets.all(defaultSpacing),
         children: [
           ..._buildGeneralRows(context),
-          if (data is WebSocket) ..._buildSocketOverviewRows(context),
+          if (data is Socket) ..._buildSocketOverviewRows(context),
           const PaddedDivider(
             padding: EdgeInsets.only(bottom: denseRowSpacing),
           ),
@@ -699,7 +699,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
       _buildRow(
         context: context,
         title: 'Timing',
-        child: data is WebSocket
+        child: data is Socket
             ? _buildSocketTimeGraph(context)
             : _buildHttpTimeGraph(),
       ),
@@ -710,7 +710,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
         child: _valueText(data.durationDisplay),
       ),
       const SizedBox(height: defaultSpacing),
-      ...data is WebSocket
+      ...data is Socket
           ? _buildSocketTimingRows(context)
           : _buildHttpTimingRows(context),
       const SizedBox(height: defaultSpacing),
@@ -832,7 +832,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
   }
 
   List<Widget> _buildSocketOverviewRows(BuildContext context) {
-    final socket = data as WebSocket;
+    final socket = data as Socket;
     return [
       _buildRow(
         context: context,
@@ -870,7 +870,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
   }
 
   List<Widget> _buildSocketTimingRows(BuildContext context) {
-    final data = this.data as WebSocket;
+    final data = this.data as Socket;
     final lastReadTimestamp = data.lastReadTimestamp;
     final lastWriteTimestamp = data.lastWriteTimestamp;
     return [

packages/devtools_app/lib/src/screens/network/network_screen.dart

@@ -305,7 +305,7 @@ class NetworkRequestsTable extends StatelessWidget {
   });
 
   static final methodColumn = MethodColumn();
-  static final addressColumn = UriColumn();
+  static final addressColumn = AddressColumn();
   static final statusColumn = StatusColumn();
   static final typeColumn = TypeColumn();
   static final durationColumn = DurationColumn();
@@ -353,11 +353,11 @@ class NetworkRequestsTable extends StatelessWidget {
   }
 }
 
-class UriColumn extends ColumnData<NetworkRequest>
+class AddressColumn extends ColumnData<NetworkRequest>
     implements ColumnRenderer<NetworkRequest> {
-  UriColumn()
+  AddressColumn()
       : super.wide(
-          'Uri',
+          'Address',
           minWidthPx: scaleByFontFactor(isEmbedded() ? 100 : 150.0),
           showTooltip: true,
         );

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -26,8 +26,7 @@ TODO: Remove this section if there are not any general updates.
 
 ## Memory updates
 
-* Enable offline analysis of snapshots, historical data analysis and save/load.
-- [#7843](https://github.com/flutter/devtools/pull/7843)
+* Enable offline analysis of snapshots, historical data analysis and save/load. - [#7843](https://github.com/flutter/devtools/pull/7843)
 
     ![Memory offline data](images/memory-save-load.png "Memory offline data")
 
@@ -37,8 +36,9 @@ TODO: Remove this section if there are not any general updates.
 
 ## Network profiler updates
 
-TODO: Remove this section if there are not any general updates.
+* Fixed issue where socket statistics were being reported as web sockets. - [#8061](https://github.com/flutter/devtools/pull/8061)
 
+    ![Network profiler correctly displaying socket statistics](images/socket-profiling.png "Network profiler correctly displaying socket statistics")
 ## Logging updates
 
 TODO: Remove this section if there are not any general updates.

packages/devtools_app/release_notes/images/socket-profiling.png

@@ -132,7 +132,10 @@ void main() {
       expect(profile.length, numRequests);
 
       expect(controller.matchesForSearch('jsonplaceholder').length, equals(5));
-      expect(controller.matchesForSearch('IPv6').length, equals(2));
+      expect(
+        controller.matchesForSearch('2606:4700:3037::ac43').length,
+        equals(2),
+      );
       expect(controller.matchesForSearch('').length, equals(0));
 
       // Search with incorrect case.
@@ -155,7 +158,7 @@ void main() {
       expect(matches.length, equals(5));
       verifyIsSearchMatch(profile, matches);
 
-      controller.search = 'IPv6';
+      controller.search = '2606:4700:3037::ac43';
       matches = controller.searchMatches.value;
       expect(matches.length, equals(2));
       verifyIsSearchMatch(profile, matches);
@@ -184,7 +187,11 @@ void main() {
 
       controller.setActiveFilter(query: 'method:get');
       expect(profile, hasLength(numRequests));
-      expect(controller.filteredData.value, hasLength(6));
+      expect(controller.filteredData.value, hasLength(4));
+
+      controller.setActiveFilter(query: 'method:socket');
+      expect(profile, hasLength(numRequests));
+      expect(controller.filteredData.value, hasLength(2));
 
       controller.setActiveFilter(query: 'm:put');
       expect(profile, hasLength(numRequests));
@@ -200,7 +207,7 @@ void main() {
 
       controller.setActiveFilter(query: 's:101');
       expect(profile, hasLength(numRequests));
-      expect(controller.filteredData.value, hasLength(3));
+      expect(controller.filteredData.value, hasLength(1));
 
       controller.setActiveFilter(query: '-s:Error');
       expect(profile, hasLength(numRequests));
@@ -210,11 +217,11 @@ void main() {
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(4));
 
-      controller.setActiveFilter(query: 't:ws');
+      controller.setActiveFilter(query: 't:tcp');
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(2));
 
-      controller.setActiveFilter(query: '-t:ws');
+      controller.setActiveFilter(query: '-t:tcp');
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(7));
 
@@ -234,17 +241,17 @@ void main() {
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(numRequests));
 
-      controller.setActiveFilter(query: '-t:ws,http');
+      controller.setActiveFilter(query: '-t:tcp,http');
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(4));
 
-      controller.setActiveFilter(query: '-t:ws,http method:put');
+      controller.setActiveFilter(query: '-t:tcp,http method:put');
       expect(profile, hasLength(numRequests));
       expect(controller.filteredData.value, hasLength(1));
 
       controller.setActiveFilter(query: '-status:error method:get');
       expect(profile, hasLength(numRequests));
-      expect(controller.filteredData.value, hasLength(5));
+      expect(controller.filteredData.value, hasLength(3));
 
       controller.setActiveFilter(query: '-status:error method:get t:http');
       expect(profile, hasLength(numRequests));

packages/devtools_app/test/network/network_controller_test.dart

@@ -460,17 +460,17 @@ void main() {
         // Verify general information.
         expect(find.text('Request uri: '), findsOneWidget);
         expect(
-          find.text('InternetAddress(\'2606:4700:3037::ac43:bd8f\', IPv6)'),
+          find.text('[2606:4700:3037::ac43:bd8f]:443'),
           findsOneWidget,
         );
         expect(find.text('Method: '), findsOneWidget);
-        expect(find.text('GET'), findsOneWidget);
+        expect(find.text('SOCKET'), findsOneWidget);
         expect(find.text('Status: '), findsOneWidget);
-        expect(find.text('101'), findsOneWidget);
+        expect(find.text('Closed'), findsOneWidget);
         expect(find.text('Port: '), findsOneWidget);
         expect(find.text('443'), findsOneWidget);
         expect(find.text('Content type: '), findsOneWidget);
-        expect(find.text('websocket'), findsOneWidget);
+        expect(find.text('socket'), findsOneWidget);
         expect(find.text('Socket id: '), findsOneWidget);
         expect(find.text('10000'), findsOneWidget);
         expect(find.text('Socket type: '), findsOneWidget);
@@ -513,17 +513,17 @@ void main() {
         // Verify general information.
         expect(find.text('Request uri: '), findsOneWidget);
         expect(
-          find.text('InternetAddress(\'2606:4700:3037::ac43:0000\', IPv6)'),
+          find.text('[2606:4700:3037::ac43:0000]:80'),
           findsOneWidget,
         );
         expect(find.text('Method: '), findsOneWidget);
-        expect(find.text('GET'), findsOneWidget);
+        expect(find.text('SOCKET'), findsOneWidget);
         expect(find.text('Status: '), findsOneWidget);
-        expect(find.text('101'), findsOneWidget);
+        expect(find.text('Open'), findsOneWidget);
         expect(find.text('Port: '), findsOneWidget);
         expect(find.text('80'), findsOneWidget);
         expect(find.text('Content type: '), findsOneWidget);
-        expect(find.text('websocket'), findsOneWidget);
+        expect(find.text('socket'), findsOneWidget);
         expect(find.text('Socket id: '), findsOneWidget);
         expect(find.text('11111'), findsOneWidget);
         expect(find.text('Socket type: '), findsOneWidget);

packages/devtools_app/test/network/network_profiler_test.dart

@@ -57,7 +57,7 @@ void main() {
     }
 
     test('UriColumn', () {
-      final column = UriColumn();
+      final column = AddressColumn();
       for (final request in requests) {
         expect(column.getDisplayValue(request), request.uri.toString());
       }