diff --git a/android/gradle.properties b/android/gradle.properties index bbd7acc..ba4f3b8 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,3 +1,5 @@ org.gradle.jvmargs=-Xmx1536M android.useAndroidX=true android.enableJetifier=false +android.builtInKotlin=false +android.newDsl=false diff --git a/docs/architecture.md b/docs/architecture.md index 124ad2c..97510f2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -83,6 +83,7 @@ which is mitigated by overriding `fileSystemProvider` and `httpClientProvider` i 1. **`StartupPage`** (`/`) - Decides where to navigate based on persisted state in `SharedPreferences`. See [onboarding-flow.md](onboarding-flow.md). + - Loads languages in stages so the spinner isn't blocked by all 34 of them: a cheap `lazyInit()` for every language, a full `init()` for the one or two the first screen renders, the rest in the background. See [routing.md](routing.md). 1. **`/view//`** @@ -101,13 +102,14 @@ ViewPage(page, langCode) └─ ref.watch(pageContentProvider((name, langCode))).future • watches Language(langCode) for the on-disk path • reads / from FileSystem - • inlines images by replacing - with + • loads all referenced images in parallel and inlines them: + • throws LanguageNotDownloadedException / PageNotFoundException / LanguageCorruptedException for the matching cases → HtmlView(content, direction) • sanitize(content, isDarkMode) ← workarounds for flutter_html bugs + (cached per content + brightness, so a rebuild doesn't redo it) • Html.fromDom(...) with TagWrapExtension({'table'}) + TableHtmlExtension; tables get wrapped in a horizontal scroll view • onAnchorTap pushes /view so internal worksheet links navigate @@ -125,18 +127,19 @@ DownloadLanguageButton(langCode).onPressed dio.get(htmlZipUrl, responseType: bytes), dio.get(pdfZipUrl, responseType: bytes), ]) ← github.com/4training/{html,pdf}-/archive/main.zip - – ZipDecoder.decodeBytes(...) → write each entry into .staging via FileSystem + – decode each zip in a worker isolate (max 2 at a time process-wide) + → write each entry into .staging via FileSystem – on any failure: rm -rf .staging and rethrow (prior data untouched) – rename assets- → assets-.old (if it existed) – rename .staging → assets- (atomic swap) – best-effort rm -rf .old • _load(): + – stat structure/contents.json → downloadTimestamp (UTC) – read structure/contents.json – build pages: Map, pageIndex: List, images: Map, pdf paths - – compute disk usage - – read modified timestamp of contents.json (UTC) → downloadTimestamp – emit new Language state + (disk usage is not part of it — see languageSizeProvider) → snackbar, button stops spinning ``` diff --git a/docs/content-rendering.md b/docs/content-rendering.md index 5b2c9e2..ae99268 100644 --- a/docs/content-rendering.md +++ b/docs/content-rendering.md @@ -11,7 +11,7 @@ ViewPage Padding → SingleChildScrollView → Column → SelectionArea → Directionality(LTR | RTL based on Globals.rtlLanguages) → Html.fromDom( - document: sanitize(content, isDarkMode), + document: _sanitizedDocument(isDarkMode), // cached sanitize() extensions: [TagWrapExtension({'table'}, …), TableHtmlExtension()], style: { body, td, th, h1, h2, h3, li, p, ul }, onAnchorTap: (url, _, __) => Navigator.pushNamed(context, '/view$url'), @@ -20,6 +20,8 @@ ViewPage ## `sanitize()` (`lib/widgets/html_view.dart`) +`HtmlView` is a `StatefulWidget` purely so it can cache the result of `sanitize()` for the current `(content, isDarkMode)` pair. Parsing the HTML and running the passes below over the whole document is expensive on a slow device, while `build()` runs again for all sorts of unrelated reasons (a snackbar, the drawer opening, an orientation change). `flutter_html` only re-reads the document when its dependencies change, so recomputing it on every build was pure waste. + The HTML emitted by the upstream `pywikitools` exporter has a few constructs that `flutter_html` can't render correctly. `sanitize()` runs a tree-rewrite step over the parsed DOM to fix them. Each rewrite is documented in-place, but here is the index: | Pattern in source HTML | What `sanitize()` does | Why | @@ -83,4 +85,4 @@ A single breadcrumb is emitted via `debugPrint` on the first suppression per app ## Image handling -Images are inlined as base64 by `pageContentProvider` *before* `HtmlView` ever sees the HTML. This avoids `flutter_html` having to load `file://`-prefixed URIs (which is fragile on Android/iOS). The downside is the rendered HTML can be large — for image-heavy worksheets like "God's Story (five fingers)" the inlined string can be hundreds of KB. So far this hasn't been a problem in practice. +Images are inlined as base64 by `pageContentProvider` *before* `HtmlView` ever sees the HTML. This avoids `flutter_html` having to load `file://`-prefixed URIs (which is fragile on Android/iOS). The downside is the rendered HTML can be large — for image-heavy worksheets like "God's Story (five fingers)" the inlined string can be hundreds of KB, and every image is base64-encoded here only to be decoded again by `flutter_html`. So far this hasn't been a problem in practice; `pageContentProvider` at least reads and encodes all images of a page in parallel rather than one at a time. Serving images through a custom image extension that reads file paths directly would remove the round-trip altogether if this ever does become a problem. diff --git a/docs/data-layer.md b/docs/data-layer.md index 9fac1a9..d8f4f7e 100644 --- a/docs/data-layer.md +++ b/docs/data-layer.md @@ -57,7 +57,6 @@ class Language { final List pageIndex; // menu order (subset of pages.keys) final Map images; // by filename final String path; // local path to html--main/ - final int sizeInKB; final DateTime downloadTimestamp; // UTC, taken from contents.json mtime bool get downloaded => languageCode != ''; } @@ -106,25 +105,29 @@ The branch name (`main`) is hardcoded; switching branches would require a code c - `await ref.read(languageDownloaderProvider).download(languageCode)`. The downloader handles the full atomic flow internally (see below); on success the on-disk directory at `pathFor(langCode)` is the new content. On failure it throws and the prior on-disk directory (if any) is left untouched. - The caller wraps in a `try/catch` to preserve the existing `Future` shape. 2. `_load()`: - - Recompute `assetsDirAlreadyExists()`. - - Sum file sizes recursively → `sizeInKB`. - - Read `contents.json` mtime → `downloadTimestamp` (UTC). + - A single `stat()` of `structure/contents.json` answers both "is this language on the device?" and "when was it stored there?" (`downloadTimestamp`, UTC). Not found → `return false`. - Parse `contents.json` worksheets, build `pages`, `pageIndex`. - Scan `pdf--main/` for `.pdf` files; match each `worksheet.pdf` → `Page.pdfPath`. - Scan `html--main/files/` for images; build `Map`. - - `_checkConsistency()` warns about HTML files referenced but missing, or HTML files present but unreferenced. + - `_checkConsistency()` warns about HTML files referenced but missing, or HTML files present but unreferenced. **Debug builds only** — it is another full directory listing and only produces log output. - On any throw: log, call `deleteResources()` (which delegates to `languageDownloader.delete(languageCode)`), reset to empty `Language`, return `false`. -`init()` calls `_load()` only — no network. `lazyInit()` only checks for `contents.json` existence and returns a sparse `Language(languageCode, {}, [], {}, path, 0, timestamp)` without parsing — used by the background isolate which doesn't need page details. Both `lazyInit()` and `_load()` read the path from `ref.read(languageDownloaderProvider).pathFor(languageCode)`. +`_load()` runs for every language on the UI isolate, so every call in it is asynchronous and it deliberately does *no* disk-usage accounting — see `languageSizeProvider` in [state-management.md](state-management.md). + +`init()` calls `_load()` only — no network. `lazyInit()` only checks for `contents.json` existence and returns a sparse `Language(languageCode, {}, [], {}, path, timestamp)` without parsing — used by the background isolate, and by `StartupPage` for all languages before the first frame. Both `lazyInit()` and `_load()` read the path from `ref.read(languageDownloaderProvider).pathFor(languageCode)`. ### Inside `LanguageDownloaderImpl` (`lib/data/language_downloader.dart`) The downloader owns the atomicity, concurrency, and crash-recovery guarantees so callers don't need to reason about partial state. One `download(langCode)` call performs: -1. **Serialize** against any in-flight download — at most one zip pair is held in memory at a time (concurrency cap; protects low-end devices from rapid taps on the per-language download buttons). +1. **Serialize** against any in-flight download of the *same* language (different languages still download in parallel; protects against rapid taps on the per-language download buttons). 2. **Crash recovery** — `rm -rf .staging` so a leftover from a prior crashed run never accumulates. 3. **Fetch concurrently** — `Future.wait` over two `dio.get(..., responseType: bytes)` calls for the HTML and PDF zips. -4. **Extract into staging** — `ZipDecoder().decodeBytes(...)` over each response, writing every `ArchiveFile` via the injected `FileSystem`. Not `extractArchiveToDisk` (it is tied to `dart:io` and not testable against `MemoryFileSystem`). +4. **Extract into staging** — `decodeZipEntries()` (a `ZipDecoder().decodeBytes(...)` pass) followed by writing every entry via the injected `FileSystem`. Not `extractArchiveToDisk` (it is tied to `dart:io` and not testable against `MemoryFileSystem`). + + Decoding is pure, synchronous CPU work that takes seconds per archive on a slow device, so by default it runs in a worker isolate (`decodeZipInIsolate`, injectable via the `zipDecoder` constructor parameter). The decoded entries are copied back to this isolate rather than written from inside the worker, which keeps *all* file access going through the injected `FileSystem`; copying a few MB is negligible next to the decoding. + + At most `kMaxParallelZipDecodes` (2) archives decode at the same time, process-wide. Onboarding downloads up to `kMaxParallelLanguageDownloads` (4) languages at once, each with two archives — letting all of those decode simultaneously would hold far too many decompressed archives in memory on a 4 GB device. 5. **Atomic swap** — rename existing `assets-` → `assets-.old` (if any), rename `.staging` → `assets-` (rename is atomic on a single filesystem), then best-effort `rm -rf .old`. 6. **On any throw mid-flight** — `rm -rf .staging` and `rethrow`. The prior on-disk directory (if any) is never touched until step 5, so a failed update never destroys offline content. @@ -149,7 +152,7 @@ FutureProvider.family((ref, page) async { … }, retry: null) Behaviour: - Reads `/` as a string. -- Replaces `` with `` by base64-encoding the local file via `imageContentProvider`. (Inlining is necessary because `flutter_html` can't load arbitrary local file URIs.) +- Collects every `` reference, loads all of them at once through `imageContentProvider` (`Future.wait`), then replaces the references with ``. (Inlining is necessary because `flutter_html` can't load arbitrary local file URIs.) Loading them up-front and in parallel keeps the blocking disk reads out of the render path — a worksheet like "God's Story (five fingers)" references five images. - Throws: - `LanguageNotDownloadedException(langCode)` if the language is gone. - `PageNotFoundException(name, langCode)` if the page isn't in `pages`. diff --git a/docs/features.md b/docs/features.md index 54eaf6e..4187f07 100644 --- a/docs/features.md +++ b/docs/features.md @@ -72,7 +72,7 @@ Header row has the four "all-languages" buttons: - `DownloadAllLanguagesButton` - `DeleteAllLanguagesButton` -Below the table: `diskUsage` total and a "X of Y languages" counter. +Below the table: `diskUsage` total (calculated asynchronously — a `…` placeholder is shown until it resolves) and a "X of Y languages" counter. ### Language buttons - **`DownloadLanguageButton`** (`ConsumerStatefulWidget`): icon with internal `_isLoading` flag — swaps to `CircularProgressIndicator` during `LanguageController.download()`. Optional `highlight` flag wraps it in a tinted rounded box (used during onboarding). diff --git a/docs/routing.md b/docs/routing.md index 16e2114..af84373 100644 --- a/docs/routing.md +++ b/docs/routing.md @@ -31,23 +31,40 @@ StartupPage.init(): if SharedPreferences['appLanguage'] is null: return '/onboarding/1' # first time - for each available language: - ref.read(languageProvider(code).notifier).init() # load disk state + # step 1: which languages are on the device? one stat() each, in parallel + await Future.wait(languageProvider(code).notifier.lazyInit() for all codes) if app language is not yet downloaded: return '/onboarding/2' # resume onboarding # (commented out for v0.9: third onboarding step on missing checkFrequency) - ref.read(backgroundSchedulerProvider.notifier).schedule() - if SharedPreferences['recentPage'] && 'recentLang' && language is downloaded: - return '/view//' # resume last worksheet - return '/home' + navigateTo = '/view//' # resume last worksheet + else: + navigateTo = '/home' + + # step 2: fully load only what the first screen renders + await Future.wait(languageProvider(code).notifier.init() + for code in {appLanguage, recentLang?}) + + # step 3: the remaining downloaded languages, unawaited, 3 at a time + unawaited(_loadRemainingLanguages(...)) + unawaited(backgroundSchedulerProvider.notifier.schedule()) + + return navigateTo ``` The first `await` in `init()` is what makes the loading spinner appear; once `init()` resolves, `Navigator.pushReplacementNamed` jumps to the chosen route, so the user never sees the home screen flash. +### Why the loading is staged + +Fully loading all 34 languages before the first frame is what made cold start feel broken on slow Android devices (see [in_progress_notes/investigation_cold_start.md](in_progress_notes/investigation_cold_start.md)): the cost grew linearly with the number of downloaded languages while the spinner sat frozen. + +Only the app language (for the menu) and the language of the resumed worksheet are needed before navigating; `lazyInit()` gives the *downloaded* flag for all the others, which is all the routing decision needs. Everything else is loaded afterwards — the widgets that use it (language selection menu, the drawer's translate icons) are driven by `languageProvider` and rebuild by themselves as languages arrive. + +Step 3 gets the `LanguageController`s handed to it rather than the `WidgetRef`: `StartupPage` is disposed by `pushReplacementNamed` while that work is still running, and a disposed `WidgetRef` must not be used. + ## Navigation primitives - **`Navigator.pushNamed`** for normal in-app navigation. diff --git a/docs/state-management.md b/docs/state-management.md index bb0036c..41ac456 100644 --- a/docs/state-management.md +++ b/docs/state-management.md @@ -47,21 +47,24 @@ This page is the index of every provider in the app — what it holds, what it d | Provider | Type | Purpose | | --- | --- | --- | | `fileSystemProvider` | `Provider` | `LocalFileSystem()` by default; overridden with `MemoryFileSystem` in tests | -| `imageContentProvider` | `Provider.family` | Returns base64-encoded PNG bytes for a given image. Used by `pageContentProvider` | +| `imageContentProvider` | `FutureProvider.family` | Returns base64-encoded PNG bytes for a given image. Used by `pageContentProvider` | | `pageContentProvider` | `FutureProvider.family` | The HTML body of a worksheet, with images inlined as base64. Throws `LanguageNotDownloadedException` / `PageNotFoundException` / `LanguageCorruptedException`. `retry: null` | -| `languageProvider` | `NotifierProvider.family` | Per-language state: pages, images, PDFs, disk path, size, download timestamp | +| `languageProvider` | `NotifierProvider.family` | Per-language state: pages, images, PDFs, disk path, download timestamp | | `countDownloadedLanguagesProvider` | `Provider` | Derived count for the settings page | -| `diskUsageProvider` | `Provider` | Sum of all `Language.sizeInKB` | +| `languageSizeProvider` | `FutureProvider.family` | Disk usage of one language in kB, walked on demand | +| `diskUsageProvider` | `FutureProvider` | Sum of all `languageSizeProvider`s | + +Disk usage is deliberately **not** part of `Language`: computing it means listing a language directory recursively and statting every file, which is far too expensive to do for every language at every cold start. Only the settings page asks for it, so it is its own provider and the `LanguagesTable` shows a placeholder while it resolves. `LanguageController` is the heart of the app. Methods: -- `init()`: idempotent load from disk. Call once per language at startup. -- `lazyInit()`: like `init()` but cheap — only sets `downloaded` + `path` + timestamp without parsing JSON. Used in the background isolate. +- `init()`: idempotent load from disk. Everything it does is asynchronous — it runs on the UI isolate, once per language. +- `lazyInit()`: like `init()` but cheap — a single `stat()` that sets `downloaded` + `path` + timestamp without parsing JSON. Used by the background isolate and by `StartupPage` for every language before the first frame (see [routing.md](routing.md)). - `download({force=false})`: clears (if forcing), downloads HTML+PDF zips, parses structure. - `deleteResources()`: clears assets dir, resets state. - `_load()`: the parser. Reads `structure/contents.json`, scans `pdf--main/` for PDF files, registers images in `files/`. Catches all errors and clears the assets dir on failure. `Language` (immutable data class): -- `languageCode`, `pages: Map`, `pageIndex: List` (menu order), `images`, `path`, `sizeInKB`, `downloadTimestamp` (always UTC). +- `languageCode`, `pages: Map`, `pageIndex: List` (menu order), `images`, `path`, `downloadTimestamp` (always UTC). - `downloaded` getter is `languageCode != ''`. - `getPageTitles()` returns the menu in order: English-name → translated-title. diff --git a/docs/testing.md b/docs/testing.md index 195e800..1524bbc 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -73,7 +73,7 @@ generates `test/full_coverage_test.dart` that imports every file under `lib/`. T - **`FakeLanguageDownloader`** — the single fake for the new `LanguageDownloader` interface. Implements `pathFor`, `isDownloaded`, `download`, `delete` against a configurable backing store, and exposes counters (e.g. `downloadCalls`) so tests can assert on call shape. Lives under `lib/` rather than `test/` because the integration test imports it via the production `background_task.dart` path — see [background-tasks.md](background-tasks.md). All download-related tests (`languages_test.dart`, `update_language_button_test.dart`, `download_language_button_test.dart`, `background_task_test.dart`, and the integration test) inject this fake via `languageDownloaderProvider.overrideWithValue(...)`. ### `test/languages_test.dart` -- **`TestLanguageController`** — overrides `LanguageController` to short-circuit `init`/`download` to a configured boolean. Used pervasively by widget tests that don't care about disk state. +- **`TestLanguageController`** — overrides `LanguageController` to short-circuit `init`/`lazyInit`/`download` to a configured boolean. Used pervasively by widget tests that don't care about disk state. - **Helper functions** to build `MemoryFileSystem` instances pre-populated with the German/English fixtures from `test/assets-de/`, `test/assets-en/`. ### `test/language_downloader_test.dart` @@ -83,8 +83,11 @@ Exercises `LanguageDownloaderImpl` directly against `MemoryFileSystem` + a mocke - **Corrupted-zip failure** — bytes fail to decode; same cleanup invariant. (Note: `archive`'s `ZipDecoder` silently returns an empty archive for pure garbage — to force a real decode error the test feeds bytes with a `PK` header followed by garbage.) - **Atomic update preserves prior data** — `pathFor(langCode)` is seeded with files; a failing download leaves the seed intact and readable. - **Concurrent calls serialized** — two `download()` calls fired without awaiting; the second only starts after the first completes (observable via dio mock ordering). +- **Zip decoding is capped** — three languages download at once, but a gated `zipDecoder` shows that never more than `kMaxParallelZipDecodes` archives decode simultaneously. - **Crash recovery** — a pre-seeded `.staging` directory is wiped by the next successful `download()`. +The production `zipDecoder` runs `Isolate.run` — the tests use it unchanged (isolates work fine in `flutter test`), and only the concurrency test injects its own to control timing. + ### `test/updates_test.dart` - **`TestLanguageStatus`** — overrides `LanguageStatusNotifier` for tests that don't want to mock HTTP. - **`mockCheckResponse({'de': 2, 'en': 0, ...})`** — builds an `http.Client` (via `mocktail`) that returns the right number of fake commits per language code. @@ -125,6 +128,8 @@ Almost every test: Uses a `TestObserver extends NavigatorObserver` to record `didPush` and `didReplace` calls. The asserts then check that, given a starting state, the right `pushReplacementNamed` was invoked. This is the cleanest way to verify `StartupPage`'s decision matrix end-to-end. +`startup_page_test.dart` additionally pins the staged loading described in [routing.md](routing.md): a `GatedLanguageController` holds each `init()` open until the test releases it, so the test can assert that navigation happens once the app language and the recent page's language are loaded — while another downloaded language is still loading in the background. + ## Integration test `integration_test/background_interaction_test.dart` is the only test that runs on a real Android emulator. CI runs it via `reactivecircus/android-emulator-runner@v2` at API level 29 with `arch: x86_64`. Locally: diff --git a/lib/data/language_downloader.dart b/lib/data/language_downloader.dart index 9861eb6..4f26e96 100644 --- a/lib/data/language_downloader.dart +++ b/lib/data/language_downloader.dart @@ -1,4 +1,6 @@ import 'dart:async'; +import 'dart:collection'; +import 'dart:isolate'; import 'dart:typed_data'; import 'package:archive/archive.dart'; @@ -7,6 +9,45 @@ import 'package:dio/dio.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; +/// One entry of a decoded zip archive: a file together with its contents, +/// or - when [bytes] is null - a directory that has to exist even if it +/// ends up empty (e.g. the files/ dir of a language without images). +typedef ArchiveEntry = ({String path, Uint8List? bytes}); + +/// Decodes a zip archive into a flat list of [ArchiveEntry]s +typedef ZipDecoderFn = Future> Function(Uint8List zipBytes); + +/// How many zip archives may be decoded at the same time. +/// +/// Onboarding downloads up to kMaxParallelLanguageDownloads languages at +/// once, each of them with an HTML and a PDF archive. Decoding all of those +/// simultaneously would hold several decompressed archives in memory at the +/// same time - too much to ask of a 4 GB device. +const int kMaxParallelZipDecodes = 2; + +final _zipDecodeLimit = _Semaphore(kMaxParallelZipDecodes); + +/// Decode a zip archive. This is pure, synchronous CPU work: on a slow +/// device it takes seconds per archive, which is why [decodeZipInIsolate] +/// (the default of [LanguageDownloaderImpl]) keeps it away from the UI. +List decodeZipEntries(Uint8List zipBytes) { + final archive = ZipDecoder().decodeBytes(zipBytes); + return [ + for (final file in archive) + (path: file.name, bytes: file.isFile ? file.content : null) + ]; +} + +/// Run [decodeZipEntries] in a short-lived worker isolate, so that +/// downloading a language doesn't freeze every frame while it is unpacked. +/// +/// The decoded contents are copied back to this isolate (instead of being +/// written to disk inside the worker) so that all file access keeps going +/// through the injected [FileSystem] and stays testable. Copying a few MB +/// is negligible next to the decoding itself. +Future> decodeZipInIsolate(Uint8List zipBytes) => + Isolate.run(() => decodeZipEntries(zipBytes)); + abstract interface class LanguageDownloader { String pathFor(String langCode); Future isDownloaded(String langCode); @@ -18,15 +59,18 @@ class LanguageDownloaderImpl implements LanguageDownloader { final String _root; final Dio _dio; final FileSystem _fileSystem; + final ZipDecoderFn _decodeZip; final Map> _inFlightByLang = {}; LanguageDownloaderImpl({ required String root, required Dio dio, required FileSystem fileSystem, + ZipDecoderFn? zipDecoder, }) : _root = root, _dio = dio, - _fileSystem = fileSystem; + _fileSystem = fileSystem, + _decodeZip = zipDecoder ?? decodeZipInIsolate; @override String pathFor(String langCode) => @@ -70,18 +114,7 @@ class LanguageDownloaderImpl implements LanguageDownloader { // Extract both zips into staging for (final response in results) { - final bytes = Uint8List.fromList(response.data!); - final archive = ZipDecoder().decodeBytes(bytes); - for (final file in archive) { - final filePath = p.join(staging, file.name); - if (file.isFile) { - final outFile = _fileSystem.file(filePath); - await outFile.parent.create(recursive: true); - await outFile.writeAsBytes(file.content as List); - } else { - await _fileSystem.directory(filePath).create(recursive: true); - } - } + await _extractInto(staging, response.data!); } // Atomic swap @@ -112,6 +145,33 @@ class LanguageDownloaderImpl implements LanguageDownloader { } } + /// Unpack the zip archive in [zipData] into the [staging] directory + Future _extractInto(String staging, List zipData) async { + // dio hands us a Uint8List already - don't pay for a second copy of a + // multi-megabyte buffer just to satisfy the type + final bytes = zipData is Uint8List ? zipData : Uint8List.fromList(zipData); + final entries = await _zipDecodeLimit.run(() => _decodeZip(bytes)); + + // An archive holds hundreds of files in a handful of directories, so + // remember which ones we created instead of asking for each file again + final createdDirs = {}; + for (final entry in entries) { + final entryPath = p.join(staging, entry.path); + final bytes = entry.bytes; + if (bytes == null) { + if (createdDirs.add(entryPath)) { + await _fileSystem.directory(entryPath).create(recursive: true); + } + continue; + } + final outFile = _fileSystem.file(entryPath); + if (createdDirs.add(outFile.parent.path)) { + await outFile.parent.create(recursive: true); + } + await outFile.writeAsBytes(bytes); + } + } + @override Future delete(String langCode) async { final dir = _fileSystem.directory(pathFor(langCode)); @@ -120,3 +180,30 @@ class LanguageDownloaderImpl implements LanguageDownloader { } } } + +/// Lets at most [_permits] operations run at the same time; the rest queue up +class _Semaphore { + _Semaphore(this._permits); + + int _permits; + final Queue> _waiting = Queue>(); + + Future run(Future Function() action) async { + if (_permits > 0) { + _permits--; + } else { + final completer = Completer(); + _waiting.add(completer); + await completer.future; // the permit is handed over to us directly + } + try { + return await action(); + } finally { + if (_waiting.isEmpty) { + _permits++; + } else { + _waiting.removeFirst().complete(); + } + } + } +} diff --git a/lib/data/languages.dart b/lib/data/languages.dart index c076e51..441a4ce 100644 --- a/lib/data/languages.dart +++ b/lib/data/languages.dart @@ -2,7 +2,7 @@ import 'dart:collection'; import 'dart:convert'; import 'package:app4training/data/exceptions.dart'; import 'package:file/local.dart'; -import 'package:flutter/material.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:app4training/data/globals.dart'; import 'package:file/file.dart'; @@ -18,9 +18,13 @@ final fileSystemProvider = Provider((ref) { /// Unique identifier of an image or a page typedef Resource = ({String name, String langCode}); +/// Which images does a page reference? +final _imageReference = RegExp(r'src="files/([^.]+.png)"'); + /// Provide image data (base64-encoded) /// Returns empty string in case something went wrong -final imageContentProvider = Provider.family((ref, res) { +final imageContentProvider = + FutureProvider.family((ref, res) async { final String path = ref.watch(languageProvider(res.langCode)).path; if (path == '') { debugPrint( @@ -30,13 +34,13 @@ final imageContentProvider = Provider.family((ref, res) { final fileSystem = ref.watch(fileSystemProvider); try { File image = fileSystem.file(join(path, 'files', res.name)); - debugPrint('Successfully loaded ${res.name}'); - return base64Encode(image.readAsBytesSync()); + if (kDebugMode) debugPrint('Successfully loaded ${res.name}'); + return base64Encode(await image.readAsBytes()); } on FileSystemException catch (e) { debugPrint("Couldn't load ${res.name}: $e"); return ''; } -}); +}, retry: null); /// Provide HTML content of a specific page in a specific language /// throws [LanguageNotDownloadedException]: just download the language again @@ -63,18 +67,31 @@ final pageContentProvider = String content = await fileSystem .file(join(lang.path, pageDetails.fileName)) .readAsString(); + + // Read and encode all images of this page at once: doing that one by one + // while building the HTML string meant a series of blocking disk reads + // right before the first frame of a page could be painted. + final Map imageData = {}; + await Future.wait(_imageReference + .allMatches(content) + .map((match) => match.group(1)!) + .where(lang.images.containsKey) + .toSet() + .map((name) async { + imageData[name] = await ref.watch( + imageContentProvider((name: name, langCode: page.langCode)).future); + })); + // Load images directly into the HTML: // Replace with - return content.replaceAllMapped(RegExp(r'src="files/([^.]+.png)"'), - (match) { - if (!lang.images.containsKey(match.group(1))) { + return content.replaceAllMapped(_imageReference, (match) { + final String name = match.group(1)!; + if (!imageData.containsKey(name)) { debugPrint( - 'Warning: image ${match.group(1)} missing (in ${pageDetails.fileName})'); + 'Warning: image $name missing (in ${pageDetails.fileName})'); return match.group(0)!; } - String imageData = ref.watch(imageContentProvider( - (name: match.group(1)!, langCode: page.langCode))); - return 'src="data:image/png;base64,$imageData"'; + return 'src="data:image/png;base64,${imageData[name]}"'; }); } on FileSystemException catch (e) { throw LanguageCorruptedException( @@ -113,8 +130,7 @@ class LanguageController extends Notifier { // system). This is needed for overrideWith() where the arg isn't passed // through the constructor. languageCode = ref.$arg as String; - return Language( - '', const {}, const [], const {}, '', 0, DateTime.utc(2023)); + return Language('', const {}, const [], const {}, '', DateTime.utc(2023)); } /// Download this language and make it available. @@ -141,13 +157,15 @@ class LanguageController extends Notifier { .watch(fileSystemProvider) .stat(join(path, 'structure', 'contents.json')); bool downloaded = (stat.type != FileSystemEntityType.notFound); - debugPrint( - "QuickInit trying to load '$languageCode', downloaded: $downloaded"); + if (kDebugMode) { + debugPrint( + "QuickInit trying to load '$languageCode', downloaded: $downloaded"); + } if (downloaded) { DateTime timestamp = stat.modified.toUtc(); // Always store UTC internally - state = Language( - languageCode, const {}, const [], const {}, path, 0, timestamp); + state = + Language(languageCode, const {}, const [], const {}, path, timestamp); return true; } return false; @@ -156,34 +174,37 @@ class LanguageController extends Notifier { /// Load our Language structure from the file system resources. /// Returns whether everything went well and the language is available now. /// This method shouldn't throw + /// + /// Everything in here must stay asynchronous and cheap: this runs for every + /// language at every cold start, on the UI isolate. Deliberately *not* done + /// here: computing the disk usage (see [languageSizeProvider]) and, outside + /// of debug builds, the consistency check. Future _load() async { final downloader = ref.read(languageDownloaderProvider); final fileSystem = ref.watch(fileSystemProvider); try { // Now we store the full path to the language - String path = join( - downloader.pathFor(languageCode), Globals.getResourcesDir(languageCode)); - debugPrint("Path: $path"); - - bool downloaded = await downloader.isDownloaded(languageCode); - debugPrint("Trying to load '$languageCode', downloaded: $downloaded"); - if (!downloaded) return false; - - // Store the size of the downloaded files (HTML + PDF) - int sizeInKB = await _calculateMemoryUsage( - fileSystem.directory(downloader.pathFor(languageCode))); + String path = join(downloader.pathFor(languageCode), + Globals.getResourcesDir(languageCode)); - // Get the timestamp: When were our contents stored on the device? + // One stat() answers both questions we have about contents.json: + // is the language on the device at all, and when was it stored there? FileStat stat = await fileSystem.stat(join(path, 'structure', 'contents.json')); + bool downloaded = (stat.type != FileSystemEntityType.notFound); + if (kDebugMode) { + debugPrint("Trying to load '$languageCode' from $path," + " downloaded: $downloaded"); + } + if (!downloaded) return false; DateTime timestamp = stat.modified.toUtc(); // Always store UTC internally // Read structure/contents.json as our source of truth: // Which pages are available, what is the order in the menu - var structure = jsonDecode(fileSystem + var structure = jsonDecode(await fileSystem .file(join(path, 'structure', 'contents.json')) - .readAsStringSync()); + .readAsString()); final Map pages = {}; final List pageIndex = []; @@ -222,7 +243,9 @@ class LanguageController extends Notifier { if (pdfFiles.isNotEmpty) { debugPrint('Found unexpected PDF file(s): $pdfFiles'); } - await _checkConsistency(fileSystem.directory(path), pages); + if (kDebugMode) { + await _checkConsistency(fileSystem.directory(path), pages); + } // Register available images var filesDir = fileSystem.directory(join(path, 'files')); @@ -236,8 +259,8 @@ class LanguageController extends Notifier { } } } - state = Language( - languageCode, pages, pageIndex, images, path, sizeInKB, timestamp); + state = + Language(languageCode, pages, pageIndex, images, path, timestamp); return true; } catch (e) { String msg = 'Error initializing data structure: $e'; @@ -245,7 +268,7 @@ class LanguageController extends Notifier { // Delete the whole folder await downloader.delete(languageCode); state = - Language('', const {}, const [], const {}, '', 0, DateTime.utc(2023)); + Language('', const {}, const [], const {}, '', DateTime.utc(2023)); return false; } } @@ -254,7 +277,7 @@ class LanguageController extends Notifier { Future deleteResources() async { await ref.read(languageDownloaderProvider).delete(languageCode); state = - Language('', const {}, const [], const {}, '', 0, DateTime.utc(2023)); + Language('', const {}, const [], const {}, '', DateTime.utc(2023)); } /// Download all files for one language via [LanguageDownloader] @@ -271,16 +294,6 @@ class LanguageController extends Notifier { return true; } - /// Return the total size of all files in our directory in kB - Future _calculateMemoryUsage(Directory dir) async { - var entities = await dir.list(recursive: true).toList(); - var sizeInBytes = entities.fold(0, (int sum, entity) { - if (entity is File) return sum + entity.statSync().size; - return sum; - }); - return (sizeInBytes / 1000).ceil(); // let's never round down - } - /// Check whether all files mentioned in structure/contents.json are present /// and whether there is no extra file present /// @@ -355,14 +368,11 @@ class Language { /// local path to the directory holding all content final String path; - /// The size of the downloaded directory (kB = kilobytes) - final int sizeInKB; - /// When were the files downloaded on our device? file system attribute, UTC final DateTime downloadTimestamp; const Language(this.languageCode, this.pages, this.pageIndex, this.images, - this.path, this.sizeInKB, this.downloadTimestamp); + this.path, this.downloadTimestamp); /// Returns an list with all the worksheet titles in the menu. /// The list is ordered as identifier -> translated title @@ -377,18 +387,44 @@ class Language { @override String toString() { return 'Language $languageCode. Downloaded: $downloaded' - ' ($downloadTimestamp), size: $sizeInKB, local path: $path,' + ' ($downloadTimestamp), local path: $path,' ' #pages: ${pages.length}, #images: ${images.length}'; } } -/// Provide combined disk usage of all languages (in KB) -final diskUsageProvider = Provider((ref) { - int sizeInKB = 0; - for (String langCode in ref.watch(availableLanguagesProvider)) { - Language lang = ref.watch(languageProvider(langCode)); - assert(lang.downloaded || (lang.sizeInKB == 0)); - sizeInKB += lang.sizeInKB; - } - return sizeInKB; +/// Provide the disk usage of one language (in kB) +/// +/// This is computed on demand and not while loading a language: it means +/// walking the whole language directory and statting every single file +/// (HTML worksheets, images and PDFs alike), which is far too expensive to +/// do for every language at every cold start. Only the settings page needs it. +final languageSizeProvider = + FutureProvider.family((ref, langCode) async { + if (!ref.watch(languageProvider(langCode)).downloaded) return 0; + final downloader = ref.watch(languageDownloaderProvider); + final dir = + ref.watch(fileSystemProvider).directory(downloader.pathFor(langCode)); + return calculateMemoryUsage(dir); +}); + +/// Provide combined disk usage of all languages (in kB) +final diskUsageProvider = FutureProvider((ref) async { + final List sizes = await Future.wait(>[ + for (String langCode in ref.watch(availableLanguagesProvider)) + ref.watch(languageSizeProvider(langCode).future) + ]); + return sizes.fold(0, (int sum, int size) => sum + size); }); + +/// Return the total size of all files below [dir] in kB +/// +/// Uses the asynchronous stat() so the hundreds of syscalls this needs are +/// handled by the IO thread pool instead of blocking the UI isolate. +Future calculateMemoryUsage(Directory dir) async { + if (!await dir.exists()) return 0; + final entities = await dir.list(recursive: true, followLinks: false).toList(); + final stats = + await Future.wait(entities.whereType().map((file) => file.stat())); + final sizeInBytes = stats.fold(0, (int sum, FileStat s) => sum + s.size); + return (sizeInBytes / 1000).ceil(); // let's never round down +} diff --git a/lib/data/updates.dart b/lib/data/updates.dart index 2420016..0f09bab 100644 --- a/lib/data/updates.dart +++ b/lib/data/updates.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:app4training/background/background_scheduler.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:app4training/data/globals.dart'; @@ -187,7 +188,7 @@ class LanguageStatusNotifier extends Notifier { } final status = LanguageStatus(updatesAvailable, dlTimestamp, lcTimestamp); - debugPrint('Language $_languageCode: $status'); + if (kDebugMode) debugPrint('Language $_languageCode: $status'); return status; } @@ -284,6 +285,6 @@ final lastCheckedProvider = Provider((ref) { return DateTime.utc(2023); } assert(timestamp.isUtc); - debugPrint('Last checked for updates: $timestamp'); + if (kDebugMode) debugPrint('Last checked for updates: $timestamp'); return timestamp; }); diff --git a/lib/main.dart b/lib/main.dart index 18f2c23..c9d0d62 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:app4training/data/language_downloader.dart'; import 'package:app4training/l10n/generated/app_localizations.dart'; import 'package:dio/dio.dart'; @@ -93,20 +95,7 @@ void _installHtmlTableSemanticsFilter() { final FlutterExceptionHandler? previousHandler = FlutterError.onError; var suppressedBreadcrumbEmitted = false; FlutterError.onError = (FlutterErrorDetails details) { - final String exceptionText = details.exception.toString(); - final String stackText = details.stack?.toString() ?? ''; - final bool exceptionMatchesKnownSignature = - exceptionText.contains('RenderBox was not laid out') || - exceptionText.contains('computeDryBaseline') || - exceptionText.contains('renderBoxDoingDryBaseline') || - exceptionText.contains("'child!.hasSize'"); - final bool stackMatchesHtmlOrSemanticsPath = - stackText.contains('flutter_html/') || - stackText.contains('flutter_layout_grid/') || - stackText.contains('flushSemantics'); - final bool isKnownHtmlTableAssertion = - exceptionMatchesKnownSignature && stackMatchesHtmlOrSemanticsPath; - if (isKnownHtmlTableAssertion) { + if (_isKnownHtmlTableAssertion(details)) { if (!suppressedBreadcrumbEmitted) { suppressedBreadcrumbEmitted = true; debugPrint( @@ -126,12 +115,37 @@ void _installHtmlTableSemanticsFilter() { }; } +/// Does [details] match one of the four known, non-fatal +/// `flutter_html_table` assertions? See [_installHtmlTableSemanticsFilter] +/// for the matching strategy. +bool _isKnownHtmlTableAssertion(FlutterErrorDetails details) { + final String exceptionText = details.exception.toString(); + final bool exceptionMatchesKnownSignature = + exceptionText.contains('RenderBox was not laid out') || + exceptionText.contains('computeDryBaseline') || + exceptionText.contains('renderBoxDoingDryBaseline') || + exceptionText.contains("'child!.hasSize'"); + // Stringifying the stack is by far the expensive half of this check, and + // these assertions fire hundreds of times per page load in debug/profile + // builds - so only pay for it once the cheap message check has matched. + if (!exceptionMatchesKnownSignature) return false; + final String stackText = details.stack?.toString() ?? ''; + return stackText.contains('flutter_html/') || + stackText.contains('flutter_layout_grid/') || + stackText.contains('flushSemantics'); +} + void main() async { WidgetsFlutterBinding.ensureInitialized(); _installHtmlTableSemanticsFilter(); - final prefs = await SharedPreferences.getInstance(); - final packageInfo = await PackageInfo.fromPlatform(); - final appDocsDir = await getApplicationDocumentsDirectory(); + // None of these three depend on each other, so don't pay for three + // sequential platform channel round trips - the native splash screen is up + // for all of it, without a single Flutter frame rendered yet. + final (prefs, packageInfo, appDocsDir) = await ( + SharedPreferences.getInstance(), + PackageInfo.fromPlatform(), + getApplicationDocumentsDirectory(), + ).wait; final languageDownloader = LanguageDownloaderImpl( root: appDocsDir.path, dio: Dio(), diff --git a/lib/routes/startup_page.dart b/lib/routes/startup_page.dart index 84f22a9..6547224 100644 --- a/lib/routes/startup_page.dart +++ b/lib/routes/startup_page.dart @@ -1,7 +1,10 @@ import 'dart:async'; +import 'dart:collection'; + import 'package:app4training/background/background_scheduler.dart'; import 'package:app4training/data/app_language.dart'; import 'package:app4training/routes/error_page.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:app4training/widgets/loading_animation.dart'; @@ -9,6 +12,9 @@ import 'package:app4training/widgets/loading_animation.dart'; import '../data/globals.dart'; import '../data/languages.dart'; +/// How many languages to load in parallel after the first screen is up +const int _maxParallelLanguageLoads = 3; + /// Handle the initial route "/": Show a loading indicator /// while we're initializing the data in the background. /// In case the user is new: Lead him to the onboarding / resume onboarding @@ -18,22 +24,36 @@ class StartupPage extends ConsumerWidget { const StartupPage({super.key, this.initFunction}); /// Initialize and return the route where to continue now + /// + /// This runs while the user is staring at the loading spinner, so it does + /// as little work as possible before it can hand over to the first real + /// screen (see docs/in_progress_notes/investigation_cold_start.md): + /// + /// 1. [LanguageController.lazyInit] for every language - one stat() each, + /// which is all we need to decide where to navigate to. + /// 2. A full [LanguageController.init] for the one or two languages the + /// first screen actually renders. + /// 3. The remaining downloaded languages are loaded afterwards, in the + /// background: nothing on the first screen depends on them, and the + /// widgets that do (language selection, drawer translation icons) + /// rebuild by themselves once a language arrives. Future init(WidgetRef ref) async { if (ref.read(sharedPrefsProvider).getString('appLanguage') == null) { // First app usage: Let's start onboarding return '/onboarding/1'; } - // Read downloaded languages from the device - for (String languageCode in ref.read(availableLanguagesProvider)) { - await ref.read(languageProvider(languageCode).notifier).init(); - // TODO: look at return value and show snackBar when there was an error - } + // Step 1: Which languages are on the device? + final List availableLanguages = + ref.read(availableLanguagesProvider); + await Future.wait([ + for (String languageCode in availableLanguages) + ref.read(languageProvider(languageCode).notifier).lazyInit() + ]); // Check whether app language is downloaded - if (!ref - .read(languageProvider(ref.read(appLanguageProvider).languageCode)) - .downloaded) { + final String appLangCode = ref.read(appLanguageProvider).languageCode; + if (!ref.read(languageProvider(appLangCode)).downloaded) { return '/onboarding/2'; // Go to DownloadLanguagesPage } @@ -43,21 +63,60 @@ class StartupPage extends ConsumerWidget { return '/onboarding/3'; }*/ - // Start the periodic background task - unawaited(ref.read(backgroundSchedulerProvider.notifier).schedule()); - // Go to recently opened page or to /home String navigateTo = '/home'; String page = ref.read(sharedPrefsProvider).getString('recentPage') ?? ''; String lang = ref.read(sharedPrefsProvider).getString('recentLang') ?? ''; - if ((page != '') && + final bool resumeRecentPage = (page != '') && (lang != '') && - ref.read(languageProvider(lang)).downloaded) { - navigateTo = '/view/$page/$lang'; - } + ref.read(languageProvider(lang)).downloaded; + if (resumeRecentPage) navigateTo = '/view/$page/$lang'; + + // Step 2: Load what the first screen needs - the app language for the menu + // and, if we resume a recent page, the language that page is written in. + final Set neededNow = {appLangCode, if (resumeRecentPage) lang}; + await Future.wait([ + for (String languageCode in neededNow) + ref.read(languageProvider(languageCode).notifier).init() + // TODO: look at return value and show snackBar when there was an error + ]); + + // Step 3: Everything else may take its time. We hand over the controllers + // rather than the WidgetRef: this page is disposed as soon as we navigate + // away, and a disposed WidgetRef must not be used any more. + unawaited(_loadRemainingLanguages([ + for (String languageCode in availableLanguages) + if (!neededNow.contains(languageCode) && + ref.read(languageProvider(languageCode)).downloaded) + ref.read(languageProvider(languageCode).notifier) + ])); + + // Start the periodic background task + unawaited(ref.read(backgroundSchedulerProvider.notifier).schedule()); + return navigateTo; } + /// Fully load the languages behind [controllers], a few at a time. + /// + /// Runs after the first screen is on its way, with a small concurrency + /// limit so we don't flood the IO queue of a slow device while it is still + /// busy rendering that screen. + Future _loadRemainingLanguages( + List controllers) async { + final pending = Queue.of(controllers); + + Future worker() async { + while (pending.isNotEmpty) { + await pending.removeFirst().init(); + } + } + + await Future.wait([ + for (var i = 0; i < _maxParallelLanguageLoads; i++) worker() + ]); + } + @override Widget build(BuildContext context, WidgetRef ref) { // When we're finished with loading: Go to the recently opened page @@ -72,7 +131,7 @@ class StartupPage extends ConsumerWidget { ), initialData: "Loading", builder: (BuildContext context, AsyncSnapshot snapshot) { - debugPrint(snapshot.connectionState.toString()); + if (kDebugMode) debugPrint(snapshot.connectionState.toString()); switch (snapshot.connectionState) { case ConnectionState.none: @@ -80,9 +139,10 @@ class StartupPage extends ConsumerWidget { case ConnectionState.active: return loadingAnimation('Loading'); case ConnectionState.done: - debugPrint( - 'Done, hasData: ${snapshot.hasData}, Error: ${snapshot.hasError}', - ); + if (kDebugMode) { + debugPrint('Done, hasData: ${snapshot.hasData},' + ' Error: ${snapshot.hasError}'); + } if (snapshot.hasError) { // TODO do something more helpful for the user ("try again...") return ErrorPage(snapshot.error.toString()); diff --git a/lib/routes/view_page.dart b/lib/routes/view_page.dart index 378d635..f29a779 100644 --- a/lib/routes/view_page.dart +++ b/lib/routes/view_page.dart @@ -6,6 +6,7 @@ import 'package:app4training/l10n/l10n.dart'; import 'package:app4training/widgets/error_message.dart'; import 'package:app4training/widgets/html_view.dart'; import 'package:app4training/features/share/share_button.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:riverpod/misc.dart' show ProviderException; @@ -30,7 +31,7 @@ class ViewPage extends ConsumerWidget { AppLocalizations l10n = context.l10n; final foundActivity = await ref.read(backgroundResultProvider.notifier).checkForActivity(); - debugPrint("backgroundActivity: $foundActivity"); + if (kDebugMode) debugPrint("backgroundActivity: $foundActivity"); if (foundActivity) { ref .watch(scaffoldMessengerProvider) @@ -51,7 +52,7 @@ class ViewPage extends ConsumerWidget { body: FutureBuilder( future: checkAndLoad(context, ref), builder: (BuildContext context, AsyncSnapshot snapshot) { - debugPrint(snapshot.connectionState.toString()); + if (kDebugMode) debugPrint(snapshot.connectionState.toString()); switch (snapshot.connectionState) { case ConnectionState.none: @@ -59,8 +60,10 @@ class ViewPage extends ConsumerWidget { case ConnectionState.active: return loadingAnimation("Loading content..."); case ConnectionState.done: - debugPrint( - 'Done, hasData: ${snapshot.hasData}, Error: ${snapshot.hasError}'); + if (kDebugMode) { + debugPrint('Done, hasData: ${snapshot.hasData},' + ' Error: ${snapshot.hasError}'); + } if (snapshot.hasError) { // In Riverpod v3, provider errors are wrapped in // ProviderException - unwrap to get the original error diff --git a/lib/widgets/html_view.dart b/lib/widgets/html_view.dart index b34faf5..75b76b0 100644 --- a/lib/widgets/html_view.dart +++ b/lib/widgets/html_view.dart @@ -1,4 +1,5 @@ import 'package:app4training/widgets/invertible_image_builtin.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_html/flutter_html.dart'; import 'package:flutter_html_table/flutter_html_table.dart'; @@ -7,7 +8,7 @@ import 'package:html/dom.dart' as htmldom; /// Scrollable display of HTML content, filling most of the screen. /// Uses the flutter_html package. -class HtmlView extends StatelessWidget { +class HtmlView extends StatefulWidget { /// HTML code to display final String content; @@ -16,6 +17,36 @@ class HtmlView extends StatelessWidget { const HtmlView(this.content, this.direction, {super.key}); + @override + State createState() => _HtmlViewState(); +} + +class _HtmlViewState extends State { + /// Result of [sanitize] for the current content and brightness. + /// + /// Parsing the HTML and running the ~10 full-document passes of [sanitize] + /// over it costs a noticeable amount of time on a slow device, while + /// build() runs again for all sorts of unrelated reasons (a snackbar, the + /// drawer opening, an orientation change). flutter_html only looks at the + /// document again when its dependencies change, so recomputing it on every + /// build was pure waste. + htmldom.Document? _document; + bool _documentIsDarkMode = false; + + @override + void didUpdateWidget(HtmlView oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.content != widget.content) _document = null; + } + + htmldom.Document _sanitizedDocument(bool isDarkMode) { + if (_document == null || _documentIsDarkMode != isDarkMode) { + _document = sanitize(widget.content, isDarkMode); + _documentIsDarkMode = isDarkMode; + } + return _document!; + } + @override Widget build(BuildContext context) { final bool isDarkMode = Theme.of(context).brightness == Brightness.dark; @@ -26,7 +57,7 @@ class HtmlView extends StatelessWidget { children: [ SelectionArea( child: Directionality( - textDirection: direction, + textDirection: widget.direction, // child: Html( // data: content, child: Html.fromDom( @@ -34,7 +65,7 @@ class HtmlView extends StatelessWidget { // is the canonical dark-mode source for this widget: // the sanitize pass and the image inversion must flip // together with the widget colors. - document: sanitize(content, isDarkMode), + document: _sanitizedDocument(isDarkMode), extensions: [ // Order matters: TagWrapExtension must come BEFORE // TableHtmlExtension so it matches first @@ -176,14 +207,18 @@ htmldom.Document sanitize(String inputHtml, bool isDarkMode) { // FIXME: That could actually be fixed in the HTML generated by pywikitools for (var element in dom.querySelectorAll('td div')) { if (element.attributes['class'] == 'mw-translate-fuzzy') { - debugPrint('Found fuzzy translated content. Removing
tag...'); + if (kDebugMode) { + debugPrint('Found fuzzy translated content. Removing
tag...'); + } element.parent!.innerHtml = element.innerHtml; element.remove(); } } for (var element in dom.querySelectorAll('p span')) { if (element.attributes['class'] == 'mw-translate-fuzzy') { - debugPrint('Found fuzzy translated content. Removing tag...'); + if (kDebugMode) { + debugPrint('Found fuzzy translated content. Removing tag...'); + } element.parent!.innerHtml = element.innerHtml; element.remove(); } @@ -192,14 +227,18 @@ htmldom.Document sanitize(String inputHtml, bool isDarkMode) { // Change
to // FIXME: That could actually be fixed in the HTML generated by pywikitools for (var element in dom.querySelectorAll('td p')) { - debugPrint('Warning: Found

element in

to // FIXME: That could actually be fixed in the HTML generated by pywikitools for (var element in dom.querySelectorAll('th p')) { - debugPrint('Warning: Found

element in

// FIXME: Remove once the issue 1188 (see above) is solved for (var element in dom.querySelectorAll('td ul')) { - debugPrint('Warning: Found
    element in

Content

Content, removing...'); + if (kDebugMode) { + debugPrint('Warning: Found

element in

, removing...'); + } element.parent!.innerHtml = element.innerHtml; element.remove(); } // Change

Content

Content, removing...'); + if (kDebugMode) { + debugPrint('Warning: Found

element in

, removing...'); + } element.parent!.innerHtml = element.innerHtml; element.remove(); } @@ -208,7 +247,9 @@ htmldom.Document sanitize(String inputHtml, bool isDarkMode) { // • item1
• item2
: Working around the bug'); + if (kDebugMode) { + debugPrint('Warning: Found
    element in
: Working around the bug'); + } String newHtml = ''; for (var li in element.children) { assert(li.localName == 'li'); diff --git a/lib/widgets/languages_table.dart b/lib/widgets/languages_table.dart index be4dbba..6db05d6 100644 --- a/lib/widgets/languages_table.dart +++ b/lib/widgets/languages_table.dart @@ -22,7 +22,9 @@ class LanguagesTable extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { - int sizeInKB = ref.watch(diskUsageProvider); + // Disk usage is calculated on demand (it walks all language directories), + // so it may still be pending on the first build of this page + final AsyncValue diskUsage = ref.watch(diskUsageProvider); String countLanguages = context.l10n .countLanguages(ref.watch(countDownloadedLanguagesProvider)); @@ -118,7 +120,8 @@ class LanguagesTable extends ConsumerWidget { ))), const SizedBox(height: 5), Text( - '${context.l10n.diskUsage}: $sizeInKB kB $countLanguages', + '${context.l10n.diskUsage}: ${diskUsage.value ?? '…'} kB' + ' $countLanguages', style: Theme.of(context).textTheme.bodyMedium, ), ], diff --git a/test/language_downloader_test.dart b/test/language_downloader_test.dart index 2b449da..331cb32 100644 --- a/test/language_downloader_test.dart +++ b/test/language_downloader_test.dart @@ -1,5 +1,6 @@ -import 'dart:typed_data'; import 'dart:async'; +import 'dart:math'; +import 'dart:typed_data'; import 'package:app4training/data/globals.dart'; import 'package:app4training/data/language_downloader.dart'; @@ -263,6 +264,57 @@ void main() { expect(htmlCallCount, 2); }); + test('Only kMaxParallelZipDecodes archives are decoded at the same time', + () async { + // Every decode blocks on a gate so we can watch how many run at once + var inFlight = 0; + var maxInFlight = 0; + final gates = >[]; + Future> gatedDecoder(Uint8List zipBytes) async { + inFlight++; + maxInFlight = max(maxInFlight, inFlight); + final gate = Completer(); + gates.add(gate); + await gate.future; + inFlight--; + return decodeZipEntries(zipBytes); + } + + downloader = LanguageDownloaderImpl( + root: root, dio: dio, fileSystem: fs, zipDecoder: gatedDecoder); + + const langCodes = ['de', 'fr', 'es']; + for (final langCode in langCodes) { + mockDioGet(dio, Globals.getRemoteUrlHtml(langCode), + createTestZip({'${Globals.getResourcesDir('de')}/f.html': 'x'})); + mockDioGet(dio, Globals.getRemoteUrlPdf(langCode), + createTestZip({'${Globals.getPdfDir('de')}/f.pdf': 'x'})); + } + + final downloads = [for (final code in langCodes) downloader.download(code)]; + var finished = false; + unawaited(Future.wait(downloads).then((_) => finished = true)); + + // Let all three downloads reach their first decode + for (var i = 0; i < 5; i++) { + await Future.delayed(Duration.zero); + } + expect(inFlight, kMaxParallelZipDecodes); + expect(gates.length, kMaxParallelZipDecodes); + + // Now let them through one by one - a queued decode may only start + // once a running one has finished + for (var i = 0; !finished && i < 100; i++) { + if (gates.isNotEmpty) gates.removeAt(0).complete(); + await Future.delayed(Duration.zero); + } + expect(finished, isTrue); + expect(maxInFlight, kMaxParallelZipDecodes); + for (final langCode in langCodes) { + expect(await downloader.isDownloaded(langCode), true); + } + }); + test('Crash recovery: pre-seeded staging dir is wiped by next download', () async { // Simulate crashed prior run leaving a staging dir diff --git a/test/languages_test.dart b/test/languages_test.dart index 632d5d8..100a6d9 100644 --- a/test/languages_test.dart +++ b/test/languages_test.dart @@ -21,16 +21,13 @@ import 'package:riverpod/src/framework.dart' show $RefArg; /// For other behavior set downloadedLanguages to [] or a set of languages. class TestLanguageController extends LanguageController { final List? _downloadedLanguages; - final int _languageSize; // size in KB final Map _pages; // map of pages that are available final bool _initReturns; TestLanguageController( {List? downloadedLanguages, - int languageSize = 0, Map pages = const {}, initReturns = false}) : _downloadedLanguages = downloadedLanguages, - _languageSize = languageSize, _pages = pages, _initReturns = initReturns; @@ -45,26 +42,32 @@ class TestLanguageController extends LanguageController { downloaded = _downloadedLanguages.contains(languageCode); } return Language(downloaded ? languageCode : '', _pages, const [], const {}, - '', _languageSize, DateTime.utc(2023)); + '', DateTime.utc(2023)); } @override Future download() async { state = Language(languageCode, _pages, const [], const {}, '', - _languageSize, DateTime.now().toUtc()); + DateTime.now().toUtc()); return true; } @override Future deleteResources() async { - state = - Language('', const {}, const [], const {}, '', 0, DateTime.utc(2023)); + state = Language('', const {}, const [], const {}, '', DateTime.utc(2023)); } @override Future init() async { return _initReturns; } + + /// The state built in [build] already says whether we're downloaded, + /// so there is nothing to look up on a (non-existing) file system. + @override + Future lazyInit() async { + return state.downloaded; + } } /// Create a test file system which simulates that the specified languages @@ -259,7 +262,6 @@ void main() { 'Schritte der Vergebung', 'MissingTest' ])); - expect(deTest.state.sizeInKB, 147); expect(deTest.state.path, equals('assets-de/html-de-main')); // Test some error handling @@ -287,12 +289,27 @@ void main() { }); }); - test('Test diskUsageProvider', () { + test('Test languageSizeProvider and diskUsageProvider', () async { + final fileSystem = + ChrootFileSystem(const LocalFileSystem(), path.canonicalize('test/')); final ref = ProviderContainer(overrides: [ - languageProvider - .overrideWith2((langCode) => TestLanguageController(languageSize: 42)), + fileSystemProvider.overrideWith((ref) => fileSystem), + languageDownloaderProvider + .overrideWithValue(FakeLanguageDownloader(fileSystem: fileSystem)), ]); - expect(ref.read(diskUsageProvider), countAvailableLanguages * 42); + + // Sizes are only calculated for languages that are actually loaded + expect(await ref.read(languageSizeProvider('de').future), 0); + expect(await ref.read(languageProvider('de').notifier).init(), true); + expect(await ref.read(languageSizeProvider('de').future), 147); + + // German is the only language in test/, so it makes up the whole usage + expect(await ref.read(diskUsageProvider.future), 147); + }); + + test('Test calculateMemoryUsage on a missing directory', () async { + final fileSystem = MemoryFileSystem(); + expect(await calculateMemoryUsage(fileSystem.directory('nothing-here')), 0); }); test('Test countDownloadedLanguagesProvider', () { diff --git a/test/main_drawer_test.dart b/test/main_drawer_test.dart index e62febd..f9ce8f9 100644 --- a/test/main_drawer_test.dart +++ b/test/main_drawer_test.dart @@ -40,7 +40,7 @@ class CustomTestLanguageController extends LanguageController { pages[page] = Page(page, title, 'test', '1.0', null); pageIndex.add(page); } - return Language(arg, pages, pageIndex, const {}, '', 0, DateTime.utc(2023)); + return Language(arg, pages, pageIndex, const {}, '', DateTime.utc(2023)); } } diff --git a/test/settings_page_test.dart b/test/settings_page_test.dart index 97a2c2d..8f4ec55 100644 --- a/test/settings_page_test.dart +++ b/test/settings_page_test.dart @@ -81,13 +81,15 @@ void main() { overrides: [ appLanguageProvider.overrideWith(() => TestAppLanguage('en')), languageProvider.overrideWith2( - (languageCode) => TestLanguageController(languageSize: 42), + (languageCode) => TestLanguageController(), ), + languageSizeProvider.overrideWith((ref, languageCode) => 42), sharedPrefsProvider.overrideWith((ref) => prefs), ], child: const TestSettingsPage(), ), ); + await tester.pump(); // disk usage is calculated asynchronously int expectedSize = 42 * countAvailableLanguages; expect(find.textContaining('$expectedSize kB'), findsOneWidget); // language counter visibility basic test diff --git a/test/startup_page_test.dart b/test/startup_page_test.dart index 6fb2547..cc5ba78 100644 --- a/test/startup_page_test.dart +++ b/test/startup_page_test.dart @@ -13,6 +13,24 @@ import 'package:shared_preferences/shared_preferences.dart'; import 'background_scheduler_test.dart'; import 'languages_test.dart'; +/// Records which languages a full init() was requested for and lets the +/// test decide when each of them finishes loading +class GatedLanguageController extends TestLanguageController { + GatedLanguageController(this.initCalls, this.gates, + {super.downloadedLanguages}) + : super(initReturns: true); + + final List initCalls; + final Map> gates; + + @override + Future init() async { + initCalls.add(languageCode); + await gates[languageCode]!.future; + return super.init(); + } +} + void main() { // Mocking the globalInit() function: // We want to be able to test all the different outcomes of the future @@ -115,6 +133,67 @@ void main() { }); */ + testWidgets('Only the languages of the first screen delay the navigation', ( + WidgetTester tester, + ) async { + SharedPreferences.setMockInitialValues({ + 'appLanguage': 'en', + 'checkFrequency': 'weekly', + 'recentPage': 'Healing', + 'recentLang': 'de', + }); + final prefs = await SharedPreferences.getInstance(); + route = null; + final initCalls = []; + final gates = { + for (final languageCode in ['en', 'de', 'fr']) + languageCode: Completer() + }; + final ref = ProviderContainer( + overrides: [ + languageProvider.overrideWith2( + (languageCode) => GatedLanguageController( + initCalls, + gates, + downloadedLanguages: ['en', 'de', 'fr'], + ), + ), + backgroundSchedulerProvider.overrideWith( + () => TestBackgroundScheduler(), + ), + sharedPrefsProvider.overrideWithValue(prefs), + ], + ); + await tester.pumpWidget( + UncontrolledProviderScope( + container: ref, + child: MaterialApp( + home: const StartupPage(), + onGenerateRoute: generateRoutes, + ), + ), + ); + + // Only the app language and the language of the recent page are loaded + // before we can leave the loading screen - not all 34 languages + await tester.pump(); + expect(initCalls.toSet(), equals({'en', 'de'})); + expect(route, isNull); + + // As soon as those two are there we navigate - even though the other + // downloaded language is still being loaded in the background + gates['en']!.complete(); + gates['de']!.complete(); + await tester.pump(); + expect(route, equals('/view/Healing/de')); + expect(initCalls.toSet(), equals({'en', 'de', 'fr'})); + + // Languages that aren't on the device are never fully loaded + gates['fr']!.complete(); + await tester.pumpAndSettle(); + expect(initCalls.length, 3); + }); + testWidgets('Test failing initFunction', (WidgetTester tester) async { SharedPreferences.setMockInitialValues({'appLanguage': 'de'}); final prefs = await SharedPreferences.getInstance();