perf/fix: on-screen previews first, cache trim, text-search restart, abandoned-load leak

packages/pdfrx/lib/src/pdf_document_ref.dart

@@ -440,6 +440,12 @@ class PdfDocumentListenable extends Listenable {
         setError(err, stackTrace);
         return report?.copyWith(elapsedTime: stopwatch.elapsed);
       }
+      // Every listener left while the document was loading: this listenable was evicted, and the next resolve builds a
+      // new one. Nothing can reach the document anymore, so keeping it would leak it.
+      if (!identical(PdfDocumentRef._listenables[ref], this)) {
+        if (ref.autoDispose) await document.dispose();
+        return report?.copyWith(elapsedTime: stopwatch.elapsed);
+      }
       setDocument(document);
       return report?.copyWith(elapsedTime: stopwatch.elapsed);
     });

packages/pdfrx/lib/src/widgets/pdf_text_searcher.dart

@@ -83,6 +83,13 @@ class PdfTextSearcher extends Listenable {
     bool goToFirstMatch = true,
     bool searchImmediately = false,
   }) {
+    // Re-issuing the running pattern must not cancel it: search() below would return early for an identical pattern,
+    // leaving the matches half collected and isSearching stuck. Only drop a pending search for another pattern.
+    final last = _lastSearchCondition;
+    if (last != null && last.caseInsensitive == caseInsensitive && _isIdenticalPattern(last.pattern, pattern)) {
+      _searchTextTimer?.cancel();
+      return;
+    }
     _cancelTextSearch();
     final searchSession = ++_searchSession;
 
@@ -118,17 +125,15 @@ class PdfTextSearcher extends Listenable {
     _resetTextSearch(notify: false);
   }
 
-  void _resetTextSearch({bool notify = true, bool clearSearchCondition = true}) {
+  void _resetTextSearch({bool notify = true}) {
     _cancelTextSearch();
     _matches = const [];
     _matchesPageStartIndices = const [];
     _searchingPageNumber = null;
     _currentIndex = null;
     _currentMatch = null;
     _isSearching = false;
-    if (clearSearchCondition) {
-      _lastSearchCondition = null;
-    }
+    _lastSearchCondition = null;
     if (notify) {
       notifyListeners();
     }
@@ -145,6 +150,9 @@ class PdfTextSearcher extends Listenable {
       final textMatchesPageStartIndex = <int>[];
       var first = true;
       _isSearching = true;
+      // The previous pattern's position does not index into the new matches.
+      _currentIndex = null;
+      _currentMatch = null;
       _totalPageCount = document.pages.length;
       for (final page in document.pages) {
         _searchingPageNumber = page.pageNumber;
@@ -188,13 +196,15 @@ class PdfTextSearcher extends Listenable {
   }
 
   void _restartSearch() {
-    _resetTextSearch(clearSearchCondition: false);
+    final condition = _lastSearchCondition;
+    // Clearing the condition lets startTextSearch run the same pattern again.
+    _resetTextSearch();
     _cachedText.clear();
-    if (_lastSearchCondition != null) {
+    if (condition != null) {
       startTextSearch(
-        _lastSearchCondition!.pattern,
-        caseInsensitive: _lastSearchCondition!.caseInsensitive,
-        goToFirstMatch: _lastSearchCondition!.goToFirstMatch,
+        condition.pattern,
+        caseInsensitive: condition.caseInsensitive,
+        goToFirstMatch: condition.goToFirstMatch,
       );
     }
   }
@@ -244,6 +254,7 @@ class PdfTextSearcher extends Listenable {
     );
     controller?.setCurrentPageNumber(match.pageNumber);
     controller?.invalidate();
+    notifyListeners();
   }
 
   /// Get the matches range for the given page number.

packages/pdfrx/lib/src/widgets/pdf_viewer.dart

@@ -1661,6 +1661,7 @@ class _PdfViewerState extends State<PdfViewer>
     FilterQuality filterQuality = FilterQuality.high,
   }) {
     final unusedPageList = <int>[];
+    final previewRequests = <({PdfPage page, double scale, Rect rect})>[];
     final unmeasuredPageList = <int>[];
     // Pages inside the extent that are painting as a blank white rectangle
     // because no preview image has landed for them yet. This is the white-page
@@ -1749,7 +1750,7 @@ class _PdfViewerState extends State<PdfViewer>
 
       if (enableLowResolutionPagePreview &&
           (previewImage == null || previewImage.isDirty || previewImage.scale != previewScaleLimit)) {
-        _requestPagePreviewImageCached(cache, page, previewScaleLimit);
+        previewRequests.add((page: page, scale: previewScaleLimit, rect: rect));
       }
 
       final pageScale = page.width > 0 && page.height > 0
@@ -1797,20 +1798,28 @@ class _PdfViewerState extends State<PdfViewer>
           callback(canvas, rect, page);
         }
       }
+    }
 
-      if (unusedPageList.isNotEmpty) {
-        final currentPageNumber = _pageNumber;
-        if (currentPageNumber != null && currentPageNumber > 0) {
-          final currentPage = _document!.pages[currentPageNumber - 1];
-          cache.removeCacheImagesIfCacheBytesExceedsLimit(
-            unusedPageList,
-            maxImageCacheBytes,
-            currentPage,
-            dist: (pageNumber) =>
-                (_layout!.pageLayouts[pageNumber - 1].center - _layout!.pageLayouts[currentPage.pageNumber - 1].center)
-                    .distanceSquared,
-          );
-        }
+    // The preview lock serves requests in call order, and the loop above walks pages by index -- so the page above
+    // the viewport used to render before the one on screen. Ask for the visible pages first.
+    previewRequests.sort((a, b) => _distanceToRect(a.rect, targetRect).compareTo(_distanceToRect(b.rect, targetRect)));
+    for (final request in previewRequests) {
+      _requestPagePreviewImageCached(cache, request.page, request.scale);
+    }
+
+    // Once per paint, after the loop: inside it the list was still growing and the cache was trimmed once per page.
+    if (unusedPageList.isNotEmpty) {
+      final currentPageNumber = _pageNumber;
+      if (currentPageNumber != null && currentPageNumber > 0) {
+        final currentPage = _document!.pages[currentPageNumber - 1];
+        cache.removeCacheImagesIfCacheBytesExceedsLimit(
+          unusedPageList,
+          maxImageCacheBytes,
+          currentPage,
+          dist: (pageNumber) =>
+              (_layout!.pageLayouts[pageNumber - 1].center - _layout!.pageLayouts[currentPage.pageNumber - 1].center)
+                  .distanceSquared,
+        );
       }
     }
 
@@ -2024,6 +2033,13 @@ class _PdfViewerState extends State<PdfViewer>
 
   void _invalidate() => _updateStream.add(_txController.value);
 
+  /// 0 for a rect that overlaps [target], otherwise the gap between them.
+  static double _distanceToRect(Rect rect, Rect target) {
+    final dx = max(0.0, max(target.left - rect.right, rect.left - target.right));
+    final dy = max(0.0, max(target.top - rect.bottom, rect.top - target.bottom));
+    return dx * dx + dy * dy;
+  }
+
   Future<void> _requestPagePreviewImageCached(_PdfPageImageCache cache, PdfPage page, double scale) async {
     final width = page.width * scale;
     final height = page.height * scale;
@@ -4232,6 +4248,9 @@ class _PdfPageImageCache {
         pageImages.values.fold(0, (sum, e) => sum + getBytesConsumed(e.image)) +
         pageImagesPartial.values.fold(0, (sum, e) => sum + getBytesConsumed(e.image));
     for (final key in pageNumbers) {
+      if (bytesConsumed <= acceptableBytes) {
+        break;
+      }
       final removed = pageImages.remove(key);
       if (removed != null) {
         bytesConsumed -= getBytesConsumed(removed.image);
@@ -4242,9 +4261,6 @@ class _PdfPageImageCache {
         bytesConsumed -= getBytesConsumed(removedPartial.image);
         removedPartial.dispose();
       }
-      if (bytesConsumed <= acceptableBytes) {
-        break;
-      }
     }
   }
 }

packages/pdfrx/test/pdf_document_ref_abandoned_load_test.dart

@@ -0,0 +1,49 @@
+import 'dart:async';
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:pdfrx/pdfrx.dart';
+
+class _FakeDocument extends Fake implements PdfDocument {
+  bool disposed = false;
+
+  @override
+  Future<void> dispose() async => disposed = true;
+}
+
+void main() {
+  test('a load that finishes after every listener left disposes its document', () async {
+    final loading = Completer<PdfDocument>();
+    final ref = PdfDocumentRefByLoader((_) => loading.future, key: PdfDocumentRefKey('abandoned-load-test'));
+    final listenable = ref.resolveListenable();
+    void listener() {}
+    listenable.addListener(listener);
+    final load = listenable.load();
+
+    // e.g. the viewer is torn down to retry a slow load.
+    listenable.removeListener(listener);
+    final document = _FakeDocument();
+    loading.complete(document);
+    await load;
+
+    expect(document.disposed, isTrue);
+    expect(listenable.document, isNull);
+  });
+
+  test('a load with a listener still attached keeps its document', () async {
+    final loading = Completer<PdfDocument>();
+    final ref = PdfDocumentRefByLoader((_) => loading.future, key: PdfDocumentRefKey('kept-load-test'));
+    final listenable = ref.resolveListenable();
+    void listener() {}
+    listenable.addListener(listener);
+    final load = listenable.load();
+
+    final document = _FakeDocument();
+    loading.complete(document);
+    await load;
+
+    expect(document.disposed, isFalse);
+    expect(listenable.document, same(document));
+    listenable.removeListener(listener);
+    expect(document.disposed, isTrue);
+  });
+}

packages/pdfrx/test/pdf_text_searcher_test.dart

@@ -0,0 +1,143 @@
+import 'dart:async';
+
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:pdfrx/pdfrx.dart';
+
+class _FakePage extends Fake implements PdfPage {
+  _FakePage(this.pageNumber);
+
+  @override
+  final int pageNumber;
+}
+
+class _FakeDocument extends Fake implements PdfDocument {
+  _FakeDocument(int pageCount) : pages = [for (var i = 1; i <= pageCount; i++) _FakePage(i)];
+
+  @override
+  final List<PdfPage> pages;
+
+  @override
+  Stream<PdfDocumentEvent> get events => const Stream.empty();
+}
+
+class _FakeController extends PdfViewerController {
+  _FakeController(this.fakeDocument);
+
+  final _FakeDocument fakeDocument;
+  int currentPageSet = 0;
+
+  @override
+  bool get isReady => true;
+
+  @override
+  PdfDocument get document => fakeDocument;
+
+  @override
+  void invalidate() {}
+
+  @override
+  Rect calcRectForRectInsidePage({required int pageNumber, required PdfRect rect}) => Rect.zero;
+
+  @override
+  Future<void> ensureVisible(
+    Rect rect, {
+    Duration duration = const Duration(milliseconds: 200),
+    double margin = 0,
+  }) async {}
+
+  @override
+  void setCurrentPageNumber(int pageNumber) => currentPageSet = pageNumber;
+
+  @override
+  FutureOr<T?> useDocument<T>(
+    FutureOr<T> Function(PdfDocument document) task, {
+    bool ensureLoaded = true,
+    Completer<dynamic>? cancelLoading,
+  }) => task(fakeDocument);
+}
+
+/// Serves page text from [texts]; a page listed in [gates] waits for its completer.
+class _GatedSearcher extends PdfTextSearcher {
+  _GatedSearcher(super.controller, this.texts);
+
+  final Map<int, String> texts;
+  final Map<int, Completer<void>> gates = {};
+
+  @override
+  Future<PdfPageText?> loadText({required int pageNumber}) async {
+    await gates[pageNumber]?.future;
+    final text = texts[pageNumber] ?? '';
+    return PdfPageText(
+      pageNumber: pageNumber,
+      fullText: text,
+      charRects: [for (var i = 0; i < text.length; i++) PdfRect(i * 10.0, 10, i * 10.0 + 10, 0)],
+      fragments: const [],
+    );
+  }
+}
+
+void main() {
+  test('re-issuing the running pattern does not cut the search short', () async {
+    final searcher = _GatedSearcher(_FakeController(_FakeDocument(3)), {1: 'word', 2: 'word word', 3: 'word'});
+    addTearDown(searcher.dispose);
+    final gate = searcher.gates[2] = Completer<void>();
+
+    searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true);
+    await pumpEventQueue();
+    expect(searcher.isSearching, isTrue);
+    expect(searcher.matches, hasLength(1));
+
+    // e.g. a search field that fires again with the text unchanged.
+    searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true);
+    gate.complete();
+    await pumpEventQueue();
+
+    expect(searcher.matches.map((m) => m.pageNumber), [1, 2, 2, 3]);
+    expect(searcher.isSearching, isFalse);
+  });
+
+  test('a different pattern still replaces the running search', () async {
+    final searcher = _GatedSearcher(_FakeController(_FakeDocument(2)), {1: 'alpha beta', 2: 'beta'});
+    addTearDown(searcher.dispose);
+    final gate = searcher.gates[2] = Completer<void>();
+
+    searcher.startTextSearch('alpha', goToFirstMatch: false, searchImmediately: true);
+    await pumpEventQueue();
+    searcher.startTextSearch('beta', goToFirstMatch: false, searchImmediately: true);
+    gate.complete();
+    await pumpEventQueue();
+
+    expect(searcher.matches.map((m) => m.pageNumber), [1, 2]);
+    expect(searcher.isSearching, isFalse);
+  });
+
+  test('a new pattern clears the position left by the previous one', () async {
+    final searcher = _GatedSearcher(_FakeController(_FakeDocument(1)), {1: 'alpha alpha beta'});
+    addTearDown(searcher.dispose);
+    searcher.startTextSearch('alpha', goToFirstMatch: false, searchImmediately: true);
+    await pumpEventQueue();
+    await searcher.goToMatchOfIndex(1);
+
+    searcher.startTextSearch('beta', goToFirstMatch: false, searchImmediately: true);
+    await pumpEventQueue();
+
+    expect(searcher.matches, hasLength(1));
+    expect(searcher.currentIndex, isNull);
+  });
+
+  test('moving to a match notifies listeners, so a "3/120" counter can follow', () async {
+    final searcher = _GatedSearcher(_FakeController(_FakeDocument(1)), {1: 'word word'});
+    addTearDown(searcher.dispose);
+    searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true);
+    await pumpEventQueue();
+
+    var notified = 0;
+    searcher.addListener(() => notified++);
+    await searcher.goToMatchOfIndex(1);
+
+    expect(searcher.currentIndex, 1);
+    expect(notified, greaterThan(0));
+    expect((searcher.controller! as _FakeController).currentPageSet, 1);
+  });
+}