[fix] html-table-render-jank

lib/features/perf/perf_logger.dart

@@ -26,8 +26,13 @@ const bool kPerfLoggingEnabled = bool.fromEnvironment('ENABLE_PERF_LOGGING');
 /// Keep at most this many session files on the device
 const int kMaxStoredPerfSessions = 20;
 
-/// Frames slower than this count as jank and get their own record
-const int kJankThresholdUs = 16700; // ~ one 60Hz frame budget
+/// Frames slower than this count as jank and get their own record.
+///
+/// Two 60Hz frame budgets, not one: on slow devices plain scrolling produces
+/// hundreds of ~20 ms frames, which used up [kMaxJankRecords] within minutes
+/// and left no records for the long frames later in the session. The frame
+/// histogram still counts every frame.
+const int kJankThresholdUs = 33400;
 
 /// At most this many individual jank records per session
 /// (the frame histogram keeps counting beyond that)
@@ -254,6 +259,28 @@ class PerfLogger {
     }
   }
 
+  /// Measure from now until the end of the current frame, i.e. including
+  /// the build, layout and paint work that whatever triggered it causes.
+  ///
+  /// Wrapping a widget's construction in [spanSync] measures next to nothing:
+  /// the expensive part happens later in the frame pipeline. Call this from
+  /// build() instead to attribute that cost.
+  static void spanToEndOfFrame(
+    String name, {
+    Map<String, Object?> Function()? data,
+  }) {
+    if (!enabled) return;
+    try {
+      if (!_clock.isRunning) _clock.start();
+      final int startUs = _clock.elapsedMicroseconds;
+      SchedulerBinding.instance.addPostFrameCallback(
+        (_) => _endSpan(name, startUs, true, data),
+      );
+    } catch (e) {
+      debugPrint('PerfLogger: starting span $name failed: $e');
+    }
+  }
+
   static void _endSpan(
     String name,
     int startUs,

lib/features/perf/perf_navigator_observer.dart

@@ -0,0 +1,45 @@
+import 'package:app4training/features/perf/perf_logger.dart';
+import 'package:flutter/widgets.dart';
+
+/// Records which kind of screen is shown, so jank frames in the perf logs
+/// can be attributed to a screen (e.g. a worksheet vs. the settings).
+///
+/// Only the first route segment is recorded: `/view/Healing/de` becomes
+/// `view` - page names and language codes stay out of the logs.
+/// `depth` is the number of screens on the stack (hidden ones included).
+class PerfNavigatorObserver extends NavigatorObserver {
+  int _depth = 0;
+
+  @override
+  void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    if (route is PageRoute) _depth++;
+    _recordScreen(route);
+  }
+
+  @override
+  void didPop(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    if (route is PageRoute) _depth--;
+    _recordScreen(previousRoute);
+  }
+
+  @override
+  void didRemove(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    if (route is PageRoute) _depth--;
+  }
+
+  @override
+  void didReplace({Route<dynamic>? newRoute, Route<dynamic>? oldRoute}) {
+    _recordScreen(newRoute);
+  }
+
+  void _recordScreen(Route<dynamic>? route) {
+    final String? name = route?.settings.name;
+    if (name == null) return;
+    final List<String> segments = name.split('/');
+    final String screen = segments.length > 1 ? segments[1] : name;
+    PerfLogger.event(
+      'screen',
+      data: {'screen': screen.isEmpty ? '/' : screen, 'depth': _depth},
+    );
+  }
+}

lib/main.dart

@@ -3,12 +3,14 @@ import 'dart:async';
 
 import 'package:app4training/data/language_downloader.dart';
 import 'package:app4training/features/perf/perf_logger.dart';
+import 'package:app4training/features/perf/perf_navigator_observer.dart';
 import 'package:app4training/l10n/generated/app_localizations.dart';
 import 'package:dio/dio.dart';
 import 'package:file/local.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_riverpod/flutter_riverpod.dart';
+import 'package:app4training/routes/route_stack_limiter.dart';
 import 'package:app4training/routes/routes.dart';
 import 'package:package_info_plus/package_info_plus.dart';
 import 'package:path/path.dart' as p;
@@ -191,6 +193,15 @@ void main() async {
 class App4Training extends ConsumerWidget {
   const App4Training({super.key});
 
+  /// One instance each for the app's lifetime: new ones on every rebuild
+  /// would make the Navigator detach and re-attach its observers each time
+  /// (and the limiter would lose track of the stack)
+  static final RouteStackLimiter _routeStackLimiter = RouteStackLimiter(
+    maxDepth: 5,
+  );
+  static final PerfNavigatorObserver _perfNavigatorObserver =
+      PerfNavigatorObserver();
+
   @override
   Widget build(BuildContext context, WidgetRef ref) {
     final AppLanguage appLanguage = ref.watch(appLanguageProvider);
@@ -201,6 +212,7 @@ class App4Training extends ConsumerWidget {
       themeMode: ThemeMode.system,
       initialRoute: '/',
       onGenerateRoute: (settings) => generateRoutes(settings),
+      navigatorObservers: [_routeStackLimiter, _perfNavigatorObserver],
       locale: appLanguage.locale,
       localizationsDelegates: AppLocalizations.localizationsDelegates,
       supportedLocales: AppLocalizations.supportedLocales,

lib/routes/route_stack_limiter.dart

@@ -0,0 +1,59 @@
+import 'dart:async';
+
+import 'package:app4training/features/perf/perf_logger.dart';
+import 'package:flutter/widgets.dart';
+
+/// Keeps the navigation stack from growing without bounds.
+///
+/// Every worksheet the user opens (drawer, language menu, links inside a
+/// worksheet) is pushed on top of the previous one, and each hidden route
+/// keeps its whole flutter_html tree alive - and rebuilds it whenever
+/// something it depends on changes (dark mode, app language, content
+/// updates). So once the stack is deeper than [maxDepth], the oldest route
+/// is dropped: the back button still works for the most recent screens.
+class RouteStackLimiter extends NavigatorObserver {
+  final int maxDepth;
+
+  /// The page routes currently on the stack, oldest first
+  final List<Route<dynamic>> _routes = [];
+
+  RouteStackLimiter({required this.maxDepth});
+
+  @override
+  void didPush(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    // Dialogs and popup menus are short-lived and keep no page alive
+    if (route is! PageRoute) return;
+    _routes.add(route);
+    if (_routes.length > maxDepth) {
+      final Route<dynamic> oldest = _routes.first;
+      // The navigator is locked while it notifies its observers
+      scheduleMicrotask(() {
+        if (!oldest.isActive) return;
+        navigator?.removeRoute(oldest);
+        // How often does that happen, i.e. users lose back history?
+        PerfLogger.event('stack.trimmed');
+      });
+    }
+  }
+
+  @override
+  void didPop(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    _routes.remove(route);
+  }
+
+  @override
+  void didRemove(Route<dynamic> route, Route<dynamic>? previousRoute) {
+    _routes.remove(route);
+  }
+
+  @override
+  void didReplace({Route<dynamic>? newRoute, Route<dynamic>? oldRoute}) {
+    final int index = oldRoute == null ? -1 : _routes.indexOf(oldRoute);
+    if (newRoute == null) return;
+    if (index == -1) {
+      _routes.add(newRoute);
+    } else {
+      _routes[index] = newRoute;
+    }
+  }
+}

lib/routes/view_page.dart

@@ -18,12 +18,28 @@ import 'package:app4training/widgets/language_selection_button.dart';
 
 /// The standard view of this app:
 /// Show a page (worksheet)
-class ViewPage extends ConsumerWidget {
+class ViewPage extends ConsumerStatefulWidget {
   final String page; // Name of the currently selected page
   final String langCode;
 
   const ViewPage(this.page, this.langCode, {super.key});
 
+  @override
+  ConsumerState<ViewPage> createState() => _ViewPageState();
+}
+
+class _ViewPageState extends ConsumerState<ViewPage> {
+  /// Result of [checkAndLoad], kept across builds.
+  ///
+  /// The route builds a new ViewPage whenever its dependencies change (e.g.
+  /// the theme on a dark mode switch, also while the page is hidden under
+  /// another screen). Starting a new load on each of these builds flashed the
+  /// loading spinner and rebuilt the whole flutter_html tree from scratch.
+  Future<String>? _content;
+
+  String get page => widget.page;
+  String get langCode => widget.langCode;
+
   /// First check whether the background process did something since
   /// the last time we checked.
   /// Then load the pageContent
@@ -39,22 +55,34 @@ class ViewPage extends ConsumerWidget {
             .watch(scaffoldMessengerProvider)
             .showSnackBar(SnackBar(content: Text(l10n.foundBgActivity)));
       }
-      return ref.watch(
+      // build() listens to this provider, which also keeps it alive
+      return ref.read(
         pageContentProvider((name: page, langCode: langCode)).future,
       );
     });
   }
 
   @override
-  Widget build(BuildContext context, WidgetRef ref) {
+  Widget build(BuildContext context) {
+    // ...but load again when a finished result (content or error) changes,
+    // e.g. the language got updated or downloaded while the page is open.
+    // Not on the first load: that one is still loading.
+    ref.listen(pageContentProvider((name: page, langCode: langCode)), (
+      previous,
+      next,
+    ) {
+      if ((previous != null) && !previous.isLoading) {
+        setState(() => _content = null);
+      }
+    });
     return Scaffold(
       appBar: AppBar(
         title: const Text(Globals.appTitle),
         actions: const [ShareButton(), LanguageSelectionButton()],
       ),
       drawer: MainDrawer(page, langCode),
       body: FutureBuilder(
-        future: checkAndLoad(context, ref),
+        future: _content ??= checkAndLoad(context, ref),
         builder: (BuildContext context, AsyncSnapshot<dynamic> snapshot) {
           if (kDebugMode) debugPrint(snapshot.connectionState.toString());
 

lib/widgets/html_view.dart

@@ -50,6 +50,25 @@ class _HtmlViewState extends State<HtmlView> {
         data: () => {'htmlBytes': widget.content.length},
       );
       _documentIsDarkMode = isDarkMode;
+      // Building and laying out the flutter_html tree for a new document is
+      // the expensive part (hundreds of ms on slow devices). Record the page
+      // structure (no identity) to tell which kind of content is to blame.
+      final htmldom.Document document = _document!;
+      // Routes covered by another screen get their tickers disabled by the
+      // Overlay. Reading the notifier (unlike TickerMode.valuesOf) adds no
+      // dependency, so this doesn't cause rebuilds of its own.
+      final bool hidden = !TickerMode.getValuesNotifier(context).value.enabled;
+      PerfLogger.spanToEndOfFrame(
+        'page.render',
+        data:
+            () => {
+              'hidden': hidden,
+              'tables': document.querySelectorAll('table').length,
+              'rows': document.querySelectorAll('tr').length,
+              'cells': document.querySelectorAll('td, th').length,
+              'images': document.querySelectorAll('img').length,
+            },
+      );
     }
     return _document!;
   }

test/html_view_perf_test.dart

@@ -0,0 +1,93 @@
+import 'package:app4training/features/perf/perf_logger.dart';
+import 'package:app4training/widgets/html_view.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import 'perf_logger_test.dart';
+
+void main() {
+  tearDown(PerfLogger.reset);
+
+  Widget htmlView(String body) => MaterialApp(
+    home: Scaffold(
+      body: HtmlView('<html><body>$body</body></html>', TextDirection.ltr),
+    ),
+  );
+
+  testWidgets('rendering a page records its structure in page.render', (
+    tester,
+  ) async {
+    final fileSystem = startLogger();
+
+    await tester.pumpWidget(
+      htmlView('''
+<p>Intro</p>
+<table>
+  <tr><th>A</th><th>B</th></tr>
+  <tr><td>1</td><td>2</td></tr>
+  <tr><td>3</td><td><img src="x.png"></td></tr>
+</table>
+'''),
+    );
+    await tester.pumpAndSettle();
+    await PerfLogger.flush();
+
+    final renders =
+        (await readRecords(
+          fileSystem,
+        )).where((r) => r['name'] == 'page.render').toList();
+    expect(renders, hasLength(1));
+    expect(renders.single['t'], 'span');
+    expect(renders.single['durUs'], isA<int>());
+    expect(renders.single['tables'], 1);
+    expect(renders.single['rows'], 3);
+    expect(renders.single['cells'], 6);
+    expect(renders.single['images'], 1);
+  });
+
+  testWidgets('rebuilds of the same page record no further page.render', (
+    tester,
+  ) async {
+    final fileSystem = startLogger();
+
+    await tester.pumpWidget(htmlView('<p>Same</p>'));
+    await tester.pumpAndSettle();
+    // An unrelated rebuild above HtmlView (e.g. a snackbar or the drawer)
+    await tester.pumpWidget(htmlView('<p>Same</p>'));
+    await tester.pumpAndSettle();
+    await PerfLogger.flush();
+
+    final records = await readRecords(fileSystem);
+    expect(records.where((r) => r['name'] == 'page.render'), hasLength(1));
+  });
+
+  testWidgets('re-renders of a page hidden under another screen are flagged', (
+    tester,
+  ) async {
+    final fileSystem = startLogger();
+    final navigatorKey = GlobalKey<NavigatorState>();
+    Widget app(Brightness brightness) => MaterialApp(
+      navigatorKey: navigatorKey,
+      theme: ThemeData(brightness: brightness),
+      home: const Scaffold(
+        body: HtmlView('<p>Worksheet</p>', TextDirection.ltr),
+      ),
+    );
+
+    await tester.pumpWidget(app(Brightness.light));
+    await tester.pumpAndSettle();
+    navigatorKey.currentState!.push(
+      MaterialPageRoute<void>(builder: (_) => const Text('Settings')),
+    );
+    await tester.pumpAndSettle();
+    // The system switches to dark mode while the worksheet is covered
+    await tester.pumpWidget(app(Brightness.dark));
+    await tester.pumpAndSettle();
+    await PerfLogger.flush();
+
+    final renders = (await readRecords(
+      fileSystem,
+    )).where((r) => r['name'] == 'page.render');
+    expect(renders.map((r) => r['hidden']), [false, true]);
+  });
+}