Delete unnecessary wrapper classes and further densify log display fo…

…r logging v2 (#8409)

packages/devtools_app/lib/devtools_app.dart

@@ -29,6 +29,7 @@ export 'src/screens/inspector_shared/inspector_screen.dart';
 export 'src/screens/inspector_shared/inspector_screen_controller.dart';
 export 'src/screens/logging/logging_controller.dart';
 export 'src/screens/logging/logging_screen.dart';
+export 'src/screens/logging/logging_screen_v2/log_data.dart';
 export 'src/screens/logging/logging_screen_v2/logging_controller_v2.dart';
 export 'src/screens/logging/logging_screen_v2/logging_model.dart';
 export 'src/screens/logging/logging_screen_v2/logging_screen_v2.dart';

packages/devtools_app/lib/src/screens/logging/logging_screen_v2/log_data.dart

@@ -0,0 +1,96 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+
+import 'package:flutter/foundation.dart';
+
+import '../../../shared/diagnostics/diagnostics_node.dart';
+import '../../../shared/primitives/utils.dart';
+import '../../../shared/ui/search.dart';
+
+/// A log data object that includes optional summary information about whether
+/// the log entry represents an error entry, the log entry kind, and more
+/// detailed data for the entry.
+///
+/// The details can optionally be loaded lazily on first use. If this is the
+/// case, this log entry will have a non-null `detailsComputer` field. After the
+/// data is calculated, the log entry will be modified to contain the calculated
+/// `details` data.
+class LogDataV2 with SearchableDataMixin {
+  LogDataV2(
+    this.kind,
+    this._details,
+    this.timestamp, {
+    this.summary,
+    this.isError = false,
+    this.detailsComputer,
+    this.node,
+  }) {
+    // Fetch details immediately on creation.
+    unawaited(compute());
+  }
+
+  final String kind;
+  final int? timestamp;
+  final bool isError;
+  final String? summary;
+
+  final RemoteDiagnosticsNode? node;
+  String? _details;
+  Future<String> Function()? detailsComputer;
+
+  static const prettyPrinter = JsonEncoder.withIndent('  ');
+
+  String? get details => _details;
+
+  ValueListenable<bool> get detailsComputed => _detailsComputed;
+  final _detailsComputed = ValueNotifier<bool>(false);
+
+  Future<void> compute() async {
+    if (!detailsComputed.value) {
+      if (detailsComputer != null) {
+        _details = await detailsComputer!();
+      }
+      detailsComputer = null;
+      _detailsComputed.value = true;
+    }
+  }
+
+  /// The current calculated display height.
+  double? height;
+
+  /// The current offset of this log entry in the logs table.
+  double? offset;
+
+  String? prettyPrinted() {
+    if (!detailsComputed.value) {
+      return details?.trim();
+    }
+
+    try {
+      return prettyPrinter
+          .convert(jsonDecode(details!))
+          .replaceAll(r'\n', '\n')
+          .trim();
+    } catch (_) {
+      return details?.trim();
+    }
+  }
+
+  String asLogDetails() {
+    return !detailsComputed.value ? '<fetching>' : prettyPrinted() ?? '';
+  }
+
+  @override
+  bool matchesSearchToken(RegExp regExpSearch) {
+    return kind.caseInsensitiveContains(regExpSearch) ||
+        (summary?.caseInsensitiveContains(regExpSearch) == true) ||
+        (details?.caseInsensitiveContains(regExpSearch) == true);
+  }
+
+  @override
+  String toString() => 'LogData($kind, $timestamp)';
+}

packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_controller_v2.dart

@@ -19,14 +19,14 @@ import '../../../shared/globals.dart';
 import '../../../shared/primitives/byte_utils.dart';
 import '../../../shared/primitives/message_bus.dart';
 import '../../../shared/primitives/utils.dart';
-import '../../../shared/ui/search.dart';
 import '../logging_controller.dart'
     show
         FrameInfo,
         ImageSizesForFrame,
         NavigationInfo,
         ServiceExtensionStateChangedInfo;
 import '../logging_screen.dart';
+import 'log_data.dart';
 import 'logging_model.dart';
 
 final _log = Logger('logging_controller');
@@ -585,80 +585,3 @@ String? _valueAsString(InstanceRef? ref) {
       ? '${ref.valueAsString}...'
       : ref.valueAsString;
 }
-
-/// A log data object that includes optional summary information about whether
-/// the log entry represents an error entry, the log entry kind, and more
-/// detailed data for the entry.
-///
-/// The details can optionally be loaded lazily on first use. If this is the
-/// case, this log entry will have a non-null `detailsComputer` field. After the
-/// data is calculated, the log entry will be modified to contain the calculated
-/// `details` data.
-class LogDataV2 with SearchableDataMixin {
-  LogDataV2(
-    this.kind,
-    this._details,
-    this.timestamp, {
-    this.summary,
-    this.isError = false,
-    this.detailsComputer,
-    this.node,
-  }) {
-    // Fetch details immediately on creation.
-    unawaited(compute());
-  }
-
-  final String kind;
-  final int? timestamp;
-  final bool isError;
-  final String? summary;
-
-  final RemoteDiagnosticsNode? node;
-  String? _details;
-  Future<String> Function()? detailsComputer;
-
-  static const prettyPrinter = JsonEncoder.withIndent('  ');
-
-  String? get details => _details;
-
-  ValueListenable<bool> get detailsComputed => _detailsComputed;
-  final _detailsComputed = ValueNotifier<bool>(false);
-
-  Future<void> compute() async {
-    if (!detailsComputed.value) {
-      if (detailsComputer != null) {
-        _details = await detailsComputer!();
-      }
-      detailsComputer = null;
-      _detailsComputed.value = true;
-    }
-  }
-
-  String? prettyPrinted() {
-    if (!detailsComputed.value) {
-      return details;
-    }
-
-    try {
-      return prettyPrinter
-          .convert(jsonDecode(details!))
-          .replaceAll(r'\n', '\n');
-    } catch (_) {
-      return details;
-    }
-  }
-
-  String asLogDetails() {
-    return !detailsComputed.value ? '<fetching>' : prettyPrinted() ?? '';
-  }
-
-  @override
-  bool matchesSearchToken(RegExp regExpSearch) {
-    return kind.caseInsensitiveContains(regExpSearch) ||
-        (summary?.caseInsensitiveContains(regExpSearch) == true) ||
-        (details?.caseInsensitiveContains(regExpSearch) == true);
-  }
-
-  @override
-  String toString() => 'LogData($kind, $timestamp)';
-}

packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_model.dart

@@ -28,6 +28,7 @@ import '../logging_controller.dart'
         ImageSizesForFrame,
         NavigationInfo,
         ServiceExtensionStateChangedInfo;
+import 'log_data.dart';
 import 'logging_controller_v2.dart';
 import 'logging_table_row.dart';
 import 'logging_table_v2.dart';
@@ -170,7 +171,7 @@ class LoggingTableModel extends DisposableController
   ObjectGroup get objectGroup =>
       serviceConnection.consoleService.objectGroup as ObjectGroup;
 
-  final _logs = ListQueue<_LogEntry>();
+  final _logs = ListQueue<LogDataV2>();
 
   /// [FilterControllerMixin] uses [ListValueNotifier] which isn't well optimized to the
   /// retention limit behavior that [LoggingTableModel] uses. So we use
@@ -179,7 +180,7 @@ class LoggingTableModel extends DisposableController
   /// we use [_filteredLogs]. After any changes are done to [_filteredLogs], [notifyListeners]
   /// must be manually triggered, since the listener behaviour is accomplished by the
   /// [LoggingTableModel] being a [ChangeNotifier].
-  final _filteredLogs = ListQueue<_FilteredLogEntry>();
+  final _filteredLogs = ListQueue<LogDataV2>();
 
   final _selectedLogs = ListQueue<LogDataV2>();
   late int _retentionLimit;
@@ -464,16 +465,15 @@ class LoggingTableModel extends DisposableController
   }
 
   void add(LogDataV2 log) {
-    final newEntry = _LogEntry(log);
-    _logs.add(newEntry);
+    _logs.add(log);
     getLogHeight(_logs.length - 1);
     _trimOneOutOfRetentionLog();
 
-    if (!_filterCallback(newEntry)) {
+    if (!_filterCallback(log)) {
       // Only add the log to filtered logs if it matches the filter.
       return;
     }
-    _filteredLogs.add(_FilteredLogEntry(newEntry));
+    _filteredLogs.add(log);
 
     // TODO(danchevalier): Calculate the new offset here
 
@@ -604,9 +604,7 @@ class LoggingTableModel extends DisposableController
 
     _filteredLogs
       ..clear()
-      ..addAll(
-        _logs.where(_filterCallback).map((e) => _FilteredLogEntry(e)).toList(),
-      );
+      ..addAll(_logs.where(_filterCallback).toList());
 
     _recalculateOffsets();
 
@@ -615,10 +613,9 @@ class LoggingTableModel extends DisposableController
     notifyListeners();
   }
 
-  bool _filterCallback(_LogEntry entry) {
+  bool _filterCallback(LogDataV2 log) {
     final filter = activeFilter.value;
 
-    final log = entry.log;
     final filteredOutByToggleFilters = filter.toggleFilters.any(
       (toggleFilter) =>
           toggleFilter.enabled.value && !toggleFilter.includeCallback(log),
@@ -672,8 +669,7 @@ class LoggingTableModel extends DisposableController
   }
 
   /// Get the filtered log at [index].
-  LogDataV2 filteredLogAt(int index) =>
-      _filteredLogs.elementAt(index).logEntry.log;
+  LogDataV2 filteredLogAt(int index) => _filteredLogs.elementAt(index);
 
   double _tableWidth = 0.0;
 
@@ -690,7 +686,7 @@ class LoggingTableModel extends DisposableController
 
   void _trimOneOutOfRetentionLog() {
     if (_logs.length > _retentionLimit) {
-      if (identical(_logs.first.log, _filteredLogs.first.logEntry.log)) {
+      if (identical(_logs.first, _filteredLogs.first)) {
         // Remove a filtered log if it is about to go out of retention.
         _filteredLogs.removeFirst();
       }
@@ -713,23 +709,23 @@ class LoggingTableModel extends DisposableController
   }
 
   double getLogHeight(int index) {
-    final entry = _logs.elementAt(index);
-    final cachedHeight = entry.height;
+    final log = _logs.elementAt(index);
+    final cachedHeight = log.height;
     if (cachedHeight != null) return cachedHeight;
-    return entry.height ??= LoggingTableRow.estimateRowHeight(
-      entry.log,
+    return log.height ??= LoggingTableRow.estimateRowHeight(
+      log,
       _tableWidth,
     );
   }
 
   /// Get the height of a filtered Log at [index].
   double getFilteredLogHeight(int index) {
     final filteredLog = _filteredLogs.elementAt(index);
-    final cachedHeight = filteredLog.logEntry.height;
+    final cachedHeight = filteredLog.height;
     if (cachedHeight != null) return cachedHeight;
 
-    return filteredLog.logEntry.height ??= LoggingTableRow.estimateRowHeight(
-      filteredLog.logEntry.log,
+    return filteredLog.height ??= LoggingTableRow.estimateRowHeight(
+      filteredLog,
       _tableWidth,
     );
   }
@@ -844,29 +840,3 @@ String? _valueAsString(InstanceRef? ref) {
       ? '${ref.valueAsString}...'
       : ref.valueAsString;
 }
-
-/// A class for holding a [LogDataV2] and its current estimated [height].
-///
-/// The [log] and its [height] have similar lifecycles, so it is helpful to keep
-/// them tied together.
-class _LogEntry {
-  _LogEntry(this.log);
-  final LogDataV2 log;
-
-  /// The current calculated height [log].
-  double? height;
-}
-
-/// A class for holding a [logEntry] and its [offset] from the top of a list of
-/// filtered entries.
-///
-/// The [logEntry] and its [offset] have similar lifecycles, so it is helpful to keep
-/// them tied together.
-class _FilteredLogEntry {
-  _FilteredLogEntry(this.logEntry);
-
-  final _LogEntry logEntry;
-
-  /// The offset of this log entry in a view.
-  double? offset;
-}

packages/devtools_app/lib/src/screens/logging/logging_screen_v2/logging_screen_v2.dart

@@ -12,6 +12,7 @@ import '../../../shared/analytics/analytics.dart' as ga;
 import '../../../shared/analytics/constants.dart' as gac;
 import '../../../shared/screen.dart';
 import '../../../shared/utils.dart';
+import 'log_data.dart';
 import 'logging_controller_v2.dart';
 import 'logging_table_v2.dart';