ADFA-5176: Serve documentation to the app's WebViews in-process

[davidschachterADFA]

Stacked on #1725 — this PR's base is task/ADFA-5241-charset, so its diff is just this ticket's work. GitHub retargets it to stage when #1725 merges.

One pipeline for reading documentation.db, in common, with two transports over it.

DocumentationContentSource owns row lookup, chunked-row reassembly, the dictionary-aware Brotli decode and the sdcard debug-database swap, under a read/write lock so readers don't have the handle closed under them. It also renders the Pebble template rows, so both transports serve finished pages and neither carries the template engine.

DocumentationRequestInterceptor answers the app's own WebViews through WebViewClient.shouldInterceptRequest, so a page's assets cost a database read instead of a TCP connection each. It matches the same http://localhost:6174/... URL space, so strings.xml, ToolTipManager's link builder and the DocumentationExtension contract need no changes; anything it declines — a /pr/ endpoint, an unknown path, a failed read — falls through to WebServer.

WebServer keeps port 6174 for WebViews not wired to the interceptor and for the /pr/ endpoints, but now reads through the shared source: no database handle, no Pebble engine, no gson, no decode path of its own. What's left is HTTP.

This is a port, not the original branch

The original was stacked on ADFA-5172 and ADFA-5175. Those are an abandoned investigation and a declined design, so this branch carries neither: the accept loop is stage's single-threaded one, and the stallThresholdMs/maxWorkerThreads config fields, the stall instrumentation and the worker pool are all gone.

The two rules the original duplicated are now shared rather than repeated:

• The dictionary is gated on the version the database declares (DatabaseVersionResolver.resolveMajorVersion), as WebServer already does, instead of on whether a CompressionDictionary table happens to exist.
• The charset comes from ContentTypeHeaders (ADFA-5241). The interceptor previously declared utf-8 for text/ only, so an SVG served in-process declared no encoding while the same row over the socket declared one.

Verified on hardware

SM-N986U, both transports, same page and same build. In-process (default):

D DocumentationRequestInterceptor: Served 'a/android/R.id.html' in-process, 115518 bytes.
I HelpActivity: Loaded 'http://localhost:6174/a/android/R.id.html' in 142 ms;
                2 requests, 231036 bytes served in-process.
D DocumentationRequestInterceptor: Served 'favicon.ico' in-process, 0 bytes.

grep -c 'Request is GET /a/android/R.id.html' against the server: 0. The socket never saw it. 115,518 bytes matches that row's plaintext size exactly, and the page renders correctly in the app's WebView.

Fallback, with /sdcard/Download/CodeOnTheGo.nointercept present:

D WebServer:     Request is GET /a/android/R.id.html HTTP/1.1
I HelpActivity:  Loaded '...' in 99 ms; in-process serving is off (Download/CodeOnTheGo.nointercept exists).

Driving HelpActivity from adb needs android:exported="true", which I added locally to measure and reverted before committing — the manifest in this PR is unchanged.

On the numbers: 142 ms in-process against 99 ms over the socket. These are single samples on a page with one asset, so they show both paths working, not a speedup. The connection-per-asset cost this ticket targets only shows up on pages with many assets, and I have not measured that fairly — worth doing before anyone claims a performance win.

Tests

356 across app and common, 0 failures: 14 for the content source, 9 for the interceptor, 9 for ContentTypeHeaders, 7 for the dictionary decode, 5 for the server. The two WebServerTest cases asserting the dictionary loads once per database now declare a version — without that the new gate would have left them passing while testing nothing.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[davidschachterADFA]

All fifteen review findings are now addressed across 7d47b098 and fd4c8fa0. Summary for anyone reading the PR rather than the commits.

Would have broken in production

• switchToDatabase never cleared templateCache — Pebble templates compiled from a closed database kept rendering. The old WebServer.switchToDatabase cleared it, and the field comment still promised it did.
• The brotli warm-up did not move with the decode. WebServer.decompressBrotli wrapped Brotli4jLoader.ensureAvailability() so a missing native library cost one failed read instead of the process; the guard stayed behind when the decode moved to common, and UnsatisfiedLinkError is an Error, so it escapes every catch (e: Exception) between there and the accept loop.
• The interceptor warmed brotli eagerly in shared's initializer, which runs during Activity/Fragment construction — turning a failed page into a hard crash on opening Help. Removed; the source warms lazily and converts the Error, covering both transports.
• Six unused imports in WebServer.kt, four of them the abandoned worker pool's, would have failed spotlessCheck in CI under the file-level ratchet.

Wrong answers rather than crashes

• Cache staleness was judged before the swap it depends on. The source swaps inside lookup()/withDatabase(), so checking generation first read the pre-swap value: on the swapping request, bookshelfTemplateId still pointed at the previous database's template row. refreshDatabase() now applies a pending swap without reading, and the discard runs after it.
• Priming the dictionary could fail an unrelated lookup — a locked database during one dictionary query failed every request, including compression = 'none' rows needing no dictionary. Priming is best-effort now; a brotli row that genuinely cannot resolve its dictionary still fails loudly from inside the decode.
• The two transports disagreed about what a path is. The interceptor matches url.path (decoded), WebServer matched the raw target, so an encoded space meant the interceptor found the row and the server 404ed it — the nointercept sentinel changed which pages work. WebServer decodes now, protecting + first (URLDecoder alone turns it into a space, and c++.html is a real shape here) and falling back to the verbatim path on a malformed escape.
• mimeAndCharset re-parsed the charset with substringAfter("charset="), disagreeing with the socket transport on quoted parameters and on case — for ; Charset=UTF-8 it missed the parameter entirely and the response went out with no encoding, the exact ADFA-5241 failure its KDoc claims to prevent. It delegates to ContentTypeHeaders.typeAndCharset now.
• sendContent built its header after the status line was already flushed, so a throw appended a second status line to a response that had claimed 200.
• failedDebugSwapTimestamp is @Volatile — the check reads it outside the write lock, and a 64-bit read is not atomic on armeabi-v7a.

Caught in my own evidence

HelpActivity loaded every page twice, once in onCreate and again through updateUIFromIntent. The proof was in this PR's own device log: "2 requests, 231036 bytes" for one 115,518-byte page. So the 142 ms in-process figure was timing two loads against the socket path's one, and the comparison I flagged as "not a speedup" was worse than that. The instrumentation also used wall-clock time and presented process-cumulative counters as the page's own; it uses elapsedRealtime now and says "totals so far".

The three ADFA-5220 version-gate tests deleted when WebServerTest was replaced are restored in DocumentationContentSourceTest, where dictionaryBytes now lives.

Declined, with reasons

Three WebSettings property reads with no assignment were deleted rather than turned into = true. They are no-ops today; assigning them would silently enable three security-relevant settings — universal file access among them — as a side effect of a reindentation. If the fragment's file:///android_asset handling needs file access, that deserves its own change and its own reasoning.

Test-helper duplication and Truth-vs-JUnit assertions are left as they are: both are worth doing, neither is worth expanding this PR's diff for, and the JUnit style matches the file this test was ported from.

Also fixed: comments describing a worker pool, awaitTermination and a templateCache this class no longer has, and ARCHITECTURE.md's raw-SQLite exception, which still named WebServer as the holder of the database handle.

366 tests across app and common pass; spotlessCheck is clean.

[davidschachterADFA]

On-device re-verify (Galaxy Note 20 Ultra, arm64, v8 debug)

The numbers previously in this PR predated 7d47b0986 and fd4c8fa0c, which changed exactly what they measured — brotli priming moved into DocumentationContentSource, the interceptor's eager ensureAvailability() came out, and HelpActivity stopped loading every page twice. Re-measured on hardware from a fresh install; both transports from the one build via the CodeOnTheGo.nointercept sentinel.

Page	Transport	Socket connections	In-process requests	Load
i/index.html	in-process	0	16 (144,651 bytes)	137 ms
i/index.html	socket	39	0	269 ms
k/html/basic-syntax.html (templated)	in-process	0	4	55 ms
k/html/basic-syntax.html (templated)	socket	6	0	56 ms

Renders are pixel-identical across transports — compare -metric AE returns 0 differing pixels for both pages (status bar cropped, so the clock can't count as a difference). The templated page matters most here: Pebble now renders it in common, so this is the first hardware evidence that the in-process path produces the same page as the socket path rather than merely a page.

The three review fixes, checked individually

• Double load is gone. i/index.html was served exactly once (grep -c "Served 'i/index.html'" = 1). The tripled .js fetches in the log are the frameset's three frames each pulling the same script — normal browser behaviour, not the regression.
• Brotli priming after the move: no UnsatisfiedLinkError, no dictionary-decode failures, no W/E from any documentation class across every run. 144 KB of brotli content decoded and rendered.
• Template cache cleared on swap: after touching the debug database, Swapped to the debug database '/storage/emulated/0/Download/documentation.db' followed by a correct render, and the same 144,651-byte total as the pre-swap run.

Boundaries hold

• /pr/db with interception on: 1 socket connection, 0 in-process. The developer endpoints stay on the socket server, as contentFor intends with its path.startsWith("pr/") decline.
• The sentinel reports itself rather than going quiet: in-process totals so far: in-process serving is off (Download/CodeOnTheGo.nointercept exists).

Accessibility

Verified at font scale 1.3 (the device's own setting) and 2.0: text reflows, nothing clipped or overrun, both system bars intact, content still scrollable. Scale restored afterwards.

What I am not claiming

The 137 ms vs 269 ms gap is one page, one device, one run each, warm-vs-cold not controlled — directionally consistent with removing a loopback socket per request, but not a benchmark. The many-asset comparison this PR's description mentions still has not been measured fairly.

[davidschachterADFA]

Pushed 8da9283 for the Environment.DOC_DB finding.

HelpActivity and IDETooltipWebViewFragment forced DocumentationRequestInterceptor.shared from a property initializer, so the whole lazy ran during construction, before onCreate, on the main thread. Two consequences:

• Environment.DOC_DB is a plain public static File with no initializer, assigned only by Environment.init(). DeviceProtectedApplicationLoader wraps that call in runCatching, and the credential-protected loader returns before it when storage isn't ready — so null is a state the app can genuinely be in, and the non-null parameter turned it into an NPE that killed the activity before it existed. shared is nullable now and declines instead, which puts the request back on the local web server. That's the same thing a null from intercept() already means everywhere else, so the contract is unchanged.
• The lazy also stats external storage for the nointercept sentinel. On the main thread that's a disk read under a detectAll() StrictMode policy, and on a contended FUSE mount it stalls the frame that opens the screen. Both call sites use by lazy now, so it happens on a WebView thread — the way FAQActivity already did it.

Left alone deliberately, from the same review: intercept() still has no throw guard, so an Error — an OOM decoding a large row — escapes onto a Chromium thread instead of falling through to the server the class documents. Worth its own change.

267 app + 99 common tests pass.

One formatting note in case anyone hits it: ktlint and the 140-column limit fight over that shouldInterceptRequest override. As an expression body it gets re-joined onto one 141-column line and spotlessApply then can't fix what it just made, so it's a block body now.

[itsaky-adfa]

Correctness pass over the diff. Five findings, one of them a behavioural regression against stage's WebServer.

[davidschachterADFA]

@itsaky-adfa — all five findings are closed. The verdict is pinned to 8da9283; head is now 8c29578.

#	Finding	Closed by	Where to look
1	switchToDatabase never clears templateCache, so a debug swap keeps rendering the old database's templates	3336ad643	templateCache.clear() now sits between compressionDictionaryStale = true and generation++ — your suggestion applied verbatim
2	cs0 sentinel clears a cache no user-visible page reads	3336ad643	WebServer.kt:566-571 clears both: contentSource.clearTemplateCache() and DocumentationRequestInterceptor.clearSharedTemplateCache()
3	?: return false swallows a genuine NULL blob; /pr/bs answers with zero bytes	3336ad643	Explicit if (json == null) branch that logs and sends a 500, with the group_concat-over-empty-join reason in the comment
4	servedSummary() can be the first touch of the lazy on the main thread	3336ad643	Guarded by documentationLazy.isInitialized(); falls back to "interceptor not yet used" rather than building the interceptor in onPageFinished
5	Stale comment at the serveRequest call site (and the drifted one in WebServerTest)	28d7608b1	Both rewritten to describe the code that is actually there

On #3 — worth recording that your diagnosis of why was the useful part. The platform-type inference (T inferring as ByteArray? because one branch is literally null, so no null check is generated) is what made it invisible, and it is the reason the same shape on stage NPE'd into a 500 while this version wrote nothing at all.

One commit since you looked that is not on your list: 8c29578ca stops caching a missing DOC_DB refusal forever, so a database that appears later is picked up instead of the process staying broken until restart.

Ready for another look.