Dissolve the loader chain - #583
Merged
Merged
Conversation
With one loader left there is no chain to drive, so the machinery built for four goes and the work it was carrying moves to components named after it. DocumentLoader replaces LoaderService, the FileLoader base and LoaderServiceQueue. It is a ViewModel scoped to MainActivity: it survives a configuration change and is there before anything asks it for a document, so the queue that held work until the binder arrived has nothing left to hold. Under it, FileCache stores the working copy, FileIdentifier names and types it, CoreLoader renders it and DocumentSaver writes it back - the last two lifted out of MetadataLoader and LoaderService unchanged. The mutable Options bag was the baton one loader passed to the next, so it splits: DocumentRequest is what the user asked for, IdentifiedFile the cached copy it turned out to be, LoadedDocument the two plus the parts to show. fileExists goes with it - holding an IdentifiedFile is the answer - along with LoaderType and the already dead Options.limit. A bundle written by the shipped version can name FileLoader$Result, and Bundle reads its whole map at the first access, so the restore in DocumentFragment is guarded rather than each retired name being mapped. Losing the reopened document beats throwing on launch, and DocumentParcelTest covers the round trip that nothing else did: the recreation test restores from the view model, which never parcels. An empty file now reports back with no IdentifiedFile, so "Open With" offers the uri the user picked rather than a zero byte copy of it. Nothing else changes for the user: a render sweep over 60 documents matches the previous build on every one, signal, text and all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018y4srZKc2SQa7JRYcDbTcc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With one loader left there is no chain to drive. The machinery built for four goes, and the work it was carrying moves to components named after it.
DocumentLoaderis aViewModelscoped toMainActivity: it survives a configuration change and is there before anything asks it for a document, so the queue that held work until the binder arrived has nothing left to hold. The<service>element goes with it.The mutable
Optionsbag was the baton one loader passed to the next, so it splits:DocumentRequestis what the user asked for,IdentifiedFilethe cached copy it turned out to be,LoadedDocumentthe two plus the parts to show.fileExistsgoes with it — holding anIdentifiedFileis the answer — along withLoaderTypeand the already deadOptions.limit.Net −717 lines.
Worth a look in review
FileLoader$Result, andBundlereads its whole map at the first access, so one stale entry takes the rest with it.DocumentFragment.restoreis guarded rather than each retired name being mapped — losing the reopened document beats throwing on launch. This is what retiresLoaderType.ofParcelledand its test.reloadis notload. Entering or leaving edit mode, and retrying with a password, render the copy already in the cache. Sending them throughloadwould re-read and re-identify the file and move it up the recent list on every toggle.Odr.identify()loads the native library; that is not work for the thread putting the first frame up. Everything reaching the core is posted to the background thread anyway, so FIFO keeps the ordering.IdentifiedFile, so "Open With" offers the uri the user picked rather than a zero byte copy of it.loader_success_*/loader_error_*named a stage that no longer exists and fired alongsideload_success/load_error; they collapse into those.load_odf_error_expectedgoes — that check is now theonUnsupportedsplit itself. The screen name loses its constantCORE_prefix.Verification
DocumentParcelTestis new and covers the parcel round trip nothing else did —testDocumentSurvivesRecreationrestores from the view model, which never parcels.tools/render-sweepover 60 corpus documents, diffed against a build ofmain: identical on every document — same signal, same text node count, same liveness.Not reachable by hand on an emulator: the process-death restore.
am killplus a launcher relaunch gives a fresh task with a nullsavedInstanceStatein both builds, so the parcel test and the guard stand in for it.Unrelated, left alone:
tools/render-sweep/render-sweep.shstill explains at length that it never taps anything because the app might offer to upload a document. That offer went away in #581.🤖 Generated with Claude Code