← All reports

REGRESSION(310826@main): "Zhuyin - Traditional" input method stalls for multiple seconds

Component: WebKit UIProcess (macOS) | 2b95292

WebKit on macOS routes keyboard events through NSTextInputClient's handleEventByInputMethod:, which dispatches asynchronously to system input methods over XPC. A prior fix (297270@main) introduced m_interpretKeyEventHoldingTank to enforce DOM event ordering — a keydown must reach the web process before any compositionstart/update events the IM emits, as editors like Google Docs require — by serializing all keyboard events behind any in-flight IM call.

Source/WebKit/UIProcess/mac/WebViewImpl.h

- std::optional<Vector<WebCore::KeypressCommand>> m_collectedKeypressCommands;
std::optional<NSRange> m_stagedMarkedRange;
- Vector<Function<void()>> m_interpretKeyEventHoldingTank;
+ Deque<Vector<WebCore::KeypressCommand>> m_collectedKeypressCommands;
+ Deque<Function<void()>> m_interpretKeyEventHoldingTank;

Source/WebKit/UIProcess/mac/WebViewImpl.mm

- if (m_collectedKeypressCommands) {
+ if ([event type] == NSEventTypeKeyDown)
+ m_collectedKeypressCommands.append(Vector<WebCore::KeypressCommand> { });
+ else if (!m_collectedKeypressCommands.isEmpty()) {
m_interpretKeyEventHoldingTank.append([weakThis = WeakPtr { *this }, capturedEvent = retainPtr(event), capturedBlock = makeBlockPtr(completionHandler)] {
CheckedPtr checkedThis = weakThis.get();
- if (!checkedThis)
+ if (!checkedThis) {
capturedBlock(NO, { });
- else
- checkedThis->interpretKeyEvent(capturedEvent.get(), capturedBlock.get());
+ return;
+ }
+ RetainPtr inputContext { checkedThis->inputContext() };
+ [inputContext handleEventByInputMethod:capturedEvent completionHandler:[weakThis, capturedEvent, capturedBlock](BOOL handled) mutable {
+ if (!weakThis.get()) { capturedBlock(NO, { }); return; }
+ capturedBlock(handled, { });
+ }];
});
return;
}
- m_collectedKeypressCommands = Vector<WebCore::KeypressCommand> { };

TCIM breaks under the serialized model because it uses its XPC queue depth as a liveness signal: an empty queue lets its main thread drift into background work and stall for seconds. The fix makes keydowns bypass the holding tank entirely so the IM's runloop is fed continuously, while keyups stay held until the paired keydown's completion fires. The single optional command vector becomes a Deque of per-keydown queues, with the front slot always corresponding to the keydown the IM is currently processing.

Before:
  KeyDown N        KeyDown N+1         KeyUp N
      │                 │                  │
      ├──► IM (XPC)     ├──► HoldingTank   ├──► HoldingTank
      │    in-flight    │    (TCIM XPC     │    (waiting)
      │                 │     queue empty, │
      │                 │     stalls)      │
      └── completes ────┴──────────────────┴─── drain tank (burst)

After:
  KeyDown N        KeyDown N+1         KeyUp N
      │                 │                  │
      ├──► IM (XPC)     ├──► IM (XPC)      ├──► HoldingTank
      │    Deque[0]     │    Deque[1]      │    (waiting)
      │    (front)      │    (back)        │
      └── N completes ──┘                  └── released by N's completion
                                               └──► handleEventByInputMethod: (bypass tank)

Multiple keydowns can now be simultaneously in flight to the input method, replacing a flat holding tank with a per-keydown Deque on a pipeline that bridges the UI process to system input methods and the web process. The keydown→keyup IPC ordering guarantee to the web process is preserved, but it now rests on a Deque front-slot association rather than on strict serialization.

Narrow: the front-of-deque invariant — "the front is always the keydown the IM is currently processing" — is an assumption about IM thread serialization, not something WebKit enforces. A misbehaving or attacker-controlled input method (a malicious IME installed system-wide) that fires doCommandBySelector:/insertText:/setMarkedText: callbacks out of order corrupts the association and misroutes commands from one keystroke into another's queue. The tell is a takeFirst() on a queue whose ordering is guaranteed only by an external component.

Wider: dual-path population of one Deque. collectKeyboardLayoutCommandsForEvent uses prepend while the IM path uses append, with the prepend explicitly designed to route callbacks into the synchronous path's queue even when other IM-driven keys are in flight at the back. Two writers with opposite insertion ends into a shared ordered structure is a pattern worth sweeping generally — audit other Deque members in WebViewImpl and WKContentView for mixed prepend/append writers and check the interleaving cases.

// collectKeyboardLayoutCommandsForEvent: prepend (synchronous path, must be front)
m_collectedKeypressCommands.prepend(Vector<WebCore::KeypressCommand> { });
// ...
auto commands = m_collectedKeypressCommands.takeFirst();

// interpretKeyEvent IM path: append (concurrent keydowns go to back)
if ([event type] == NSEventTypeKeyDown)
    m_collectedKeypressCommands.append(Vector<WebCore::KeypressCommand> { });

Widest: re-entrancy through synchronous completion handlers. The released-keyup path dispatches handleEventByInputMethod: with an inline completion that never drains the Deque. Synchronous completion is valid Cocoa behavior, so the next keydown's completion handler can fire while the keyup's inline block is still on the stack, and the drain logic may not account for that nesting. This is the same shape as the WKRevealItemPresenter UAF above — audit any Cocoa completion handler assumed to be asynchronous. Related: every WeakPtrCheckedPtr dereference in these async lambdas is a teardown-during-composition surface that existed before but is now exercised by more concurrent paths.