Avoid conflating concept of duration in TimeRange util class (#9325)

`TimeRange` represents the range between two `start` and `end` times marked in microseconds (so a duration). However, it stored `start` and `end` as `Duration` as well, resulting in the existence of three durations which was a bit confusing as I worked to understand code using `TimeRange`.

Beyond that, `TimeRange` was not immutable so had a concept of being "well formed" (or having both a start and end), resulting in multiple checks for that state and many not-null assertion operations. To maintain the intermediate status and also reduce the necessity of the not-null assertions, split out the intermediate state to a new `TimeRangeBuilder` class with a `build` method that consolidates the not-null assertions to that singular function.

Author: parlough

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

@@ -696,7 +696,7 @@ class NetworkRequestOverviewView extends StatelessWidget {
     // flex so that we can set a minimum width for small timing chunks.
     final timingWidgets = <Widget>[];
     for (final instant in data.instantEvents) {
-      final duration = instant.timeRange!.duration;
+      final duration = instant.timeRange.duration;
       timingWidgets.add(_buildTimingRow(nextColor(), instant.name, duration));
     }
     final duration = Duration(
@@ -714,14 +714,14 @@ class NetworkRequestOverviewView extends StatelessWidget {
     final data = this.data as DartIOHttpRequestData;
     final result = <Widget>[];
     for (final instant in data.instantEvents) {
-      final instantEventStart = data.instantEvents.first.timeRange!.start!;
-      final timeRange = instant.timeRange!;
+      final instantEventStart = data.instantEvents.first.timeRange.start;
+      final timeRange = instant.timeRange;
       final startDisplay = durationText(
-        timeRange.start! - instantEventStart,
+        Duration(microseconds: timeRange.start - instantEventStart),
         unit: DurationDisplayUnit.milliseconds,
       );
       final endDisplay = durationText(
-        timeRange.end! - instantEventStart,
+        Duration(microseconds: timeRange.end - instantEventStart),
         unit: DurationDisplayUnit.milliseconds,
       );
       final totalDisplay = durationText(

packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frame_model.dart

@@ -2,8 +2,6 @@
 // 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:math' as math;
-
 import '../../../../shared/primitives/utils.dart';
 import '../../performance_model.dart';
 import '../controls/enhance_tracing/enhance_tracing_model.dart';
@@ -24,12 +22,10 @@ class FlutterFrame {
   });
 
   factory FlutterFrame.fromJson(Map<String, Object?> json) {
-    final timeStart = Duration(microseconds: json[startTimeKey]! as int);
-    final timeEnd =
-        timeStart + Duration(microseconds: json[elapsedKey]! as int);
-    final frameTime = TimeRange()
-      ..start = timeStart
-      ..end = timeEnd;
+    final frameTime = TimeRange.ofDuration(
+      json[elapsedKey]! as int,
+      start: json[startTimeKey]! as int,
+    );
     return FlutterFrame._(
       id: json[numberKey]! as int,
       timeFromFrameTiming: frameTime,
@@ -58,14 +54,6 @@ class FlutterFrame {
   /// which the data was parsed.
   final TimeRange timeFromFrameTiming;
 
-  /// The time range of the Flutter frame based on the frame's
-  /// [timelineEventData], which contains timing information from the VM's
-  /// timeline events.
-  ///
-  /// This time range should be used for activities related to timeline events,
-  /// like scrolling a frame's timeline events into view, for example.
-  TimeRange get timeFromEventFlows => timelineEventData.time;
-
   /// Build time for this Flutter frame based on data from the FrameTiming API
   /// sent over the extension stream as 'Flutter.Frame' events.
   final Duration buildTime;
@@ -85,13 +73,12 @@ class FlutterFrame {
   /// (e.g. when the 'Flutter.Frame' event for this frame was received).
   ///
   /// If we did not have [EnhanceTracingState] information at the time that this
-  /// frame was drawn (e.g. the DevTools performancd page was not opened and
+  /// frame was drawn (e.g. the DevTools performance page was not opened and
   /// listening for frames yet), this value will be null.
   EnhanceTracingState? enhanceTracingState;
 
   FrameAnalysis? get frameAnalysis {
-    final frameAnalysis_ = _frameAnalysis;
-    if (frameAnalysis_ != null) return frameAnalysis_;
+    if (_frameAnalysis case final frameAnalysis?) return frameAnalysis;
     if (timelineEventData.isNotEmpty) {
       return _frameAnalysis = FrameAnalysis(this);
     }
@@ -104,15 +91,17 @@ class FlutterFrame {
 
   Duration get shaderDuration {
     if (_shaderTime != null) return _shaderTime!;
-    if (timelineEventData.rasterEvent == null) return Duration.zero;
-    final shaderEvents = timelineEventData.rasterEvent!
-        .shallowNodesWithCondition((event) => event.isShaderEvent);
-    final duration = shaderEvents.fold<Duration>(Duration.zero, (
-      previous,
-      event,
-    ) {
-      return previous + event.time.duration;
-    });
+    final rasterEvent = timelineEventData.rasterEvent;
+    if (rasterEvent == null) return Duration.zero;
+    final shaderEvents = rasterEvent.shallowNodesWithCondition(
+      (event) => event.isShaderEvent,
+    );
+    final duration = shaderEvents
+        .where((event) => event.isComplete)
+        .fold<Duration>(
+          Duration.zero,
+          (previous, event) => previous + event.time.duration,
+        );
     return _shaderTime = duration;
   }
 
@@ -150,7 +139,7 @@ class FlutterFrame {
 
   Map<String, Object?> get json => {
     numberKey: id,
-    startTimeKey: timeFromFrameTiming.start!.inMicroseconds,
+    startTimeKey: timeFromFrameTiming.start,
     elapsedKey: timeFromFrameTiming.duration.inMicroseconds,
     buildKey: buildTime.inMicroseconds,
     rasterKey: rasterTime.inMicroseconds,
@@ -197,42 +186,9 @@ class FrameTimelineEventData {
 
   bool get isNotEmpty => uiEvent != null || rasterEvent != null;
 
-  final time = TimeRange();
-
-  void setEventFlow({
-    required FlutterTimelineEvent event,
-    bool setTimeData = true,
-  }) {
+  void setEventFlow({required FlutterTimelineEvent event}) {
     final type = event.type!;
     _eventFlows[type.index] = event;
-    if (setTimeData) {
-      if (type == TimelineEventType.ui) {
-        time.start = event.time.start;
-        // If [rasterEventFlow] has already completed, set the end time for this
-        // frame to [event]'s end time.
-        if (rasterEvent != null) {
-          time.end = event.time.end;
-        }
-      } else if (type == TimelineEventType.raster) {
-        // If [uiEventFlow] is null, that means that this raster event flow
-        // completed before the ui event flow did for this frame. This means one
-        // of two things: 1) there will never be a [uiEventFlow] for this frame
-        // because the UI events are not present in the available timeline
-        // events, or 2) the [uiEventFlow] has started but not completed yet. In
-        // the event that 2) is true, do not set the frame end time here because
-        // the end time for this frame will be set to the end time for
-        // [uiEventFlow] once it finishes.
-        final theUiEvent = uiEvent;
-        if (theUiEvent != null) {
-          time.end = Duration(
-            microseconds: math.max(
-              theUiEvent.time.end!.inMicroseconds,
-              event.time.end?.inMicroseconds ?? 0,
-            ),
-          );
-        }
-      }
-    }
   }
 
   FlutterTimelineEvent? eventByType(TimelineEventType type) {

packages/devtools_app/lib/src/screens/performance/panes/flutter_frames/flutter_frames_controller.dart

@@ -165,7 +165,7 @@ class FlutterFramesController extends PerformanceFeatureController {
     assert(frame.isWellFormed);
     firstWellFormedFrameMicros = math.min(
       firstWellFormedFrameMicros ?? maxJsInt,
-      frame.timeFromFrameTiming.start!.inMicroseconds,
+      frame.timeFromFrameTiming.start,
     );
   }
 

packages/devtools_app/lib/src/screens/performance/panes/frame_analysis/frame_analysis_model.dart

@@ -101,10 +101,7 @@ class FrameAnalysis {
   ///
   /// This is drawn from all events for this frame from the raster thread.
   late FramePhase rasterPhase = FramePhase.raster(
-    events: [
-      if (frame.timelineEventData.rasterEvent != null)
-        frame.timelineEventData.rasterEvent!,
-    ],
+    events: [?frame.timelineEventData.rasterEvent],
   );
 
   late FramePhase longestUiPhase = _calculateLongestFramePhase();
@@ -276,9 +273,12 @@ class FramePhase {
     : title = type.display,
       duration =
           duration ??
-          events.fold<Duration>(Duration.zero, (previous, event) {
-            return previous + event.time.duration;
-          });
+          events
+              .where((event) => event.isComplete)
+              .fold<Duration>(
+                Duration.zero,
+                (previous, event) => previous + event.time.duration,
+              );
 
   factory FramePhase.build({
     required List<FlutterTimelineEvent> events,

packages/devtools_app/lib/src/screens/performance/panes/timeline_events/perfetto/_perfetto_web.dart

@@ -19,7 +19,6 @@ import '../../../../../shared/analytics/constants.dart' as gac;
 import '../../../../../shared/globals.dart';
 import '../../../../../shared/primitives/utils.dart';
 import '../../../../../shared/utils/utils.dart';
-import '../../../performance_utils.dart';
 import '_perfetto_controller_web.dart';
 import 'perfetto_controller.dart';
 
@@ -205,10 +204,6 @@ class _PerfettoViewController extends DisposableController
   Future<void> _scrollToTimeRange(TimeRange? timeRange) async {
     if (timeRange == null) return;
 
-    if (!timeRange.isWellFormed) {
-      pushNoTimelineEventsAvailableWarning();
-      return;
-    }
     await _pingPerfettoUntilReady();
     ga.select(
       gac.performance,
@@ -217,8 +212,8 @@ class _PerfettoViewController extends DisposableController
     _postMessage({
       'perfetto': {
         // Pass the values to Perfetto in seconds.
-        'timeStart': timeRange.start!.inMicroseconds / 1000000,
-        'timeEnd': timeRange.end!.inMicroseconds / 1000000,
+        'timeStart': timeRange.start / 1000000,
+        'timeEnd': timeRange.end / 1000000,
         // The time range should take up 80% of the visible window.
         'viewPercentage': 0.8,
       },

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

@@ -116,8 +116,7 @@ class FlutterTimelineEventProcessor {
 
     // Since this event is complete, move back up the tree to the nearest
     // incomplete event.
-    while (current!.parent != null &&
-        current.parent!.time.end?.inMicroseconds != null) {
+    while (current!.parent?.isComplete ?? false) {
       current = current.parent;
     }
     currentTimelineEventsByTrackId[trackId] = current.parent;

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

@@ -436,9 +436,7 @@ class TimelineEventsController extends PerformanceFeatureController
       );
     }
 
-    if (frame.timeFromFrameTiming.isWellFormed) {
-      perfettoController.scrollToTimeRange(frame.timeFromFrameTiming);
-    }
+    perfettoController.scrollToTimeRange(frame.timeFromFrameTiming);
   }
 
   void addTimelineEvent(FlutterTimelineEvent event) {
@@ -462,7 +460,7 @@ class TimelineEventsController extends PerformanceFeatureController
       } else {
         final unassignedEventsForFrame = _unassignedFlutterTimelineEvents
             .putIfAbsent(frameNumber, () => FrameTimelineEventData());
-        unassignedEventsForFrame.setEventFlow(event: event, setTimeData: false);
+        unassignedEventsForFrame.setEventFlow(event: event);
       }
     }
   }

packages/devtools_app/lib/src/screens/performance/performance_model.dart

@@ -98,21 +98,36 @@ extension type _PerformanceDataJson(Map<String, Object?> json) {
 }
 
 class FlutterTimelineEvent extends TreeNode<FlutterTimelineEvent> {
-  FlutterTimelineEvent(PerfettoTrackEvent firstTrackEvent)
-    : trackEvents = [firstTrackEvent],
-      type = firstTrackEvent.timelineEventType {
-    time.start = Duration(microseconds: firstTrackEvent.timestampMicros);
-  }
+  factory FlutterTimelineEvent(PerfettoTrackEvent firstTrackEvent) =>
+      FlutterTimelineEvent._(
+        trackEvents: [firstTrackEvent],
+        type: firstTrackEvent.timelineEventType,
+        timeBuilder: TimeRangeBuilder(start: firstTrackEvent.timestampMicros),
+      );
+
+  FlutterTimelineEvent._({
+    required this.trackEvents,
+    required this.type,
+    required TimeRangeBuilder timeBuilder,
+  }) : _timeBuilder = timeBuilder;
 
   static const rasterEventName = 'Rasterizer::DoDraw';
   static const uiEventName = 'Animator::BeginFrame';
 
   /// Perfetto track events associated with this [FlutterTimelineEvent].
   final List<PerfettoTrackEvent> trackEvents;
 
-  TimelineEventType? type;
+  final TimelineEventType? type;
+
+  final TimeRangeBuilder _timeBuilder;
+
+  /// The time range of this event.
+  ///
+  /// Throws if [isComplete] is false.
+  TimeRange get time => _timeBuilder.build();
 
-  TimeRange time = TimeRange();
+  /// Whether this event is complete and has received an end track event.
+  bool get isComplete => _timeBuilder.canBuild;
 
   String? get name => trackEvents.first.name;
 
@@ -123,26 +138,17 @@ class FlutterTimelineEvent extends TreeNode<FlutterTimelineEvent> {
   bool get isShaderEvent =>
       trackEvents.first.isShaderEvent || trackEvents.last.isShaderEvent;
 
-  bool get isWellFormed => time.start != null && time.end != null;
-
   void addEndTrackEvent(PerfettoTrackEvent event) {
-    time.end = Duration(microseconds: event.timestampMicros);
+    _timeBuilder.end = event.timestampMicros;
     trackEvents.add(event);
   }
 
   @override
-  FlutterTimelineEvent shallowCopy() {
-    final copy = FlutterTimelineEvent(trackEvents.first);
-    for (int i = 1; i < trackEvents.length; i++) {
-      copy.trackEvents.add(trackEvents[i]);
-    }
-    copy
-      ..type = type
-      ..time = (TimeRange()
-        ..start = time.start
-        ..end = time.end);
-    return copy;
-  }
+  FlutterTimelineEvent shallowCopy() => FlutterTimelineEvent._(
+    trackEvents: trackEvents.toList(),
+    type: type,
+    timeBuilder: _timeBuilder.copy(),
+  );
 
   @visibleForTesting
   FlutterTimelineEvent deepCopy() {
@@ -174,7 +180,12 @@ class FlutterTimelineEvent extends TreeNode<FlutterTimelineEvent> {
   }
 
   void format(StringBuffer buf, String indent) {
-    buf.writeln('$indent$name $time');
+    buf.write('$indent$name');
+    if (isComplete) {
+      buf.write(time);
+    }
+
+    buf.writeln(' ');
     for (final child in children) {
       child.format(buf, '  $indent');
     }