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.

Sign up for GitHub

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

Migrate networking screen to null safety #3880

Merged
merged 8 commits into from
Mar 17, 2022
Merged

Migrate networking screen to null safety #3880

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c800aa4
Migrate networking screen to null safety
bkonyi Mar 16, 2022
95793d7
Update vm_service to 8.2.2
bkonyi Mar 17, 2022
b748867
Fix error
bkonyi Mar 17, 2022
d1dcc98
Fix tests
bkonyi Mar 17, 2022
25f3469
Address comments
bkonyi Mar 17, 2022
9b0df30
Remove redundant asserts
bkonyi Mar 17, 2022
b45ac21
Address comments
bkonyi Mar 17, 2022
4002e34
Address comment.
bkonyi Mar 17, 2022
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
17 changes: 11 additions & 6 deletions packages/devtools_app/lib/src/http/http_request_data.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ class DartIOHttpRequestData extends HttpRequestData {
bool get _hasError => _request.request?.hasError ?? false;

int? get _endTime =>
_hasError ? _request.endTime : _request.response!.endTime;
_hasError ? _request.endTime : _request.response?.endTime;

@override
Duration? get duration {
Expand All @@ -539,8 +539,8 @@ class DartIOHttpRequestData extends HttpRequestData {
'method': _request.method,
'uri': _request.uri.toString(),
if (!didFail) ...{
'connectionInfo': _request.request!.connectionInfo,
'contentLength': _request.request!.contentLength,
'connectionInfo': _request.request?.connectionInfo,
'contentLength': _request.request?.contentLength,
},
if (_request.response != null) ...{
'compressionState': _request.response!.compressionState,
Expand Down Expand Up @@ -605,9 +605,14 @@ class DartIOHttpRequestData extends HttpRequestData {
}

@override
DateTime get endTimestamp => DateTime.fromMicrosecondsSinceEpoch(
timelineMicrosecondsSinceEpoch(_endTime!),
);
DateTime? get endTimestamp {
final endTime = _endTime;
return endTime == null
? null
: DateTime.fromMicrosecondsSinceEpoch(
timelineMicrosecondsSinceEpoch(endTime),
);
}

@override
DateTime get startTimestamp => DateTime.fromMicrosecondsSinceEpoch(
Expand Down
103 changes: 52 additions & 51 deletions packages/devtools_app/lib/src/screens/network/network_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart=2.9

import 'dart:async';

import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -45,46 +43,46 @@ class NetworkController

final _requests = ValueNotifier<NetworkRequests>(NetworkRequests());

ValueListenable<NetworkRequest> get selectedRequest => _selectedRequest;
ValueListenable<NetworkRequest?> get selectedRequest => _selectedRequest;

final _selectedRequest = ValueNotifier<NetworkRequest>(null);
final _selectedRequest = ValueNotifier<NetworkRequest?>(null);

/// Notifies that the timeline is currently being recorded.
ValueListenable<bool> get recordingNotifier => _recordingNotifier;
final _recordingNotifier = ValueNotifier<bool>(false);

@visibleForTesting
NetworkService get networkService => _networkService;
NetworkService _networkService;
late NetworkService _networkService;

/// The timeline timestamps are relative to when the VM started.
///
/// This value is equal to
/// `DateTime.now().microsecondsSinceEpoch - _profileStartMicros` when
/// recording is started is used to calculate the correct wall-time for
/// timeline events.
int _timelineMicrosOffset;
late int _timelineMicrosOffset;

/// The last timestamp at which HTTP and Socket information was refreshed.
int lastRefreshMicros = 0;

Timer _pollingTimer;
Timer? _pollingTimer;

@visibleForTesting
bool get isPolling => _pollingTimer != null;

void selectRequest(NetworkRequest selection) {
void selectRequest(NetworkRequest? selection) {
_selectedRequest.value = selection;
}

void _processTimeline({
@required Timeline timeline,
@required int timelineMicrosOffset,
@required List<NetworkRequest> currentValues,
@required List<HttpRequestData> invalidRequests,
@required Map<String, HttpRequestData> outstandingRequestsMap,
required Timeline timeline,
required int timelineMicrosOffset,
required List<NetworkRequest> currentValues,
required List<HttpRequestData> invalidRequests,
required Map<String, HttpRequestData> outstandingRequestsMap,
}) {
final events = timeline.traceEvents;
final events = timeline.traceEvents!;
// Group all HTTP timeline events with the same ID.
final httpEvents = <String, List<Map<String, Object>>>{};
final httpRequestIdToResponseId = <String, String>{};
Expand All @@ -109,7 +107,7 @@ class NetworkController
if (parentId != null) {
httpRequestIdToResponseId[parentId] = id;
}
httpEvents.putIfAbsent(id, () => []).add(json);
httpEvents.putIfAbsent(id, () => []).add(json.cast<String, Object>());
}

// Build our list of network requests from the collected events.
Expand All @@ -134,7 +132,7 @@ class NetworkController
// If there's a new event which matches a request that was previously in
// flight, update the associated HttpRequestData.
if (outstandingRequestsMap.containsKey(requestId)) {
final outstandingRequest = outstandingRequestsMap[requestId];
final outstandingRequest = outstandingRequestsMap[requestId]!;
outstandingRequest.merge(requestData);
if (!outstandingRequest.inProgress) {
outstandingRequestsMap.remove(requestId);
Expand All @@ -153,10 +151,10 @@ class NetworkController
}

void _processHttpProfileRequests({
@required int timelineMicrosOffset,
@required List<HttpProfileRequest> httpRequests,
@required List<NetworkRequest> currentValues,
@required Map<String, HttpRequestData> outstandingRequestsMap,
required int timelineMicrosOffset,
required List<HttpProfileRequest> httpRequests,
required List<NetworkRequest> currentValues,
required Map<String, HttpRequestData> outstandingRequestsMap,
}) {
for (final request in httpRequests) {
final wrapped = DartIOHttpRequestData(
Expand All @@ -165,8 +163,9 @@ class NetworkController
);
final id = request.id.toString();
if (outstandingRequestsMap.containsKey(id)) {
outstandingRequestsMap[id].merge(wrapped);
if (!outstandingRequestsMap[id].inProgress) {
final request = outstandingRequestsMap[id]!;
request.merge(wrapped);
if (!request.inProgress) {
final data =
outstandingRequestsMap.remove(id) as DartIOHttpRequestData;
data.getFullRequestData().then((value) => _updateData());
Expand All @@ -184,13 +183,13 @@ class NetworkController

@visibleForTesting
NetworkRequests processNetworkTrafficHelper(
Timeline timeline,
Timeline? timeline,
List<SocketStatistic> sockets,
List<HttpProfileRequest> httpRequests,
List<HttpProfileRequest>? httpRequests,
int timelineMicrosOffset, {
@required List<NetworkRequest> currentValues,
@required List<HttpRequestData> invalidRequests,
@required Map<String, HttpRequestData> outstandingRequestsMap,
required List<NetworkRequest> currentValues,
required List<HttpRequestData> invalidRequests,
required Map<String, HttpRequestData> outstandingRequestsMap,
}) {
// [currentValues] contains all the current requests we have in the
// profiler, which will contain web socket requests if they exist. The new
Expand Down Expand Up @@ -219,7 +218,7 @@ class NetworkController
} else {
_processHttpProfileRequests(
timelineMicrosOffset: timelineMicrosOffset,
httpRequests: httpRequests,
httpRequests: httpRequests!,
currentValues: currentValues,
outstandingRequestsMap: outstandingRequestsMap,
);
Expand All @@ -233,9 +232,9 @@ class NetworkController
}

void processNetworkTraffic({
@required Timeline timeline,
@required List<SocketStatistic> sockets,
@required List<HttpProfileRequest> httpRequests,
required Timeline? timeline,
required List<SocketStatistic> sockets,
required List<HttpProfileRequest>? httpRequests,
}) {
// Trigger refresh.
_requests.value = processNetworkTrafficHelper(
Expand Down Expand Up @@ -293,7 +292,7 @@ class NetworkController
// fewer flags risks breaking functionality on the timeline view that
// assumes that all flags are set.
await allowedError(
serviceManager.service.setVMTimelineFlags(['GC', 'Dart', 'Embedder']));
serviceManager.service!.setVMTimelineFlags(['GC', 'Dart', 'Embedder']));

// TODO(kenz): only call these if http logging and socket profiling are not
// already enabled. Listen to service manager streams for this info.
Expand All @@ -316,14 +315,14 @@ class NetworkController

Future<bool> recordingHttpTraffic() async {
bool enabled = true;
await serviceManager.service.forEachIsolate(
final service = serviceManager.service!;
await service.forEachIsolate(
(isolate) async {
final httpFuture =
serviceManager.service.httpEnableTimelineLogging(isolate.id);
final httpFuture = service.httpEnableTimelineLogging(isolate.id!);
// The above call won't complete immediately if the isolate is paused,
// so give up waiting after 500ms.
final state = await timeout(httpFuture, 500);
if (state == null || !state.enabled) {
if (state.enabled != true) {
enabled = false;
}
},
Expand Down Expand Up @@ -362,13 +361,13 @@ class NetworkController
String search, {
bool searchPreviousMatches = false,
}) {
if (search == null || search.isEmpty) return [];
if (search.isEmpty) return [];
final matches = <NetworkRequest>[];
final caseInsensitiveSearch = search.toLowerCase();

final currentRequests = filteredData.value;
for (final request in currentRequests) {
if (request.uri.toLowerCase().contains(caseInsensitiveSearch)) {
if (request.uri!.toLowerCase().contains(caseInsensitiveSearch)) {
matches.add(request);
// TODO(kenz): use the value request.isSearchMatch in the network
// requests table to improve performance. This will require some
Expand All @@ -379,41 +378,43 @@ class NetworkController
}

@override
void filterData(Filter<NetworkRequest> filter) {
void filterData(Filter<NetworkRequest>? filter) {
serviceManager.errorBadgeManager.clearErrors(NetworkScreen.id);
if (filter?.queryFilter == null) {
if (filter == null || filter.queryFilter == null) {
_requests.value.requests.forEach(_checkForError);
filteredData
..clear()
..addAll(_requests.value.requests);
} else {
final queryFilter = filter.queryFilter!;
filteredData
..clear()
..addAll(_requests.value.requests.where((NetworkRequest r) {
final methodArg = filter.queryFilter.filterArguments[methodFilterId];
final methodArg = queryFilter.filterArguments[methodFilterId];
if (methodArg != null &&
!methodArg.matchesValue(r.method.toLowerCase())) {
!methodArg.matchesValue(r.method!.toLowerCase())) {
return false;
}

final statusArg = filter.queryFilter.filterArguments[statusFilterId];
final statusArg = queryFilter.filterArguments[statusFilterId];
if (statusArg != null &&
!statusArg.matchesValue(r.status?.toLowerCase())) {
!statusArg.matchesValue(r.status!.toLowerCase())) {
return false;
}

final typeArg = filter.queryFilter.filterArguments[typeFilterId];
final typeArg = queryFilter.filterArguments[typeFilterId];
if (typeArg != null && !typeArg.matchesValue(r.type.toLowerCase())) {
return false;
}

if (filter.queryFilter.substrings.isNotEmpty) {
for (final substring in filter.queryFilter.substrings) {
if (queryFilter.substrings.isNotEmpty) {
for (final substring in queryFilter.substrings) {
final caseInsensitiveSubstring = substring.toLowerCase();
bool matches(String stringToMatch) {
bool matches(String? stringToMatch) {
if (stringToMatch
.toLowerCase()
.contains(caseInsensitiveSubstring)) {
?.toLowerCase()
.contains(caseInsensitiveSubstring) ==
true) {
_checkForError(r);
return true;
}
Expand All @@ -422,7 +423,7 @@ class NetworkController

if (matches(r.uri)) return true;
if (matches(r.method)) return true;
if (matches(r.status ?? '')) return true;
if (matches(r.status)) return true;
if (matches(r.type)) return true;
}
return false;
Expand Down
Loading