Retire the sync-dangerous WebView2 call in PublishHelper via an OffScreenBrowser

[JohnThomson]

PublishHelper.RemoveUnwantedContent (the epub/bloompub "which elements are
visible, what fonts are used" pass) got its element/font info via
Browser.RunJavascriptWithStringResult_Sync_Dangerous, and navigated via
NavigateAndWaitTillDone. Both wait by pumping the MAIN UI message loop
(Application.DoEvents), which lets unrelated user commands run in the middle of
our call stack -- the reentrancy blamed for BL-12614 / BL-13120.

Introduce OffScreenBrowser: a WebView2 browser for off-screen work (navigate to
a page and run javascript against a real, laid-out DOM -- CSS, fonts, and layout
actually resolved -- without putting the browser on screen or letting the user
interact with it). It owns a real WebView2Browser on its own private, dedicated
STA thread with its own Windows Forms message loop. Because THAT thread services
the browser's callbacks, a caller can simply BLOCK for a result: it never pumps
the main loop (no reentrancy) and never deadlocks (the thread that blocks is
never the thread that must pump). Navigation is async (awaits the completion
event; no DoEvents). Nothing about it is specific to page checks -- it suits any
task that needs to ask a real browser questions about a document off-screen.

PublishHelper now drives this browser through blocking Navigate/RunJavascript
calls, so the sync-dangerous script call and the DoEvents-pumping navigation are
both gone from that path. Since the browser owns its own thread, the work no
longer needs to run on the UI thread, so:

• The ControlForInvoke.Invoke wrapper and UI-thread Debug assert are removed, and
  RemoveUnwantedContent is merged with its former *Internal helper into one method.
• The whole vestigial ControlForInvoke plumbing that existed only to hand the page
  checks a control to Invoke on is deleted: the properties on PublishHelper and
  EpubMaker, the propagation in PublishEpubApi.SetupEpubControlContent and
  PublishView, the static on BloomPubMaker, and the control the CLI created solely
  to pass down. PublishEpubApi.ControlForInvoke is kept -- it is independently used
  to run the save-as dialog on the UI thread.

Validated against the existing epub and bloompub publish suites (which drive
RemoveUnwantedContent against real books with real fonts -- also the BL-15292 risk
area): they pass with no test changes, so no behavior change slipped through. A
small mechanics test (OffScreenBrowserTests) covers the dedicated-thread pattern.

BookProcessor still uses RunJavascriptWithStringResult_Sync_Dangerous (for polling
window globals during its off-screen per-page fix-up); migrating it to
OffScreenBrowser and then deleting the obsolete base-class method is a follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Devin review

---

This change is 

[greptile-apps]

Greptile Summary

This PR eliminates the reentrancy hazard in PublishHelper.RemoveUnwantedContent (epub/bloompub "page checks") by replacing the DoEvents-pumping NavigateAndWaitTillDone / RunJavascriptWithStringResult_Sync_Dangerous pair with a new OffScreenBrowser that owns a private STA thread and WinForms message loop. Because that thread — not the caller — services the WebView2 callbacks, callers can simply block for results without ever pumping the main UI loop, removing the reentrancy root-cause of BL-12614 / BL-13120.

• OffScreenBrowser (new): starts a dedicated STA thread, posts browser initialization and readiness polling onto it, then exposes blocking Navigate and RunJavascript calls that marshal async work to the browser thread and block the caller; Dispose posts teardown via try/finally (so ExitThread always runs even if _browser.Dispose throws) and joins with a 5 s timeout.
• PublishHelper: ControlForInvoke, the Invoke wrapper in RemoveUnwantedContent, and the old Browser-based BrowserForPageChecks are all removed; a lazily-created OffScreenBrowser (PageChecksBrowser) takes their place; a new ReleaseBrowser() method lets EpubMaker free the browser thread promptly once staging is done.
• The entire ControlForInvoke propagation chain (EpubMaker, BloomPubMaker, PublishEpubApi, PublishView, CreateArtifactsCommand) is cleaned up; PublishEpubApi.ControlForInvoke is retained because it independently drives the save-as dialog on the UI thread.

Important Files Changed

Filename	Overview
src/BloomExe/Publish/OffScreenBrowser.cs	New class: a WebView2 browser on its own dedicated STA thread with a private WinForms message loop, providing blocking Navigate/RunJavascript calls that are safe because the caller never pumps the main UI loop. The try/finally in Dispose (previously flagged) is correctly in place.
src/BloomExe/Publish/PublishHelper.cs	Replaced old Browser/ControlForInvoke plumbing with a lazily-created OffScreenBrowser; merged RemoveUnwantedContent and its Internal helper into one method; added ReleaseBrowser() for explicit early teardown. Navigation and JS calls are now safe blocking calls.
src/BloomExe/Publish/Epub/EpubMaker.cs	Removed ControlForInvoke property forwarding to PublishHelper; added a ReleaseBrowser() call at the end of StageEpub so the off-screen browser's thread and WebView2 process are freed promptly after staging completes.
src/BloomExe/Publish/Epub/PublishEpubApi.cs	Removed the ControlForInvoke assignment to EpubMaker in SetupEpubControlContent; PublishEpubApi.ControlForInvoke is retained for its unrelated save-dialog use.
src/BloomExe/Publish/BloomPub/BloomPubMaker.cs	Removed ControlForInvoke static property and its propagation to PublishHelper; no functional change to the publish path beyond eliminating now-unneeded UI-thread marshalling.
src/BloomExe/CLI/CreateArtifactsCommand.cs	Removed the dummy Control that was created solely to give EpubMaker a ControlForInvoke; the CLI path no longer needs any UI-thread handle for epub creation.
src/BloomExe/Publish/PublishView.cs	Removed the BloomPubMaker.ControlForInvoke assignment on tab activation; one-line deletion with no functional impact.
src/BloomTests/Publish/OffScreenBrowserTests.cs	New mechanics test: proves the browser runs on a dedicated thread and that a blocking RunJavascript call from the test thread completes without deadlock. Marked SkipOnTeamCity because it needs a real WebView2; full navigation coverage is provided by the existing publish suites.
src/BloomExe/Book/BookProcessor.cs	Comment-only change: updated the XML doc reference from the old BrowserForPageChecks name to the new PageChecksBrowser name.

_{Reviews (3): Last reviewed commit: "Retire the sync-dangerous WebView2 call ..." | Re-trigger Greptile}

[hatton]

[Claude Opus 4.8] Resource-cleanup gap on the failure path — this is the init-timeout leak your file overview flagged, and it's worth an inline fix too. When _startupError is set (e.g. the 20s IsReadyToNavigate timeout), the dedicated thread is still running Application.Run and _browser (with its CoreWebView2 child process) is still alive, but the constructor throws without tearing any of it down — so the thread and the WebView2 process leak, and repeated failures accumulate them. Calling Dispose() before throwing reuses the existing teardown (it posts browser-dispose + ExitThread to the still-pumping loop, then joins):

Suggested change

	if (_startupError != null)
	throw new ApplicationException(
	"OffScreenBrowser failed to initialize",
	_startupError
	);
	if (_startupError != null)
	{
	// Initialization failed (e.g. the WebView2 readiness timeout). The dedicated thread may still be
	// pumping its message loop with a live WebView2 (and its CoreWebView2 process) attached, so tear
	// that down before we throw; otherwise the thread and the browser process leak.
	Dispose();
	throw new ApplicationException(
	"OffScreenBrowser failed to initialize",
	_startupError
	);
	}

[JohnThomson]

Done.

[hatton]

[Claude Opus 4.8] Preflight review notes (analysis only — no commits made).

Two robustness suggestions on OffScreenBrowser (both error-path resource cleanup, non-blocking):

• Constructor leaks the dedicated thread + WebView2 process when init times out — inline suggestion above.
• Dispose() should wrap _browser.Dispose() in try/finally so ExitThread() always runs — endorsed Greptile's inline suggestion.

Verified clean (no action needed):

• No orphaned ControlForInvoke references after the plumbing removal — the remaining ones (PublishEpubApi for the save-as dialog, and the unrelated SpreadsheetImporter) are intentional and still compile.
• Merging RemoveUnwantedContent with its former *Internal helper reordered parameters, but both call sites (EpubMaker.WriteEpubMetadata→RemoveUnwantedContent and BloomPubMaker) match the merged public signature — no positional-argument regression.

Coverage caveat for the reviewer: the GitHub checks here (Greptile, pr-automation) do not exercise the new C# behavior. OffScreenBrowserTests is [Category("SkipOnTeamCity")], and the epub/bloompub publish suites that drive RemoveUnwantedContent need a real WebView2 + BloomServer, so they don't run on CI either. The safety net for this change is the local publish-suite run (per the commit message) plus Devin/human review — not the green checkmarks. Worth a manual publish-an-epub-and-a-bloompub smoke test before merge, since the core risk is whether WebView2 lays out CSS/fonts correctly on the dedicated STA thread rather than the UI thread.

[JohnThomson]

@JohnThomson made 2 comments and resolved 1 discussion.
Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on hatton and JohnThomson).

[JohnThomson]

Done.