fix(windows): stop blocking the platform thread on an unresponsive browser process - #9
fix(windows): stop blocking the platform thread on an unresponsive browser process#9palmoni5 wants to merge 2 commits into
Conversation
Y-PLONI
left a comment
There was a problem hiding this comment.
The added flag is valid, but it is applied to the wrong HWND for the claimed root cause. ICoreWebView2Controller get_ParentWindow returns the application-provided parent HWND, initially the HWND passed to CreateCoreWebView2Controller or CreateCoreWebView2CompositionController, not a Chrome_WidgetWin_1 owned by the WebView2 browser process: https://learn.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2controller?view=webview2-1.0.3650.58
In this code path, InAppWebViewManager createInAppWebView creates that HWND via CreateWindowEx and passes it to CreateCoreWebView2CompositionController. Therefore the get_ParentWindow call here returns the app-owned input parent, contradicting the new comment.
SWP_ASYNCWINDOWPOS only posts when caller and target owner use different input queues: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowpos. On the normal platform-thread path, it is consequently expected to have no effect and cannot solve a wait on the browser UI thread. If it does become asynchronous in another configuration, it also changes completion timing, so the claim of no behavior change is not established.
Please first identify the actual blocking HWND and call, for example by logging class name, PID, and GetWindowThreadProcessId with a stack trace, and then apply a fix to that target. The current change should be removed until that is demonstrated.
7c380fb to
a65f358
Compare
|
You're right on both counts, and instead of arguing I instrumented the code and measured. Full methodology and numbers below. Methodology
Results1. The HWND is exactly what you said. With the browser suspended:
2. Nothing in this path blocks, even against a fully suspended browser process. All measured while
All of these are posted asynchronously by the WebView2 client DLL; none can stall the platform thread regardless of how slow the browser process wakes. 3. Input stays responsive too. Clicks injected over the webview area and over the Flutter chrome while the browser was suspended arrived with 4. I also tried adding ConclusionThe claimed root cause is refuted: with the plugin's resume path exercised against a maximally unresponsive browser process, the host platform thread never blocks and input latency stays at zero. There is no blocking HWND or call in this code path to fix, so I'm withdrawing the change — the branch was rebased on current master but the flag change should not merge. Whatever causes Otzaria/otzaria#882 on the reporter's machine, it isn't this repositioning call; the investigation moves back to the app level with these findings. |
…owser process put_IsVisible and Resume are synchronous IPC served by the WebView2 browser process UI thread. pause(), resume() and setHostWindowMinimized() issue them on the Flutter platform thread, so while that browser thread is not pumping - suspended, starved, or slow to wake after the app spent time in the background - the entire host UI freezes for as long as the browser takes to respond. Measured with scope probes: put_IsVisible held the platform thread for the full 198.5s a suspended browser process was frozen, and 5-6s against a starved one, while every other call on these paths (put_Bounds, SetWindowPos, put_RasterizationScale) posts asynchronously and returns in under a millisecond regardless. Visibility flips and Resume are now delivered only after a WM_NULL SendMessageTimeout probe (50ms) confirms the browser UI thread responds; otherwise the desired state stays folded into WebViewVisibilityState and a 250ms timer retries. Deferred transitions coalesce, TrySuspend keeps its contract of running only after the hide was actually delivered, and a resume cancels a pending suspend. With a responsive browser - the normal case - behavior is unchanged apart from the probe round-trip. Addresses the freeze class of Otzaria/otzaria#882: returning to the app with a plugin tab open after the browser was backgrounded, then doing anything that pauses the plugin, or restoring from minimize.
a65f358 to
9fdf523
Compare
|
Update: kept digging with the same instrumentation and found the call that does block — |
Review follow-up. The per-webview probe was itself a serialized cost: with N webviews on one unresponsive browser, each retry round could block the platform thread N times x 50ms. The probe verdict and the retry timer now live in a per-browser-pid registry: within one 250ms interval at most one WM_NULL probe (<=50ms) runs per browser process, and every webview waiting on that process is drained together once it answers. The registry logic is pure and injected (probe/clock/timer), covered by eight new gtests including multiple waiters sharing one probe and independent browser processes. get_BrowserProcessId is no longer on the per-flip path either: the pid is queried once (measured 0.03-0.04ms, client-side), cached, and refreshed only after COREWEBVIEW2_PROCESS_FAILED_KIND_BROWSER_PROCESS_ EXITED. Runtime check with a fully suspended browser process: the app stays responsive through minimize, 10s frozen, and restore-while-frozen, with exactly one ~60ms probe per 250ms tick for the whole process.
|
All four points addressed in 1820475: 1. Serialized per-webview probes (the regression). The probe verdict and the retry timer now live in a per-browser-pid registry ( 2. WM_NULL is a proxy, and the window pick is a heuristic — agreed. I want to be precise about what the gate does and doesn't promise: it is a best-effort optimization gate, not a correctness invariant. A false positive (window answers but the IPC still blocks) degrades to exactly master's current behavior — the unconditionally blocking call; a false negative delays a flip by one 250ms tick. So correctness never depends on the proxy. Empirically the proxy tracked the IPC in every regime tested: fully suspended → probe times out and the flip is deferred (vs. 198.5s blocked on master); 5%-duty-cycle → probe passes only inside a resume window and the subsequent 3. 4. Tests. The registry takes its probe, clock and retry scheduling as injected functions, so the coalescing logic is covered directly by eight new gtests: multiple waiters sharing a single probe per interval, one probe per retry tick, drain-on-answer (and probe-free re-entry on the cached verdict), independent browser pids, waiter removal mid-wait, empty-queue timer cancellation without probing, and invalidation forcing a fresh probe. What remains uncovered is the thin Win32 glue itself (SetTimer/EnumWindows/SendMessageTimeout), which I don't see a way to gtest meaningfully — that part is validated by the runtime scenarios above. One honest observation from the re-validation runs: in one run out of several, with the browser process fully suspended, the platform thread hung during minimize before any plugin code ran (none of the added instrumentation fired until the process was resumed). So under a completely frozen browser there exists at least one more synchronous dependency outside this plugin's code paths — likely engine/compositor-level. It did not reproduce in the other runs, full suspension is an artificial amplifier, and it's independent of this PR either way, but I'd rather you know it exists. |
Problem
Otzaria/otzaria#882 — after returning to the app from another program with a plugin tab open, the entire UI ignores clicks for a noticeable delay.
The actual blocking call — measured
ICoreWebView2Controller::put_IsVisible(and by the same channel,Resume) is a synchronous IPC served by the WebView2 browser process UI thread. It is issued on the Flutter platform thread from three paths:pause()/resume()— the host app suspends/resumes a webview (in Otzaria: every tab switch away from / to a plugin tab; ships in 0.9.96),setHostWindowMinimized()— per webview on every minimize/restore.While the browser process UI thread is not pumping messages — suspended, starved, or slow to wake after the app spent time in the background — this call holds the platform thread for as long as the browser takes to respond, freezing all input and rendering of the host app. Scope-probe measurements (QPC timers around every native call in these paths, message-loop dispatch timing, and input-queue latency from
msg.time):put_IsVisibleNtSuspendProcess)¹
put_Bounds,put_RasterizationScale,surface->put_Size,SetWindowPos,get_ParentWindow,NotifyParentWindowPositionChanged— all posted asynchronously, unaffected by the browser being frozen.In the fully-suspended scenario the whole app hung: even an external
ShowWindowcall against the window blocked for the entire 3m20s and released the instant the browser process was resumed.This maps onto the pichillilorenzo#882 report: the user returns to the app (browser process still sluggish after >10s in the background), clicks something outside the plugin — a different tab, the nav rail — and the first thing the app does is pause the plugin:
put_IsVisible(false)then stalls the platform thread until the browser wakes.Fix
Visibility flips and
Resumeare delivered only after a cheapSendMessageTimeout(WM_NULL, …, 50ms)probe confirms the browser UI thread is pumping. Otherwise the desired state stays folded intoWebViewVisibilityStateand a 250 ms timer retries. The probe verdict and the retry timer are shared per browser process (browser_process_gate.h): within one interval at most one ≤50ms probe runs per browser process no matter how many webviews wait on it, and all of them are drained together once it answers:TrySuspendkeeps its documented contract of running only after the hide was actually delivered; aresume()cancels a pending suspend.Worst-case platform-thread cost while a flip is pending against an unresponsive browser: one 50 ms probe per 250 ms tick, instead of an unbounded stall.
Validation
put_IsVisible, all input dead.WebViewVisibilityStatedelivery bookkeeping and the shared gate's coalescing logic (injected probe/clock/timer) are covered by eleven new gtests; the full native suite (24 tests) passes viactest, and the example builds clean.Known limits
Chrome_WidgetWin*top-level window as a proxy for the thread that serves the IPC; it is a best-effort gate, not a correctness invariant — a false positive degrades to master's current (blocking) behavior, a false negative delays a flip by one 250ms tick. If no window is found the code behaves exactly as before.BROWSER_PROCESS_EXITED.webViewController->Close()in the destructor is likely the same class of synchronous call (closing a plugin tab against a throttled browser); left out of scope here.🤖 Generated with Claude Code