Fix performance when JSON is invalid

packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart

@@ -498,11 +498,12 @@ class TimelineEventsController extends PerformanceFeatureController
 
   @override
   Future<void> 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(

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<String, Object?> toJson() => {
     traceBinaryKey: perfettoTraceBinary,

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<String, Object?> json) {
     return connectedApp == null ? {} : connectedApp.cast<String, Object?>();
   }
 
-  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 {

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<T>
     on DevToolsScreenController, AutoDisposeControllerMixin {
   final _exportController = ExportController();
@@ -168,19 +171,41 @@ mixin OfflineScreenControllerMixin<T>
     required FutureOr<void> 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<String, Object?>.of(
-        (offlineDataController.offlineDataJson[screenId] as Map)
-            .cast<String, Object?>(),
-      );
-      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<String, Object?>.of(
+          (offlineDataController.offlineDataJson[screenId] as Map)
+              .cast<String, Object?>(),
+        );
+        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;

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -18,14 +18,19 @@ 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
 
 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
 

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);

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: <String, Object?>{
+    'devToolsSnapshot': true,
+    'example': {'title': 'example custom tools'},
+  },
+);
+final devToolsFileJsonWithNonStringActiveScreenId = DevToolsJsonFile(
+  name: 'devToolsFileJsonWithNonStringActiveScreenId',
+  lastModifiedTime: DateTime.fromMicrosecondsSinceEpoch(3500),
+  data: <String, Object?>{
+    'devToolsSnapshot': true,
+    'activeScreenId': 12345,
+    'example': {'title': 'example custom tools'},
+  },
+);
+final devToolsFileJsonWithoutScreenData = DevToolsJsonFile(
+  name: 'devToolsFileJsonWithoutScreenData',
+  lastModifiedTime: DateTime.fromMicrosecondsSinceEpoch(4000),
+  data: <String, Object?>{
+    'devToolsSnapshot': true,
+    'activeScreenId': 'example',
+  },
+);