diff --git a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart index c37291140d6..4d1e8e1382c 100644 --- a/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart +++ b/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart @@ -498,11 +498,12 @@ class TimelineEventsController extends PerformanceFeatureController @override Future setOfflineData(OfflinePerformanceData offlineData) async { - if (offlineData.perfettoTraceBinary != null) { + if (offlineData.perfettoTraceBinary != null && + offlineData.perfettoTraceBinary!.isNotEmpty) { _updatePerfettoTrace(offlineData.perfettoTraceBinary!); + processTrackEvents(); + await loadPerfettoTrace(); } - processTrackEvents(); - await loadPerfettoTrace(); if (offlineData.selectedFrame != null) { perfettoController.scrollToTimeRange( diff --git a/packages/devtools_app/lib/src/screens/performance/performance_model.dart b/packages/devtools_app/lib/src/screens/performance/performance_model.dart index 89c310747ff..2b6059b98b6 100644 --- a/packages/devtools_app/lib/src/screens/performance/performance_model.dart +++ b/packages/devtools_app/lib/src/screens/performance/performance_model.dart @@ -65,7 +65,10 @@ class OfflinePerformanceData { /// tab they exported from. final int selectedTab; - bool get isEmpty => perfettoTraceBinary == null; + bool get isEmpty => + (perfettoTraceBinary == null || perfettoTraceBinary!.isEmpty) && + frames.isEmpty && + rebuildCountModel == null; Map toJson() => { traceBinaryKey: perfettoTraceBinary, diff --git a/packages/devtools_app/lib/src/shared/config_specific/import_export/import_export.dart b/packages/devtools_app/lib/src/shared/config_specific/import_export/import_export.dart index eed968af961..2f3f0e949fa 100644 --- a/packages/devtools_app/lib/src/shared/config_specific/import_export/import_export.dart +++ b/packages/devtools_app/lib/src/shared/config_specific/import_export/import_export.dart @@ -69,6 +69,14 @@ class ImportController { final devToolsOfflineData = _DevToolsOfflineData(json); // TODO(kenz): support imports for more than one screen at a time. final activeScreenId = devToolsOfflineData.activeScreenId; + if (activeScreenId == null) { + notificationService.push( + 'The imported file is not a valid DevTools snapshot because it does ' + 'not contain an activeScreenId field.', + ); + return; + } + if (expectedScreenId != null && activeScreenId != expectedScreenId) { notificationService.push( 'Expected a data file for screen \'$expectedScreenId\' but received one' @@ -89,6 +97,14 @@ class ImportController { } } + if (!devToolsOfflineData.json.containsKey(activeScreenId) || + devToolsOfflineData.json[activeScreenId] == null) { + notificationService.push( + 'The imported file does not contain data for screen \'$activeScreenId\'.', + ); + return; + } + final connectedApp = OfflineConnectedApp.parse( devToolsOfflineData.connectedApp, ); @@ -106,8 +122,10 @@ extension type _DevToolsOfflineData(Map json) { return connectedApp == null ? {} : connectedApp.cast(); } - String get activeScreenId => - json[DevToolsExportKeys.activeScreenId.name] as String; + String? get activeScreenId { + final value = json[DevToolsExportKeys.activeScreenId.name]; + return value is String ? value : null; + } } enum ExportFileType { diff --git a/packages/devtools_app/lib/src/shared/offline/offline_data.dart b/packages/devtools_app/lib/src/shared/offline/offline_data.dart index a7c6f48ff91..7b17521f6ba 100644 --- a/packages/devtools_app/lib/src/shared/offline/offline_data.dart +++ b/packages/devtools_app/lib/src/shared/offline/offline_data.dart @@ -10,6 +10,7 @@ import 'dart:async'; import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/utils.dart'; import 'package:flutter/foundation.dart'; +import 'package:logging/logging.dart'; import '../config_specific/import_export/import_export.dart'; import '../framework/routing.dart'; @@ -131,6 +132,8 @@ class OfflineDataController { /// ), /// } /// ``` +final _log = Logger('offline_data'); + mixin OfflineScreenControllerMixin on DevToolsScreenController, AutoDisposeControllerMixin { final _exportController = ExportController(); @@ -168,19 +171,41 @@ mixin OfflineScreenControllerMixin required FutureOr Function(T data) loadData, }) async { if (offlineDataController.shouldLoadOfflineData(screenId)) { - // TODO(kenz): investigate this line of code. Do we need to be creating a - // second copy of the Map from offlineDataController.offlineDataJson or - // can we use it directly to save this `Map.of` call? - final json = Map.of( - (offlineDataController.offlineDataJson[screenId] as Map) - .cast(), - ); - final screenData = createData(json); + final T screenData; + try { + // TODO(kenz): investigate this line of code. Do we need to be creating a + // second copy of the Map from offlineDataController.offlineDataJson or + // can we use it directly to save this `Map.of` call? + final json = Map.of( + (offlineDataController.offlineDataJson[screenId] as Map) + .cast(), + ); + screenData = createData(json); + } catch (e, st) { + _log.shout('Error parsing offline data for $screenId', e, st); + notificationService.push( + 'Failed to load offline data for screen \'$screenId\': $e', + ); + return false; + } + if (shouldLoad(screenData)) { _loadingOfflineData.value = true; - await loadData(screenData); - _loadingOfflineData.value = false; - return true; + try { + await loadData(screenData); + return true; + } catch (e, st) { + _log.shout('Error loading offline data for $screenId', e, st); + notificationService.push( + 'Failed to load offline data for screen \'$screenId\': $e', + ); + } finally { + _loadingOfflineData.value = false; + } + } else { + notificationService.push( + 'The imported file does not contain any data for screen \'$screenId\'.', + ); } } return false; diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 01ad0a7d9e2..792b22f8543 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -18,6 +18,9 @@ To learn more about DevTools, check out the * Fixed unreadable text in the release notes panel, where blockquotes were drawn on a hard coded light blue background in the dark theme. [#9957](https://github.com/flutter/devtools/pull/9957) +* Added user-facing error notifications when importing data files that are + missing required fields or contain no data for the screen. + [TODO](https://github.com/flutter/devtools/pull/TODO) ## Inspector updates @@ -25,7 +28,9 @@ TODO: Remove this section if there are not any updates. ## Performance updates -TODO: Remove this section if there are not any updates. +* Fixed an issue where importing performance data with Flutter frames but no + timeline trace would treat the data as empty. + [TODO](https://github.com/flutter/devtools/pull/TODO) ## CPU profiler updates diff --git a/packages/devtools_app/test/screens/performance/performance_model_test.dart b/packages/devtools_app/test/screens/performance/performance_model_test.dart index 7dade0fd72f..23a0cccb6a0 100644 --- a/packages/devtools_app/test/screens/performance/performance_model_test.dart +++ b/packages/devtools_app/test/screens/performance/performance_model_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. +import 'dart:typed_data'; + import 'package:devtools_app/devtools_app.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -19,6 +21,25 @@ void main() { expect(offlineData.selectedTab, 0); }); + test('isEmpty', () { + expect(OfflinePerformanceData().isEmpty, isTrue); + expect( + OfflinePerformanceData(perfettoTraceBinary: Uint8List(0)).isEmpty, + isTrue, + ); + expect( + OfflinePerformanceData( + perfettoTraceBinary: Uint8List.fromList([1, 2, 3]), + ).isEmpty, + isFalse, + ); + expect(OfflinePerformanceData(frames: [testFrame0]).isEmpty, isFalse); + expect( + OfflinePerformanceData(rebuildCountModel: RebuildCountModel()).isEmpty, + isFalse, + ); + }); + test('init from parse', () { OfflinePerformanceData offlineData = OfflinePerformanceData.fromJson({}); expect(offlineData.frames, isEmpty); diff --git a/packages/devtools_app/test/shared/import_export_test.dart b/packages/devtools_app/test/shared/import_export_test.dart index 409531bfd3a..c723bb89e42 100644 --- a/packages/devtools_app/test/shared/import_export_test.dart +++ b/packages/devtools_app/test/shared/import_export_test.dart @@ -88,6 +88,46 @@ void main() { equals(attemptingToImportMessage('example')), ); }); + + test('importData pushes notification when activeScreenId is missing', () { + importController.importData(devToolsFileJsonWithoutActiveScreenId); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + equals( + 'The imported file is not a valid DevTools snapshot because it does ' + 'not contain an activeScreenId field.', + ), + ); + }); + + test( + 'importData pushes notification when activeScreenId is not a String', + () { + importController.importData( + devToolsFileJsonWithNonStringActiveScreenId, + ); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + equals( + 'The imported file is not a valid DevTools snapshot because it does ' + 'not contain an activeScreenId field.', + ), + ); + }, + ); + + test('importData pushes notification when screen data is missing', () { + importController.importData(devToolsFileJsonWithoutScreenData); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + equals( + 'The imported file does not contain data for screen \'example\'.', + ), + ); + }); }); } @@ -110,3 +150,28 @@ final devToolsFileJson = DevToolsJsonFile( 'example': {'title': 'example custom tools'}, }, ); +final devToolsFileJsonWithoutActiveScreenId = DevToolsJsonFile( + name: 'devToolsFileJsonWithoutActiveScreenId', + lastModifiedTime: DateTime.fromMicrosecondsSinceEpoch(3000), + data: { + 'devToolsSnapshot': true, + 'example': {'title': 'example custom tools'}, + }, +); +final devToolsFileJsonWithNonStringActiveScreenId = DevToolsJsonFile( + name: 'devToolsFileJsonWithNonStringActiveScreenId', + lastModifiedTime: DateTime.fromMicrosecondsSinceEpoch(3500), + data: { + 'devToolsSnapshot': true, + 'activeScreenId': 12345, + 'example': {'title': 'example custom tools'}, + }, +); +final devToolsFileJsonWithoutScreenData = DevToolsJsonFile( + name: 'devToolsFileJsonWithoutScreenData', + lastModifiedTime: DateTime.fromMicrosecondsSinceEpoch(4000), + data: { + 'devToolsSnapshot': true, + 'activeScreenId': 'example', + }, +); diff --git a/packages/devtools_app/test/shared/offline/offline_data_test.dart b/packages/devtools_app/test/shared/offline/offline_data_test.dart new file mode 100644 index 00000000000..d0f08a810ba --- /dev/null +++ b/packages/devtools_app/test/shared/offline/offline_data_test.dart @@ -0,0 +1,178 @@ +// Copyright 2026 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +import 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_app/src/shared/config_specific/import_export/import_export.dart'; +import 'package:devtools_app_shared/utils.dart'; +import 'package:devtools_test/devtools_test.dart'; +import 'package:flutter_test/flutter_test.dart'; + +class _TestScreenController extends DevToolsScreenController + with + AutoDisposeControllerMixin, + OfflineScreenControllerMixin> { + Map? loadedData; + + @override + String get screenId => 'test_screen'; + + Future initOfflineData( + String screenId, { + bool shouldLoad = true, + bool throwOnLoad = false, + }) { + return maybeLoadOfflineData( + screenId, + createData: (json) { + if (json.containsKey('throw')) { + throw Exception('Corrupted data'); + } + return json; + }, + shouldLoad: (data) => shouldLoad && data.isNotEmpty, + loadData: (data) { + if (throwOnLoad) { + throw Exception('Failed during loadData'); + } + loadedData = data; + }, + ); + } + + @override + OfflineScreenData prepareOfflineScreenData() => + OfflineScreenData(screenId: 'test_screen', data: loadedData ?? {}); +} + +void main() { + group('OfflineScreenControllerMixin', () { + late _TestScreenController controller; + late NotificationService notifications; + + setUp(() { + notifications = NotificationService(); + setGlobal(NotificationService, notifications); + setGlobal(OfflineDataController, OfflineDataController()); + setGlobal(ServiceConnectionManager, FakeServiceConnectionManager()); + controller = _TestScreenController(); + }); + + test('loads offline data when data is valid and non-empty', () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'test_screen', + 'test_screen': {'key': 'value'}, + }; + + final result = await controller.initOfflineData('test_screen'); + expect(result, isTrue); + expect(controller.loadedData, equals({'key': 'value'})); + expect(notifications.activeMessages, isEmpty); + }); + + test('notifies when screen data is empty', () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'test_screen', + 'test_screen': {}, + }; + + final result = await controller.initOfflineData('test_screen'); + expect(result, isFalse); + expect(controller.loadedData, isNull); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + equals( + 'The imported file does not contain any data for screen \'test_screen\'.', + ), + ); + }); + + test('notifies when shouldLoad returns false', () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'test_screen', + 'test_screen': {'key': 'value'}, + }; + + final result = await controller.initOfflineData( + 'test_screen', + shouldLoad: false, + ); + expect(result, isFalse); + expect(controller.loadedData, isNull); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + equals( + 'The imported file does not contain any data for screen \'test_screen\'.', + ), + ); + }); + + test('notifies when creating/loading data throws an error', () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'test_screen', + 'test_screen': {'throw': true}, + }; + + final result = await controller.initOfflineData('test_screen'); + expect(result, isFalse); + expect(controller.loadedData, isNull); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + contains('Failed to load offline data for screen \'test_screen\':'), + ); + }); + + test( + 'resets loadingOfflineData and notifies when loadData throws', + () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'test_screen', + 'test_screen': {'key': 'value'}, + }; + + expect(controller.loadingOfflineData.value, isFalse); + final result = await controller.initOfflineData( + 'test_screen', + throwOnLoad: true, + ); + expect(result, isFalse); + expect(controller.loadingOfflineData.value, isFalse); + expect(notifications.activeMessages.length, equals(1)); + expect( + notifications.activeMessages.first.text, + contains('Failed to load offline data for screen \'test_screen\':'), + ); + }, + ); + + test( + 'returns false without notifying if screen is not in offlineDataJson', + () async { + offlineDataController + ..startShowingOfflineData(offlineApp: MockConnectedApp()) + ..offlineDataJson = { + DevToolsExportKeys.activeScreenId.name: 'other_screen', + 'other_screen': {'key': 'value'}, + }; + + final result = await controller.initOfflineData('test_screen'); + expect(result, isFalse); + expect(controller.loadedData, isNull); + expect(notifications.activeMessages, isEmpty); + }, + ); + }); +}