Skip to content

Rework BookProcessor to use OffScreenBrowser (BL-16430)#8038

Open
JohnThomson wants to merge 2 commits into
masterfrom
retireRunJavascriptSync2
Open

Rework BookProcessor to use OffScreenBrowser (BL-16430)#8038
JohnThomson wants to merge 2 commits into
masterfrom
retireRunJavascriptSync2

Conversation

@JohnThomson

@JohnThomson JohnThomson commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Allows us to finally retire RunJavascriptWithStringResult_Sync_Dangerous.

Devin review


This change is Reviewable

Allows us to finally retire RunJavascriptWithStringResult_Sync_Dangerous.
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR migrates BookProcessor from driving a per-page WebView2Browser on the UI thread (using the dangerous RunJavascriptWithStringResult_Sync_Dangerous / Application.DoEvents loop) to driving a single OffScreenBrowser on its own dedicated STA thread, enabling the job to run entirely on a Task.Run background thread and keeping the UI responsive throughout.

  • OffScreenBrowser is extended with StartFreshBrowser() (fresh renderer, warm shared environment between pages), NavigateWithoutWaitingForLoad(), and RunJavascriptFireAndForget(), all safely serialized through RunAndBlock.
  • ExternalApi.HandleProcessBook switches from BeginInvoke-on-UI-thread to Task.Run; WinForms state reconciliation is marshaled back via a new InvokeOnUiThread helper; _processBookInProgress is marked volatile to handle the now-cross-thread false write.
  • RunJavascriptWithStringResult_Sync_Dangerous is fully removed from WebView2Browser and the Browser base contract, along with its Obsolete annotation.

Important Files Changed

Filename Overview
src/BloomExe/Book/BookProcessor.cs Reworked to use OffScreenBrowser instead of creating per-page WebView2Browser controls on the UI thread; removes the Debug.Assert UI-thread check, switches to blocking calls, and makes WaitForJavascriptResult internal (enabling unit tests). Logic is correct.
src/BloomExe/Publish/OffScreenBrowser.cs Gains StartFreshBrowser(), NavigateWithoutWaitingForLoad(), RunJavascriptFireAndForget(), a shared CoreWebView2Environment field, and CreateWithInjectedEnvironment plumbing; the dedicated-STA-thread model is preserved and the new code serializes correctly through RunAndBlock.
src/BloomExe/web/controllers/ExternalApi.cs Switches RunProcessBookJob from BeginInvoke-on-UI-thread to Task.Run on a background thread; adds InvokeOnUiThread helper for WinForms state updates; marks _processBookInProgress volatile to handle the now-cross-thread false write. Previous reviewer concern addressed.
src/BloomExe/WebView2Browser.cs Removes RunJavascriptWithStringResult_Sync_Dangerous and the obsolete annotation; adds CreateWithInjectedEnvironment factory and CoreEnvironment property to support OffScreenBrowser's shared-environment approach.
src/BloomExe/IBrowser.cs Removes the obsolete RunJavascriptWithStringResult_Sync_Dangerous abstract member; RunJavascriptFireAndForget already existed in this contract and is unchanged.
src/BloomTests/Book/BookProcessorTests.cs New test class covering WaitForJavascriptResult (page-load value, delayed fire-and-forget value, timeout), and StartFreshBrowser isolation; marked SkipOnTeamCity as the tests require a real WebView2 environment.

Reviews (2): Last reviewed commit: "Post-review fixes (BL-16430 part C)" | Re-trigger Greptile

@hatton

hatton commented Jul 7, 2026

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants