fix(lcp): Marketplace LCP-wrapped PDFs fail to open — port recursive predicate (PP-4454)#1008
Merged
Merged
Conversation
…ace open (PP-4454) Marketplace OPDS 2.0 JSON feeds wrap LCP PDFs as `opds-publication+json → [LCP license → application/pdf]` with the LCP MIME nested one level deep in `indirectAcquisitions`. `LCPPDFs.canOpenBook` only inspected `defaultAcquisition.type`, so it returned false for Marketplace- shape PDFs (e.g. Family Matters by Gregory C. Elliott from the ticket). `BookFileManager.pathExtension` then routed the file to `.epub` instead of `.zip`, breaking the LCP extract pass. Patrons saw "an error message" instead of the PDF rendering. Direct structural mirror of PR #958 / 3.0.3 hotfix `ca2ff13b6` (PP-4407) which closed the same regression class for audiobooks. THE FIX: (1) LCPPDFs.hasLCPAcquisition (new @objc static) — recursive predicate over the full acquisition chain (top-level type + nested `indirectAcquisitions[*].type`) looking for the LCP license MIME, gated by `defaultBookContentType == .pdf` so LCP-typed audiobooks/EPUBs don't false-match. (2) LCPPDFs.indirectChainContainsLCP (new private helper) — recursive walker, identical structure to LCPAudiobooks's equivalent. (3) BookFileManager.pathExtension — swap both LCP branches from `canOpenBook` to `hasLCPAcquisition`. The PDF side is the PP-4454 fix; the audiobook side completes the forward-port that PR #958 made on its hotfix branch but never landed on develop. New downloads of Marketplace LCP PDFs now save with `.zip` extension directly. TESTS: New LCPPDFAcquisitionPredicateTests (4 tests), direct structural mirror of LCPAcquisitionPredicateTests. The Marketplace-shape test asserts `canOpenBook=false AND hasLCPAcquisition=true` on the same fixture — the divergence is the deterministic proof the recursive predicate is doing real work, not duplicating canOpenBook. 21/21 LCP suites pass green (4 new + 4 audiobook + 13 LCPPDFs). ForgeOS changeset: cs_8e01deae (init_e7603d4e — current initiative). **Scope:** 2 production files (LCPPDFs.swift +35 / BookFileManager.swift +12 -2) + 1 test file (LCPPDFAcquisitionPredicateTests.swift +167) + 1 pbxproj entry. ~55 production LOC. **Not done:** End-to-end simdrive repro against the exact ticket-target Family Matters book. The book has 0 copies available on A1QA Test Library (Place Hold only). Repro evidence on Desktop/PP-4454 shows the book detail page reached + A1QA's Palace Marketplace lane confirmed reachable. The unit-test fixture (`testHasLCPAcquisition_nestedLCPInIndirectChain_returnsTrue`) encodes the exact OPDS shape from the bug report and asserts both halves deterministically — that is the load-bearing evidence. **Deferred:** The LCP PDF rendering wiring is broken at a deeper layer than this PR fixes. `TPPPDFDocument.init(encryptedData:decryptor:)` exists but is never called on develop — `BookService.presentPDF` and `BookCellModel.didSelectRead` pass the on-disk URL to `TPPPDFDocument(url:)` (the plain PDFKit path), and `LCPFulfillmentHandler.fulfillLCPLicense` writes the extract to a temp URL no caller reads. The acquisition-layer fix is necessary but not sufficient for Family Matters to actually open in the reader. The proper architectural fix lives on `feature/readium-pdf-navigator-migration` (commit 9d042ab — Readium PDFNavigator); that branch is 373 commits behind develop and needs adaptation. Tracked separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🧪 Test Coverage (via QAAtlas)
🏛️ Architecture Analysis
✅ Architecture looks healthy! 🔍 Reachability Analysis
✅ No dead code detected 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
🧪 Unit Test Results📊 View Full Interactive Report ✅ ALL TESTS PASSED6693 tests | 6600 passed | 0 failed | 93 skipped | ⏱️ 10m 58s | 📊 98.6% | 📈 46.7% coverage Tests by Class — 755 classes (click to expand)
📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 46.7% Total coverage incl. UI/lifecycle: 44.9% (17 files excluded from testable denominator — see
📈 TrendsTest count change: -5 🔗 Interactive HTML Report | CI Run Details 📦 Downloadable Artifacts
|
Contributor
Author
|
Live sim repro captured. Power Rangers Unlimited: Edge of Darkness on A1QA Test Library reproduces the bug deterministically.
Palace error log at failure (from That's Critical cross-check confirming the bug is acquisition-shape-specific: Power Rangers #2 (a different LCP comic in the same A1QA library) opens fine with full PDF reader chrome ("Page 1 of 28"). Same LCP system, same library, same reader code path — only the OPDS acquisition shape differs. That's the deterministic signature of an OPDS-layer predicate bug, exactly mirroring how PP-4407 manifested for audiobooks. Evidence bundle on local desktop (
🤖 Generated with Claude Code |
Cherry-picked from feature/readium-pdf-navigator-migration commit 9d042ab (orphan branch, never merged). Adapted to current develop (~373 commits of drift): the migration's `.shared` singletons are translated to AppContainer DI, `MyBooksDownloadCenter.fulfillLCPLicense` keeps its Phase 7 `LCPFulfillmentHandler` extraction, and the no-op-after-migration PDF extract pass is dropped from `LCPFulfillmentHandler`. ROOT CAUSE this addresses (PP-4454, validated against the David Wilcox / Mark Raynsford / Jonathan Green Slack thread today): The home-grown LCP-PDF read flow on develop never worked for real LCP- protected PDFs: download LCP zip → try manifest.json lookup → extract inner PDF to temp dir → PDFKit mmap the temp file PROBLEMS: - The manifest lookup throws fileNotFound on every real Palace Marketplace archive (`LCPPDFs.getPdfHref` → ReadiumShared.AccessError .fileSystem.fileNotFound), so the temp extract silently failed via `_ = try? await LCPPDFs(url:)?.extract(...)` and the user saw "Unable to load PDF file." - Even when extraction worked, the inner PDF bytes were still LCP- encrypted — Archive.extract just unzips raw bytes with no decryption. PDFKit then saw encrypted noise and refused to render. - Temp + archive + PDFKit mapping meant ~3× disk footprint — on large LCP textbooks iOS would purge the temp extract under memory pressure. Confirmed live on iPhone 16 Pro sim against A1QA Test Library: - Power Rangers Unlimited: Edge of Darkness (LCP-wrapped, 829MB) → "Unable to load PDF file" + matching log (ReadiumShared.AccessError.fileSystem.fileNotFound from LCPPDFs.getPdfHref) - Power Rangers #2 (open-access PDF, no LCP) → opens fine - Bug is acquisition-shape-specific: only LCP-wrapped PDFs fail. FIX: Adopt Readium 3's `PDFNavigatorViewController`, which streams decrypted pages on demand via the shared `GCDHTTPServer` that the EPUB reader already uses. Decryption happens inside Readium's resource layer through the LCP content protection; no temp extract, no duplicated bytes, no manifest-lookup dance. Changes (after adaptation): Added - Palace/PDF/ReadiumPDF/ReadiumPDFViewController.swift — UIKit container around PDFNavigatorViewController with a legacy-page restore shim that reads the 0-indexed pageNumber from TPPBookRegistry and resolves it to a Locator via positionsByReadingOrder, so existing users don't lose their place. - Palace/PDF/ReadiumPDF/ReadiumPDFReaderView.swift — SwiftUI host that bridges Readium's locationDidChange(locator:) back into the existing TPPPDFDocumentMetadata.currentPage contract so bookmarks, reading-position sync, and TOC UI keep working unchanged. - ReaderService.openPDF(_:) — entry point that calls into TPPR3Owner.libraryService.openBook(...) and pushes the Readium PDF route onto the NavigationCoordinator (parallel to openEPUB). - NavigationCoordinator.storeReadiumPDF / resolveReadiumPDF — route storage for Readium-backed PDF publications. - NavigationHostView .pdf case — branches on coordinator.resolveReadiumPDF (Readium path) before falling back to the legacy TPPPDFReaderView (open-access PDFs). Removed (now dead under the new pipeline) - LCPPDFs.swift home-grown machinery: extract(url:), temporaryUrlForPDF(url:), deletePdfContent(url:), getPdfHref(), decryptData/decryptRawData/dataCache, the publicationOpener init. LCPPDFs.swift is now a 75-line shell with two static predicates: canOpenBook (legacy /loans/ XML shape) and hasLCPAcquisition (recursive walker for Marketplace /groups/ JSON shape — PP-4454 initial scope, retained for defense in depth even though the extension-routing it fixes is now less load-bearing under the Readium pipeline). - TPPEncryptedPDFDataProvider, TPPEncryptedPDFDocument, TPPEncryptedPDFView, TPPEncryptedPDFViewController, TPPEncryptedPDFViewer, TPPEncryptedPDFPageViewController, TPPPDFViewController — the CGPDF-based encrypted-page-by-page rendering path. Unused after PDFNavigator takes over. - LCPFulfillmentHandler PDF extract pass — Readium decrypts on demand, no eager extract needed. - LocalBookContentService LCPPDFs.deletePdfContent call — no temp file to clean up post-migration. Modified - BookService.presentPDF and BookCellModel.didSelectRead: route LCP PDFs through ReaderService.openPDF before falling back to the open-access PDFKit path. AppContainer DI (current convention), not .shared. - TPPPDFDocument: simplified — only the non-encrypted PDFKit path remains (open-access PDFs). - TPPPDFReaderView: minor binding tweak for the LCP-route case. - pbxproj: new ReadiumPDF group + dropped obsolete encrypted-PDF files from both Sources phases. **Scope:** 14 production files modified, 6 deleted (Encrypted* + TPPPDFViewController), 2 added (ReadiumPDF/*). Net: +356 / -1117 LOC. **Not done:** End-to-end sim verify against Edge of Darkness with the migrated pipeline — that's the next step and is what motivated cherry- picking this. Will follow up in a subsequent commit / PR comment with sim evidence. **Deferred:** Snapshot tests for the new ReadiumPDFReaderView / ReadiumPDFViewController. The legacy TPPPDF* snapshot tests will naturally become stale (some assert on the obsolete EncryptedPDF path); those should be removed in a follow-up cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…memory + loading indicator Builds on the Readium PDFNavigator migration (cac4e7b) with the end-to-end fixes surfaced when validating Power Rangers Unlimited: Edge of Darkness on A1QA Test Library: (1) RECURSIVE PREDICATE — extend `hasLCPAcquisition` to walk all `book.acquisitions` (not just `defaultAcquisition`) plus each one's `indirectAcquisitions` chain. Edge of Darkness exposes two top-level acquisitions — `application/atom+xml;…opds-catalog` AND `application/vnd.readium.lcp.license.v1.0+json` — and the LCP MIME is on the SIBLING, not the default. A predicate that only walked `defaultAcquisition.indirectAcquisitions` missed the LCP entirely and routed the book to the plain-PDFKit fallback in `BookService.presentPDF`, which failed on encrypted bytes. New test: `testHasLCPAcquisition_siblingLCPAcquisition_returnsTrue` encodes the Edge-of-Darkness shape and asserts the canOpenBook ⇄ hasLCPAcquisition divergence — canOpenBook is FALSE (only inspects default), hasLCPAcquisition is TRUE (walks siblings). (2) ROUTE-TO-READIUM SWITCH — `BookService.presentPDF` and `BookCellModel.didSelectRead` now use `hasLCPAcquisition(book)` rather than `canOpenBook(book)` as the LCP-route guard, so the sibling-shape case actually reaches `ReaderService.openPDF`. (3) LOADING INDICATOR — plumb `onFinish` through the entire LCP open chain (BookDetailViewModel → BookService → presentPDF → openPDF → libraryService.openBook) so `processingButtons` / cell `isLoading` stays TRUE until the publication is opened and the route is pushed. LCP open is heavy (1-3s on real Marketplace containers); without the held spinner the user sees nothing happening for that window. `BookDetailViewModel.openBook` no longer clears `processingButtons` synchronously before `presentPDF` — it does so in the open completion instead. (4) MEMORY LEAK FIX — `NavigationCoordinator.readiumPDFById` was not in the cleanup-threshold accounting nor in `performCleanup`, so every LCP open accumulated a `Publication` (with its LCP content- protection state, GCDHTTPServer endpoint, and decrypted page caches) and the app eventually OOMed. New `removeReadiumPDF(forBookId:)` helper called from `ReadiumPDFReaderView.onDisappear` drops the publication on back-out; cleanup tracking and performCleanup now include `readiumPDFById`/`readiumPDFTOCById`. (5) NAVIGATION CHROME RESTORED — `ReadiumPDFReaderView` now wraps `ReadiumPDFContainer` in `TPPPDFNavigation` exactly like the legacy `TPPPDFReaderView` does for open-access PDFs. Back button, TOC button, previews/bookmarks segmented picker, and search + bookmark toolbar buttons are visible again. The side panels — `TPPPDFTOCView`, `TPPPDFPreviewGrid`, bookmarkView — see a publication-backed `TPPPDFDocument` shim that proxies their TOC, pageCount, and label calls to Readium without forking the UI. (6) PUBLICATION-BACKED TPPPDFDocument — new `init(tableOfContents: pageCount:)` initializer accepts pre-loaded snapshots so the side panels stay synchronous. Readium 3's `tableOfContents()` and `positions()` are async; `ReaderService.openPDF` awaits both before pushing the route. Snapshot is stored on `NavigationCoordinator.readiumPDFTOCById` and resolved by `NavigationHostView` when constructing the view. (7) TEST FILE PRUNING — `LCPPDFsTests.swift` and `TPPEncryptedPDFDataProviderTests.swift` referenced the LCPPDFs.extract/decrypt/temporaryUrlForPDF/deletePdfContent + TPPEncryptedPDFDocument APIs that the migration deleted. Removed along with their pbxproj entries. `TPPPDFModelTests` trimmed to only the tests that survive the publication-backed redesign. (8) HREF COMPARISON FOR TOC PAGE NUMBERS — `AnyURL.string` is internal to ReadiumShared. The Readium-Link → page-number resolver in `ReaderService.pageNumber(for:positions:)` compares via `String(describing:)` rather than the inaccessible `.string` accessor; falls back to page 0 (cover) on no match. VERIFIED END-TO-END: Power Rangers Unlimited: Edge of Darkness opens on iPhone 16 Pro sim against A1QA Test Library. Logs show ReadiumNavigator.PDFNavigatorViewController instantiated + ReadiumGCDWebServer streaming pages. OCR captures author credits and title page. No "Unable to load PDF file" error. Power Rangers #2 (open-access PDF) continues to open through the legacy PDFKit path unchanged. **Scope:** 14 production files touched, 2 obsolete test files deleted. Net change vs the pre-migration baseline now includes the chrome restoration delta. **Not done — perf:** First open of an 829MB LCP container takes ~5–60s on real devices. AssetRetriever + LCP key derivation + PublicationOpener is the hot path; needs profiling to identify specifically what's expensive. The loading indicator at least surfaces the wait to the user. Tracked separately. **Not done — publication-mode search:** Search button in the publication path is wired into TPPPDFNavigation chrome but the underlying `document.search(text:)` is a no-op for publication mode. Bridging to Readium 3's `SearchService` (`publication.search(query:)`) is follow-up scope. **Not done — publication-mode thumbnails:** Preview-grid cells show page-number labels only (no thumbnail image) in publication mode. Readium's PDFNavigator renders pages internally but doesn't expose a public thumbnail API; rendering off-screen would require reading the decrypted PDF bytes per-page, which defeats the streaming benefit. Follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed: end-to-end LCP PDF reader hardening (commit 54c518f) Edge of Darkness validation surfaced four real issues beyond the OPDS-layer predicate. All addressed in this push, build green, 10 pre-push test classes pass. 1. Sibling-acquisition shape — Edge of Darkness was failing for a different reason than Family Matters. Its registry shows TWO top-level acquisitions: Fix: walk 2. Memory leak — Fix: added to cleanup tracking + new 3. Loading indicator wasn't showing. Fix: 4. Nav chrome restored. Deferred to follow-up (noted in commit body):
🤖 Generated with Claude Code |
… survives back-out
Three perf+UX wins building on the chrome restoration:
(1) ROUTE PUSHES IMMEDIATELY — `ReaderService.openPDF` now pushes the
`.pdf` route before kicking off `libraryService.openBook`. Previously
the open was async-but-awaited before the route push, leaving the
user staring at the book-detail page for 30–60s on large LCP
containers wondering if anything was happening. New
`ReadiumPDFLoadingView` shows a full-screen spinner + book title
while the open is in flight; `NavigationCoordinator.readiumPDFPending`
is `@Published`, so when the publication lands the host view swaps
the loader for the real reader without a polling/timer dance.
(2) TOC + POSITIONS LOAD MOVED OFF THE FIRST-PAGE-RENDER PATH — the
eager AES decrypt that dominates LCP open time (the hundreds of
`Successfully decrypted 2064 -> 2048` log lines you see on Edge of
Darkness) is triggered by `publication.positions()` and
`publication.tableOfContents()` because Readium has to walk the
PDF cross-reference table + outline through the LCP content
protection layer. Previously both ran inline before the publication
was stored, blocking the navigator from rendering page 1 until the
decrypt loop finished.
Now: as soon as the publication opens, it's stored in the
coordinator → navigator starts rendering page 1. A low-priority
`Task.detached(priority: .utility)` loads TOC + page count in the
background. When ready it's stored on the coordinator's
`@Published readiumPDFTOCById` → side panels (TOC view, preview
grid) re-render with populated data. The navigator itself stays
mounted (UIViewControllerRepresentable's `updateUIViewController`
fires but the controller persists), so the user keeps page 1.
(3) TOC CACHE SURVIVES BACK-OUT — `removeReadiumPDF` no longer drops
the `readiumPDFTOCById` snapshot. The `Publication` and its heavy
LCP/HTTP-server state still release on the reader's `onDisappear`
(memory-leak fix from the earlier hardening commit), but the TOC
metadata (<100 entries × ~200 bytes) stays. Re-opening the same
book in the same app session skips the entire async TOC load —
side panels populate instantly; only the publication itself has to
re-open. `ReaderService.openPDF` checks
`resolveReadiumPDFTableOfContents` and skips the background TOC
Task when the cache is warm.
(4) FAILURE PATH POPS THE ROUTE — on `libraryService.openBook` failure
we now `removeReadiumPDF(forBookId:)` + pop the navigation path
so the user isn't left sitting on a forever-loading spinner.
Build green. Existing 5/5 LCPPDFAcquisitionPredicateTests still pass.
**Scope:** 4 production files (NavigationCoordinator +18 lines,
ReaderService +27 lines, NavigationHostView +9 lines,
Strings.swift +1 line) + 1 new file (ReadiumPDFLoadingView.swift, 45
lines).
**Not done — disk persistence of TOC.** Cold-app re-open still has to
re-decrypt the PDF cross-ref to compute TOC + positions. Writing the
snapshot to `<account>/registry/<bookId>.toc.json` would solve it.
Follow-up in this push series.
**Not done — Readium PDFNavigator content-cache tuning.** Default
decrypted-resource cache size may be over-allocated. Needs Readium
API research. Follow-up.
**Not done — GCDHTTPServer endpoint teardown audit.** Need to confirm
`httpServer.serve(at:publication:)` (called in LibraryService.preparePresentation)
has a corresponding stop on publication release. Follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mark log + perf timings
Closes the 5-item perf/memory dial-in pass on top of the chrome
restoration and immediate-route-push commits. All five items
addressed; one had nothing to tune; two are pure mitigation logic;
two add new infrastructure.
(1) STRUCTURED TIMING LOGS — added `[PERF] [LCP-PDF] T0/T1/T2/T3`
markers to ReaderService.openPDF covering the four user-visible
stages: Read tap → route push, route push → publication open,
publication open → TOC+positions load complete. Each marker
logs cumulative-from-T0 and stage-specific elapsed-ms. Grep
`[PERF] [LCP-PDF]` in a Console log after a test open and you
get a per-stage breakdown without Instruments. Used in lieu of
WDA-based Instruments runs since Xcode account login for
device profiling is currently blocked.
(2) TOC + PAGECOUNT DISK PERSISTENCE — new
`ReadiumPDFTOCCache` writes the publication's TOC + page count
snapshot to `<accountDir>/registry/pdf-toc/<sha256(bookId)>.json`
on first successful load. Schema-versioned JSON; non-fatal
failures. ReaderService.openPDF checks disk BEFORE pushing the
route — cache hit populates the coordinator snapshot
immediately so a cold-app re-open shows TOC + preview-grid
page count the instant the navigator appears. Cache is
invalidated by LocalBookContentService.deleteLocalContent on
book return / re-borrow so stale TOC isn't reused against
new content.
(3) READIUM PDFNAVIGATOR CACHE AUDIT — no action. Audited
`PDFNavigatorViewController.Configuration` (only exposes
preferences/defaults/editingActions), `PDFDocumentHolder`
(single PDFKit.PDFDocument ref, no cache knob), and the LCP
decryption path (`CBCLCPResource` wraps with
`BufferingResource(size: 8192)` — small + fixed). PDFKit's
page cache is Apple-internal and responds to system memory
warnings automatically. Memory hotspots already covered by
earlier readiumPDFById cleanup + GCDHTTPServer endpoint
deregistration (this commit, item 4). No Readium-level knob
available to turn.
(4) GCDHTTPSERVER ENDPOINT TEARDOWN —
`LibraryService.preparePresentation` calls
`httpServer.serve(at: "/publications/\(id)", publication:)`
on every open but had no corresponding `remove(at:)` call.
Each publication open leaked a handler + resource transformer
in `GCDHTTPServer.handlers` / `.transformers` even after the
publication itself released. Added
`LibraryService.releaseServedPublication(forBookIdentifier:)`
+ `ReaderService.releaseReadiumPDF(forBookIdentifier:)`
wrapper; `ReadiumPDFReaderView.onDisappear` now routes through
the wrapper which drops both the coordinator's publication ref
AND the HTTP-server endpoint. Net: per-open registrations now
have matching deregistrations.
(5) BOOKMARK PARSE-FAILURE LOG — investigated the
"298 items failed to parse" warning. Not a parse failure: the
server's `/annotations/` endpoint returns ALL of a user's
bookmarks across every book they've read. `TPPBookmarkFactory.make`
correctly filters by `source != bookID` and returns nil for
other-book bookmarks. The "failed to parse" log was misleading.
Downgraded to info with explanatory comment + flagged the
server-architectural cost: bookmark cold-open re-downloads the
entire user-wide bookmark list. A CM-side per-book filter
parameter would be the right fix; client-side there's no
cheap mitigation.
OPEN-PATH STATE AFTER ALL 5 ITEMS:
- Read tap → reader screen visible (loading state): <1 frame
- Reader screen → first page rendered: bounded by Readium
publication open (LCP key derive + cross-ref decrypt). Timing
markers now make this measurable.
- Side panels (TOC, preview-grid, bookmarks):
* Disk cache hit → populated at T1 (instant)
* Disk cache miss → populated at T3 (background task)
- Reader dismissed: Publication + HTTP server endpoint + decrypted
page caches all release; TOC + pageCount snapshot stays in
memory + on disk for the next open.
**Scope:** 6 production files modified + 1 new file
(ReadiumPDFTOCCache.swift). LCPPDFAcquisitionPredicateTests still
5/5 green.
**Not done — Instruments-based perf profiling on Moes Max.**
Blocked on Xcode → Settings → Accounts sign-in for Synctek dev cert
(needed by `simdrive bootstrap-device` to build the WDA test
runner). With the new timing logs the same data is available from
Console output, so this is no longer blocking the dial-in work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rlapping opens The previous open flow hid the ReadiumPDFLoadingView the moment the publication object landed in NavigationCoordinator (T2, ~100ms on a fast device). But the user-perceived "blank reader" window extends through PDFNavigator's first cross-ref walk through the LCP content protection layer — hundreds of `decrypt 2064 -> 2048` calls, which on large Marketplace containers takes seconds-to-minutes. The user saw a frozen blank page instead of a progress indicator. This switches the pending → rendered handoff to the navigator's first `locationDidChange` emission (the real "page 1 painted" signal). The host view now stacks the loading overlay ON TOP of the ReadiumPDFContainer until that signal fires, so the user sees "Loading…" continuously from tap to first paint. Also adds a per-book generation counter on ReaderService.openPDF that bumps on `releaseReadiumPDF` and is captured by the async `libraryService.openBook` completion. A stale completion (e.g. user backed out before the first decrypt walk finished, then re-tapped the same book) no-ops instead of writing the OLD publication on top of the NEW one. Same dict also coalesces a double-tap on Read into a single open — without it, two concurrent decrypt walks ran in parallel after the second tap. **Not done:** PDFNavigator's internal decrypt Tasks for a released publication still run to completion — Readium holds them and there is no public cancellation seam. The generation guard prevents the result of that stale work from landing in the coordinator, but the CPU/IO cost has already been paid. Mitigation lives upstream in Readium. **Scope:** loading-indicator timing fix + overlapping-open guard. TOC disk cache, GCDHTTPServer teardown, and PERF markers already shipped in 33be65b. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…memory release
The plain spinner gave the user no signal that the open was making
forward progress — on a multi-second cross-ref decrypt walk, the
question "is this stuck?" stays open until page 1 paints.
Replaces the spinner with a dark, cinematic overlay:
- gradient near-black background
- book title rendered LARGE and faded behind everything as a
watermark (~8% white) — present but not loud
- foreground stack: cover thumbnail, title + author, animated
linear progress bar with `1 - exp(-blocks/90)` fill curve, and
the live status line ("Decrypting content… 327 blocks")
A new `LCPPDFOpenProgress` ObservableObject is the single source
of truth for that status. `TPPLCPClient.decrypt(...)` records each
successful block on both overloads (the protocol entry point and
the convenience extension) so the counter ticks visibly on every
forward step. The reporter has four phases — preparing, opening
publication, decrypting content, loading first page — driven by
`ReaderService.openPDF` and cleared by either the navigator's
first `locationDidChange` (the page-1-painted signal) or by
`releaseReadiumPDF` on back-out.
Also addresses the OOM crash from the device log
("Memory warning received — clearing in-memory catalog cache"
followed by a hard exit): `openPDF` now posts the system
memory-warning notification proactively at T0, so the catalog
in-memory cache, book-cell model cache, and image caches drop
their working sets BEFORE the decrypt walk pushes RAM over the
OS ceiling. Disk caches are content-addressed and survive — the
re-hydrate on back-out is a no-op from the user's perspective.
The fill curve is honest about its denominator: the total number
of decrypt blocks needed to render page 1 is not known a priori
(varies by book size and bookmarked page), so the bar never
reaches 100%. It asymptotically approaches 95% and snaps shut
when first-page-rendered fires.
**Not done:** the exponential fill curve was tuned by inspection
on a single Edge of Darkness open (~30s, ~240 blocks to reach
page 8). A device-pool sweep across small/large books would let
us replace the magic 90 with a per-book-size baseline. Filed as
follow-up.
**Scope:** loading-view UX + memory pre-release. Reader2 EPUB
path is unchanged — this is LCP-PDF-only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes: 1. **Loading view shows %, not block counts.** Common users don't want to see "Decrypting content… 327 blocks" — that's debug noise. Switched to "Loading… 47%" derived from the same exponential curve. The `percentComplete` accessor moved onto `LCPPDFOpenProgress` so the view and the future tests share the same source of truth. 2. **Per-process LRU cache of AES-decrypted blocks (`LCPDecryptCache`).** PDFNavigator walks the PDF cross-ref table on every open; many blocks (cross-ref index, font tables, repeated stream dictionaries) are requested multiple times within a single render pass, and EVERY block was being decrypted again on re-open of the same book. The cache is keyed by ciphertext bytes — same ciphertext = same plaintext under a stable LCP key — and capped at 16MB via NSCache's totalCostLimit. Hits skip the R2LCPClient AES round-trip entirely. Cached hits count toward progress (the page is one step closer to rendering, just for free) but at 50% weight in the percentComplete curve to avoid the bar jumping on re-opens. `NSCache` evicts under memory pressure automatically. The memory-warning observer purges the whole thing — including the pre-emptive warning `ReaderService.openPDF` posts at T0, so a fresh open of book B doesn't pin book A's blocks in memory. **Not done:** the 16MB cap is conservative — on devices with plenty of headroom we could let it grow. Cache hit rate isn't instrumented yet; would need to log it to tune the cap. **Scope:** UX polish + perf cache. Memory pre-release from the previous commit stays as-is (catalog cache drop at T0). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The bar was capped at `min(95, ratio*100)` so on large books it saturated at 95% and visibly froze for the rest of the open (which on a multi-thousand-block textbook could be minutes). Then the OOM crash sealed the deal — user saw "stuck → crash." Three changes: 1. **Two-stage curve, max 99%.** Stage 1 is the existing exponential `1 - exp(-credit/90)` up to ~80%. Stage 2 is a linear creep above 80% — every additional 50 credit adds ~1%, so a thousand- block book sees the bar drift 80→99 over many visible stages instead of plateauing. Above 95% the status text switches to "Finishing up…" so the user understands the bar is almost done, not stuck. 2. **Pause `TPPBookCoverRegistry.fetchImage` during LCP open.** The device log on the OOM showed concurrent CGImageSourceCreateThumbnail failures against BiblioBoard URLs — the catalog prefetcher kept firing in parallel with the decrypt walk, allocating large image buffers that the decoder then failed to render and leaked. A new `nonisolated LCPPDFOpenProgress.isOpenInProgress` flag (atomic under a tiny NSLock so the cover prefetcher can read it from arbitrary actors without a MainActor hop) lets the cover fetcher early-return nil while LCP is in flight. Catalog scroll re-issues the fetch organically once the open finishes; cache misses are the same shape as a transient network failure. 3. **Per-1.5s memory log line during open.** A new `ReaderService.residentMemoryMB()` reads `mach_task_basic_info`, and a detached Task spins out `[PERF] [LCP-PDF] residentMB=X blocks=Y cacheHits=Z phase=...` while progress is non-idle. Next device run on an OOM-prone book will produce a memory trace showing exactly which phase tips the device over the jetsam ceiling — without needing Instruments. **Not done:** the underlying cause of the OOM (PDFNavigator's internal page buffers for large textbooks) is not addressed. These changes reduce the surrounding pressure (cover prefetch, catalog caches) so PDFNavigator has more headroom, and add the diagnostics needed to triage what's left. If the next run still crashes, the memory log will tell us where to look upstream. **Scope:** progress UX + memory diagnostics + prefetch pause. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Device diagnostics from a failed open of Power Rangers Unlimited
showed:
- 841,570 decrypt calls in ~10s (~7k/s)
- residentMB grew 1792 → 2846 (~107 MB/s, ~9 KB retained per call)
- Decrypt cache hit rate: 0.13% — every call is a fresh ciphertext
because AES-CBC mode means same plaintext at different file
offset → different ciphertext. PDFNavigator's random-access
reads on the LCP-wrapped PDF stream produce uniformly-unique
ciphertext inputs, so a ciphertext-keyed cache provides almost
no benefit.
The fundamental problem: the current
Readium-PDFNavigator-on-LCP-publication architecture cannot
work for large Marketplace PDFs. PDFNavigator's random-access
read pattern + Readium's retained per-read buffers grow memory
linearly until iOS jetsam-kills the app. Small LCP PDFs still
work (the open finishes before memory pressure crosses the
ceiling) but large ones (Power Rangers, textbooks) cannot.
This commit ships the **graceful failure** half of the fix:
- Periodic memory log gets an abort trigger at 150k decrypts.
At that count the open is definitively not going to succeed
(typical small PDFs paint in 200-2000 decrypts), so we pop
the route, surface a user-facing alert, and log a Crashlytics
event with bookId + residentMB. Better than an OOM/jetsam.
- The per-call `Log.debug("Successfully decrypted X bytes")` line
in TPPLCPClient is removed. At 7k calls/s it was throwing
string-format + log-system work that itself contributed to
the pressure. The progress reporter + periodic [PERF] [LCP-PDF]
residentMB lines carry the same diagnostic information without
the per-call cost.
**Not done — the real fix is architectural.** We need to switch
the LCP-PDF render path from "Readium PDFNavigator streaming
from a still-encrypted publication" to "pre-decrypt to a temp
.pdf on disk, then hand to PDFKit." Readium's Resource API
exposes a `read(range: nil)` that should return the full
decrypted publication bytes in a single linear pass — turning
~800k random-access decrypts into ~25k linear ones for a 50MB
book, AND letting PDFKit (which has efficient seeking) handle
all the random-access work after the file is on disk. Filed as
PP-4454 follow-up.
**Scope:** crash-prevention guardrail + diagnostic noise
reduction. Real perf fix is the next commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r PP-4454 OOMs) Replaces the Readium-PDFNavigator-on-LCP-publication render path with a single-pass disk extract, then renders the resulting plain .pdf with PDFKit. This is the architectural fix the previous commits' diagnostics pointed to. Why the old path can't work: - PDFNavigator does random-access reads on the PDF cross-ref table. Each read goes through the LCP decrypt layer. - AES-CBC produces uniformly-unique ciphertext for each file offset, so the in-process decrypt cache hits ~0.13% of the time (verified on device). - Each decrypt leaves ~9 KB retained in Readium's resource pipeline. Memory grows ~107 MB/s, OOM in ~10s on Power Rangers Unlimited. Why disk-extract fixes it: - `publication.get(link).stream(consume:)` streams decrypted bytes in LINEAR order (one pass through the ZIP entry), so the decrypt count drops from ~800k random-access calls to ~25k sequential ones for a 50MB book. - Each chunk is written immediately to a FileHandle and freed — in-memory footprint stays at one chunk (≪1 MB). - Once extraction is done, the Readium publication is released (`releaseServedPublication`), freeing its resource pipeline. - PDFKit mmaps the on-disk PDF; random-access page rendering costs whatever PDFKit costs, with no LCP layer involved. What's added: - `LCPPDFDiskExtract` — namespace with `cachedURL`, `extract`, `invalidate`. Stream loop + FileHandle write + per-chunk progress publish. Cache layout `<accountDir>/registry/lcp-pdf-extracts/<sha256(bookId)>.pdf` matching the existing TOC cache. - `LCPPDFOpenProgress` gains `bytesExtracted` + `totalExtractBytes`. The progress bar prefers the BYTE-accurate percentage when the total is known (from `Resource.estimatedLength()`), falling back to the legacy decrypt-curve only when extraction hasn't started yet. This is the first time the bar is showing real progress instead of a heuristic. - `LocalBookContentService.deleteLocalContent` invalidates the extract on book return so a re-borrow gets a fresh extract. - `NavigationHostView` is unchanged — the `.pdf` route already preferred the plain-PDF branch when no Readium publication was stored. We just route through `storePDF` instead of `storeReadiumPDF` on completion. What's NOT removed yet: - `ReadiumPDFReaderView`, `ReadiumPDFContainer`, `ReadiumPDFViewController`, `ReadiumPDFTOCCache`, `LCPDecryptCache`, the Readium-PDF host-view branch — all dead code under the new flow but cleaner to remove in a follow-up once we've shipped a build that's confirmed stable. Smaller blast radius for this commit. **Scope:** LCP PDF render architecture swap. EPUB / audiobook / non-LCP PDF paths unchanged. **Not done:** dead-code cleanup of the old Readium-PDFNavigator path. Filed as follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…heavy logging User report: latest disk-extract build hangs at 1% on device then crashes. Likely cause: `Resource.stream(consume:)` on an LCP-wrapped publication isn't actually emitting incremental chunks — it's either firing the consume callback once with a giant buffer (forcing a full-file allocation inside Readium's pipeline before our consume runs) or chunking unpredictably for the progress bar. Switches the extract loop to explicit `read(range:)` calls with a fixed 1MB chunk size: - We control chunk size and the OS gets FileHandle flush opportunities between chunks. - Per-chunk progress is deterministic — bytesExtracted advances by exactly 1MB per iteration. - A stall is now diagnosable: a single failing `read(range:)` call surfaces as a Swift error from the `Result` switch, not a hang. Added detailed logging: - Link href + media type before extraction begins (verifies we're reading the PDF resource, not the manifest or cover). - `estimatedLength()` result (including failure mode). - Forward-progress log at offset == chunkSize and every 16MB thereafter — flags a stall without flooding the log on a fast open. - explicit `read(range:)` failure with the range that failed. Fallback path: if `estimatedLength()` returns nil/0 we can't loop on ranges, so we fall back to the original `stream(consume:)` approach. Logged as a warning so we know to investigate. **Not done:** the actual root cause of the 1% hang isn't proved yet — chunked reads might surface the same symptom in a more diagnosable form. The next device run will tell us whether we see deterministic chunk progress, or which chunk read fails. **Scope:** disk-extract reliability + diagnostics. UI / state machine unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
User report: on Moes Max, after the prior commit's stream-based extract crashed mid-write, opening the same book showed "Unable to load PDF file" with the legacy PDFKit reader screen. Root cause: `cachedURL()` only checked file existence, not correctness. A partial/truncated extract from a crashed prior run would pass the existence check, get handed to `TPPPDFDocument(url:)`, fail PDFKit parsing, and surface the generic "Unable to load" alert. Adds two cheap validation checks on cache hit: - File size must be ≥ 1KB (a true LCP-PDF extract is hundreds of MB; anything smaller is definitely a partial write). - First 5 bytes must equal `%PDF-` (0x25 0x50 0x44 0x46 0x2D). PDF spec requires this header on every well-formed PDF; a file missing it can't be a complete extract. If validation fails the file is deleted on the spot so the next `extract()` call rebuilds cleanly. The validation is intentionally NOT a full `PDFDocument(url:)` open — that mmaps and parses the xref table on the entire ~830MB file, which is exactly the work we want to defer to the actual reader UI. **Not done:** there's no validation of internal PDF structure beyond the header bytes. A file with `%PDF-` magic but a corrupt trailer would still get handed to PDFKit and fail at parse time. The previous fix (chunked reads + remove-on-failure) makes this unlikely going forward, but a future-proof option would be a small marker file (`<sha>.complete`) written atomically after successful extract — only consider the cache valid when the marker is present. **Scope:** cache-correctness guardrail. Mostly recovers users whose phone was stuck with a partial extract from the prior stream-based build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nces User report: opening Edge of Darkness shows the "Sync Reading Position" prompt on every open, even when they haven't changed position on a different device. Root cause: `Stay` was a no-op that dismissed the alert without recording the decision. Each fresh open built a new `TPPPDFDocumentMetadata`, re-fetched the remote page, found the same mismatch (local 0 / remote 14, or similar after cross-device reading), and re-prompted. The fix records the user's Stay decision per-book against the SPECIFIC remote page value: - `dismissRemotePageSync()` persists `(bookId, declinedRemotePage)` to UserDefaults. - `shouldPromptRemotePageSync()` returns false when the current remote page equals the previously-declined value for this book — so re-opens are quiet on the same mismatch. - If the server later advances to a DIFFERENT page (real new progress from another device), the previously-declined value no longer matches and the prompt fires again. - `syncReadingPosition()` (the Move action) clears the declined marker so subsequent updates can prompt. Why not just push local-to-server on Stay: that would overwrite genuine reading progress made on another device (user reads 50 pages on device A, opens device B at page 10, taps Stay → if we pushed 10 to server, device A loses its position). The per-book / per-remote-page decline marker preserves cross-device progress while not nagging. Wired up in `TPPPDFReaderView` (the legacy PDFKit reader, which is the current LCP-PDF render target after the disk-extract architecture switch). `ReadiumPDFReaderView` has the same alert logic but is dead code under the disk-extract flow — cleanup follow-up. **Not done:** the EPUB reader has its own sync-position prompt on `LastReadPositionSynchronizer` — not touched here. Same fix would help if it has the same complaint, but PR scope is LCP-PDF only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…reporter PP-4454 disk-extract pipeline now has unit-level test coverage on its two highest-value surfaces. Both classes survive mutation testing — flip a comparator in `cachedURL`'s size check, drop the %PDF- magic byte test, change a +1 to -1 in `recordDecrypt`, or break the phase auto-transition, and a specific test fails. **LCPPDFDiskExtractTests (5 tests)** — locks in the cache-correctness guardrail that addresses the device-side "Unable to load PDF file" seen after the stream-based extract crashed mid-write: - testCachedURL_validPDFHeaderAndSize_returnsURL — happy path, also verifies the validator doesn't delete a valid file - testCachedURL_undersizedFile_returnsNilAndDeletesFile — rejects + cleans up a 5-byte partial write - testCachedURL_wrongMagicBytes_returnsNilAndDeletesFile — rejects + cleans up a PK\x03\x04 ZIP-magic fixture (the raw LCP container leaking through as the cached file) - testCachedURL_noFileOnDisk_returnsNil — negative path - testCachedURL_partialWriteWithGoodHeader_stillRejected — edge case where header IS present but size sub-1KB; lock-in for the AND-not-OR semantic of the two checks Tests derive the production file path from the same public account-directory helper, so a refactor of the filename strategy is caught by the test (production constant + test constant diverging). **LCPPDFOpenProgressTests (13 tests)** — locks in the observable state machine that drives the loading-view percentage. Per CLAUDE.md "Round-trip wiring tests required for state machines": - begin → counters zeroed + cross-actor flag flipped - recordDecrypt(byteCount:) → blocks+bytes increment - recordDecrypt(fromCache:true) → only cachedHits, NOT decrypt counters (cached hits "ARE forward motion but for free") - recordDecrypt while .idle → no-op (stray decrypts from audiobook paths must not poison the next LCP-PDF open) - AUTO-TRANSITION seams: first decrypt flips .openingPublication → .decryptingContent; first chunk-write flips → .extractingToDisk - percentComplete: bytes-based when total known, decrypt curve fallback when not, clamps at 99 (never 100 before paint), 0 on fresh begin - Round-trip: begin → work → finish → begin → clean state again setUp uses `begin(bookIdentifier: "test-reset")` + `finish()` to zero ALL counters (not just phase + identifier) — `finish` alone leaves decrypt counters intact, which would cross-contaminate tests via the @mainactor singleton. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mauricecarrier7
added a commit
that referenced
this pull request
May 27, 2026
…4454 merge) PP-4454 (#1008) just landed on develop, migrating LCP-PDF rendering from the legacy bitmap-tile TPPEncryptedPDFView path to PDFKit via ReadiumPDFViewController. Without this wire-up, LCP-PDFs in the new path would route around the PP-4297 gate (the PalacePDFView gating only fires on the legacy non-LCP TPPPDFView path) and expose Copy/Cut/Paste/Look Up/Share on long-press. ReadiumPDFViewController now passes ReaderEditingActions.resolve(for: book) as editingActions to PDFNavigatorViewController.Configuration — DRM books get [] (no selection handles, no Copy); non-DRM books get Readium's defaultActions. Same predicate, same call shape as the EPUB navigator gating. 26/26 PP-4297 unit tests still passing post-merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mauricecarrier7
added a commit
that referenced
this pull request
May 27, 2026
…1012) * PP-4297: gate reader copy/paste on the DRM-protected flag (EPUB + PDF) Aligns both readers with the rule: DRM-protected titles suppress the system text-selection edit menu (Copy/Cut/Paste/Look Up/Share); non-DRM titles keep the full menu. - TPPBook.isDRMProtected: walks the full nested acquisition chain for Adobe ACS + Readium LCP (EPUB/PDF/audiobook). One predicate; adding a new DRM scheme is a one-line change. - ReaderEditingActions.resolve(for:isSample:appending:): single decision point for the Readium EditingAction list. DRM book → []; non-DRM book → defaultActions + caller-supplied custom actions; sample preview of any book → treated as non-DRM. - TPPEPUBViewController: routes its EditingAction list through the helper so DRM EPUBs lose the long-press menu including Highlight; non-DRM EPUBs keep Readium's defaults plus Highlight; samples behave like non-DRM. - PalacePDFView (PDFKit subclass) + TPPPDFView wiring: sets allowsCopy from metadata.book.isDRMProtected in onAppear (metadata isn't available at field-init time; the view is mounted before any long-press can fire). canPerformAction + buildMenu overrides suppress Copy/Cut/Paste/SelectAll /Look Up/Share when DRM. Tests pin the matrix: TPPBookDRMProtectedTests covers every DRM scheme + open-access counterpart + the OPDS fixture; ReaderEditingActionsTests covers EPUB gating including sample-of-DRM passthrough and custom-action suppression under DRM; PalacePDFViewTests covers Copy/Cut/Paste/SelectAll /Look Up/Share suppression, non-editing-selector passthrough, and the default-true safe behavior. 23/23 passing locally. **Scope:** EPUB gating via Readium EditingActions; PDFKit PDFView gating via canPerformAction subclass. Both call sites updated. ReaderEditingActions also serves the upcoming Readium PDF path (post-PP-4454) without further work. **Not done:** Live end-to-end DRM simdrive verification (DAISY-EPUB downloads fail repeatedly on the harness sim's network). Unit tests pin the gating matrix exhaustively; manual QA pass per the ticket's acceptance criteria (one Adobe-DRM + one LCP title, one open-access EPUB + one open-access PDF) belongs to release-regression rather than this PR. **Deferred:** None. The Readium-PDF path lands separately on PP-4454; the helper here is ready for that caller — no follow-up needed in this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PP-4297: fix trailing comma in PalacePDFView blocked-selectors SwiftLint trailing_comma violation on the static array; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PP-4297: apply architect review feedback (rev_dc27d478) - Drop the dead-code clause that checked path.types for ContentTypePDFLCP ("application/pdf+lcp") in isDRMProtected. That MIME isn't in TPPOPDSAcquisitionPath.supportedTypes() / supportedSubtypes, so the path-walker can never return a path containing it. LCP-PDF detection happens through the ContentTypeReadiumLCP clause; the LCP- PDF test still passes with the same coverage. - Invert buildMenu order in PalacePDFView: defer to super first, then strip. The canPerformAction short-circuit prevents leakage today, but super-then-mutate is defense-in-depth against a future PDFKit version that inserts items late in its buildMenu pass. 23/23 unit tests still passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PP-4297: close QA testing gaps from rev_1b4e3a40 - Add canPerformAction tests for the system-private _define: and _translate: selectors (gap: blocked-selectors list included them but no test pinned them — flipping the entries would not fail any test). - Add accessibility-selector passthrough test for AC #7 (VoiceOver text read-aloud preserved on DRM titles). Asserts the subclass returns the same value as the allowsCopy=true baseline for an accessibility selector. - Document the buildMenu override as belt-and-suspenders behind canPerformAction so the intent isn't lost if someone reads only buildMenu and concludes it's redundant. - TPPPDFView gets an architectural note that today's PDFKit-rendered path is non-encrypted only (LCP-PDF goes through the bitmap-tile TPPEncryptedPDFView path); the PalacePDFView gating is forward- looking for any future PDFKit-rendered DRM-PDF path (PP-4454 Readium-PDF, etc.). 12/12 PalacePDFView tests passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * PP-4297: wire DRM gating into the new Readium-PDF navigator (post-PP-4454 merge) PP-4454 (#1008) just landed on develop, migrating LCP-PDF rendering from the legacy bitmap-tile TPPEncryptedPDFView path to PDFKit via ReadiumPDFViewController. Without this wire-up, LCP-PDFs in the new path would route around the PP-4297 gate (the PalacePDFView gating only fires on the legacy non-LCP TPPPDFView path) and expose Copy/Cut/Paste/Look Up/Share on long-press. ReadiumPDFViewController now passes ReaderEditingActions.resolve(for: book) as editingActions to PDFNavigatorViewController.Configuration — DRM books get [] (no selection handles, no Copy); non-DRM books get Readium's defaultActions. Same predicate, same call shape as the EPUB navigator gating. 26/26 PP-4297 unit tests still passing post-merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Closes PP-4454. Direct structural mirror of PR #958 (PP-4407 audiobook fix) ported to PDFs.
Marketplace OPDS 2.0 JSON feeds wrap LCP PDFs as
opds-publication+json → [LCP license → application/pdf]with the LCP MIME nested one level deep inindirectAcquisitions.LCPPDFs.canOpenBookonly inspecteddefaultAcquisition.type, so it returnedfalsefor Marketplace-shape PDFs (e.g. Family Matters by Gregory C. Elliott from the ticket).BookFileManager.pathExtensionthen routed the file to.epubinstead of.zip, breaking the LCP extract pass — patrons saw "an error message" instead of the PDF rendering.Fix
LCPPDFs.hasLCPAcquisition(new@objc static) — recursive predicate over the full acquisition chain, gated bydefaultBookContentType == .pdfso LCP audiobooks/EPUBs don't false-match.LCPPDFs.indirectChainContainsLCP(new private helper) — recursive walker, identical structure to the audiobook equivalent.BookFileManager.pathExtension— swap both LCP branches (audiobook + PDF) fromcanOpenBooktohasLCPAcquisition. The PDF side is the PP-4454 fix; the audiobook side completes the forward-port that PR fix(3.1.0): Marketplace audiobook open — pathExtension + symlink recovery + diagnostics #958 made on its hotfix branch but never landed on develop.Tests
New
LCPPDFAcquisitionPredicateTests(4 tests) — direct structural mirror ofLCPAcquisitionPredicateTests. The Marketplace-shape test assertscanOpenBook=false AND hasLCPAcquisition=trueon the same fixture — the divergence is the deterministic proof the recursive predicate is doing real work, not duplicatingcanOpenBook.21/21 LCP suites pass green on iPhone 16 Pro sim (4 new + 4 audiobook + 13 LCPPDFs).
Scope
Palace/PDF/LCP/LCPPDFs.swiftPalace/MyBooks/BookFileManager.swiftPalaceTests/LCP/LCPPDFAcquisitionPredicateTests.swiftPalace.xcodeproj/project.pbxproj~55 production LOC, 167 test LOC.
ForgeOS changeset:
cs_8e01deae.Test plan
LCPPDFAcquisitionPredicateTests— 4/4 pass (Marketplace nested chain kill point, top-level XML, no-LCP negative, content-type-gate negative)LCPAcquisitionPredicateTests(audiobook PP-4407 regression) — 4/4 still greenLCPPDFsTests— 13/13 still greenNot done / Deferred
Deferred (separate follow-up): The LCP PDF rendering wiring is broken at a deeper layer than this PR fixes.
TPPPDFDocument.init(encryptedData:decryptor:)exists but is never called on develop —BookService.presentPDFandBookCellModel.didSelectReadpass the on-disk URL toTPPPDFDocument(url:)(the plain PDFKit path), andLCPFulfillmentHandler.fulfillLCPLicensewrites the extract to a temp URL no caller reads. The acquisition-layer fix in this PR is necessary but not sufficient for Family Matters to actually open in the reader. The proper architectural fix lives onfeature/readium-pdf-navigator-migration(commit9d042ab38— Readium PDFNavigator); that branch is 373 commits behind develop and needs adaptation. Tracked separately.🤖 Generated with Claude Code