Refactor JsMessageProcessor Factory#272
Conversation
The global processor factory could only capture app-wide scope, so the AppMessageProcessor was made a shared singleton with its page pushed in via a setter. Each page load overwrote that callback (last-writer-wins), so a setUserAgentMode message from one tab could mutate another tab's web view. Make the factory per-tab and composed at two sites along the module graph: TopazMain supplies app-domain builders (Bluetooth, logging, keyboard), and AppModel merges in page-coupled builders that capture the freshly-created page. AppMessageProcessor now receives an AppMessageHost at construction (bridged to WebPageModel by WebPageAppMessageHost in the App module) and is built fresh per page, so the shared mutable callback and the race are both gone. Builders still construct per JS context, so Bluetooth's reset-on-cross-origin behavior is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
VirtualKeyboardModel was a shared singleton, so detaching an old web view's handler could clobber the newly-attached web view's keyboard state. The reset-on-default was wedged into Coordinator.attachNewHandler as a workaround for that cross-instance race. Now that AppModel can build page-coupled processors, give each page its own VirtualKeyboardModel and add a page-domain VirtualKeyboard builder alongside AppMessage. The processor's didDetach reset is re-enabled (safe per-page) and the Coordinator workaround is removed. enableDebugLogging is threaded through AppModel so both page-coupled processors keep their debug logging. Co-authored-by: Cursor <cursoragent@cursor.com> Cleanup a bit
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9b018bc. Configure here.
| await self.detachOldHandlerAndWait(from: webView) | ||
| self.contextId = newContextId | ||
| self.attachNewHandler(to: webView) | ||
| } |
There was a problem hiding this comment.
Cross-origin handler Task races
High Severity
Cross-origin JS handler teardown and re-attach run inside an unstructured Task, so work continues after didInitiateNavigation returns. Overlapping cross-origin navigations can finish out of order and restore a stale contextId with mismatched handlers, the page can load before handlers are registered, and a pending task can call attachNewHandler after deinitialize has started tearing the web view down.
Reviewed by Cursor Bugbot for commit 9b018bc. Configure here.
There was a problem hiding this comment.
This is a subtle edge case that we haven't seen manifest. The proper fix requires moving the context swap to occur at a different navigation point so it can be awaited. Will address this in a follow up PR.
2 participants
Refactor, primarily to relocate the
TopazScriptHandleradded in #266 asAppMessage, and secondarily to fix the issue with the life-cycle race condition.Closes #269
Note
Medium Risk
Changes the WebView JS bridge composition and navigation teardown ordering; mistakes could affect BLE context lifetime, keyboard overlay state, or user-agent switching across tabs.
Overview
Refactors how native JS message handlers are wired: app-wide builders (Bluetooth, logger) stay in
TopazMain/AppModel, while per-tab handlers (topaz/AppMessage, virtual keyboard) are merged when eachWebPageModelis built. That replaces the separateTopazScriptHandlerwith anAppMessagemodule and routessetUserAgentModethroughAppMessageProcessor→WebPageAppMessageHost→WebPageModel.Virtual keyboard state is no longer global (
VirtualKeyboardModel.sharedremoved); each tab gets its own model injected intoVirtualKeyboard, and detach resetsoverlaysContenton that instance—addressing cross-webview teardown races without resetting another tab’s keyboard inCoordinator.Shared JsMessage plumbing adds
DispatchingJsMessageProcessor, generic action decoding,JsMessageLog, and common decode errors;VirtualKeyboardadopts that pattern. On cross-origin navigation, processor detach is awaited before attaching a new JS context (detachProcessorsAndWait), sodidDetachfinishes before the next page’s handlers run.Reviewed by Cursor Bugbot for commit 9b018bc. Configure here.