Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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([]);
Expand All @@ -384,7 +384,7 @@ class CurrentNetworkRequests extends ValueNotifier<List<NetworkRequest>> {
required int timelineMicrosOffset,
}) {
_updateOrAddRequests(requests);
_updateWebSocketRequests(sockets, timelineMicrosOffset);
_updateSocketProfiles(sockets, timelineMicrosOffset);
notifyListeners();
}

Expand Down Expand Up @@ -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;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
Expand Down Expand Up @@ -699,7 +699,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
_buildRow(
context: context,
title: 'Timing',
child: data is WebSocket
child: data is Socket
? _buildSocketTimeGraph(context)
: _buildHttpTimeGraph(),
),
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
);
Expand Down
6 changes: 3 additions & 3 deletions packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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.
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 8 additions & 8 deletions packages/devtools_app/test/network/network_profiler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
12 changes: 6 additions & 6 deletions packages/devtools_app/test/test_infra/test_data/network.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,42 +72,42 @@ final httpGetResponseBodyData = [
125,
];

final testSocket1 = WebSocket(SocketStatistic.parse(testSocket1Json)!, 0);
final testSocket1 = Socket(SocketStatistic.parse(testSocket1Json)!, 0);
final testSocket1Json = <String, Object?>{
'id': '10000',
'startTime': 1000000,
'endTime': 2000000,
'lastReadTime': 1800000,
'lastWriteTime': 1850000,
'address': 'InternetAddress(\'2606:4700:3037::ac43:bd8f\', IPv6)',
'address': '[2606:4700:3037::ac43:bd8f]',
'port': 443,
'socketType': 'tcp',
'readBytes': 10,
'writeBytes': 15,
};

final testSocket2 = WebSocket(SocketStatistic.parse(testSocket2Json)!, 0);
final testSocket2 = Socket(SocketStatistic.parse(testSocket2Json)!, 0);
final testSocket2Json = <String, Object?>{
'id': '11111',
'startTime': 3000000,
// This socket has no end time.
'lastReadTime': 3500000,
'lastWriteTime': 3600000,
'address': 'InternetAddress(\'2606:4700:3037::ac43:0000\', IPv6)',
'address': '[2606:4700:3037::ac43:0000]',
'port': 80,
'socketType': 'tcp',
'readBytes': 20,
'writeBytes': 25,
};

final testSocket3 = WebSocket(SocketStatistic.parse(testSocket3Json)!, 0);
final testSocket3 = Socket(SocketStatistic.parse(testSocket3Json)!, 0);
final testSocket3Json = <String, Object?>{
'id': '10000',
'startTime': 1000000,
'endTime': 2000000,
'lastReadTime': 1800000,
'lastWriteTime': 1850000,
'address': 'InternetAddress(\'2606:4700:3037::ac43:bd8f\', IPv6)',
'address': '[2606:4700:3037::ac43:bd8f]',
'port': 443,
'socketType': 'tcp',
'readBytes': 100,
Expand Down
Loading