1589497 - [TSF] Crash in [@ mozilla::widget::NativeKey::InitWithKeyOrChar] because of `TSFTextStore` trying to dispatch keyboard event with unexpected message

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[[:philipp]]

This bug is for crash report bp-dbcd6cf0-6ea1-4afe-9d9f-f90410191017.

Top 10 frames of crashing thread:

0 xul.dll void mozilla::widget::NativeKey::InitWithKeyOrChar widget/windows/KeyboardLayout.cpp:1564
1 xul.dll mozilla::widget::NativeKey::NativeKey widget/windows/KeyboardLayout.cpp:1304
2 xul.dll void mozilla::widget::TSFTextStore::DispatchKeyboardEventAsProcessedByIME widget/windows/TSFTextStore.cpp:2684
3 xul.dll void mozilla::widget::TSFTextStore::FlushPendingActions widget/windows/TSFTextStore.cpp:2348
4 xul.dll HRESULT mozilla::widget::TSFTextStore::RequestLock widget/windows/TSFTextStore.cpp
5 msctf.dll CInputContext::OnLayoutChange 
6 msctf.dll CACPWrap::OnLayoutChange 
7 xul.dll mozilla::widget::TSFTextStore::NotifyTSFOfLayoutChange widget/windows/TSFTextStore.cpp:6252
8 xul.dll mozilla::widget::TSFTextStore::OnLayoutChangeInternal widget/windows/TSFTextStore.cpp:6200
9 xul.dll mozilla::widget::IMEHandler::NotifyIME widget/windows/WinIMEHandler.cpp:336

this is a long-standing but low volume crash signature with reports containing MOZ_CRASH(Unsupported message) and 95% affecting users of chinese locale builds.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This is unexpected case. While TSFTextStore is handling a key press, it shouldn't receive notifications from child process. And that should be impossible. Actually, the stack tells us that it's truly kicked by another event loop. So, I guess that TSFTextStore failed to clear pending actions at previous key handling.

philipp: Can you check whether there are useful comments about the STR? I cannot check it with my permission.

Comment 2

[[:philipp]]

i'm afraid the current comments don't point to any obvious STR (or the meaning is lost to me in google translation) - some of the more helpful extracts:

• bp-1a4d94ca-b268-4b3b-b5cc-1e6e10190817: 只能打开首页，打开其他网页的时候就崩溃
• bp-8080ebc8-8054-48a0-8b0c-3eed50190422: 就是正常打字，没打几个就崩溃了 赶紧解决呀，再不解决就换浏览器了 ，用崩溃
• bp-49edddb1-06e9-4127-90f4-ef39d0190606: 打文字的时候，总是自动闪退
• bp-54cd427c-290c-4989-a5ff-bf5010190720: 搜狗输入法打字经常崩溃
• bp-16a50ea0-0423-482e-977d-52fd90190524: 输入权利二字。。。。直接自动关闭

crashing urls seem to be just popular pages in the locale:
1 https://home.firefoxchina.cn/ 31 5.18 %
2 https://offlintab.firefoxchina.cn/ 26 4.34 %
3 http://iot.ddsaas.cn/ 20 3.34 %
4 http://pmp.ddsaas.cn/ 13 2.17 %
5 https://www.baidu.com/ 13 2.17 %
7 https://hao.360.com/ 9 1.50 %
8 https://mail.google.com/mail/u/0/ 9 1.50 %
9 https://wx.qq.com/?&lang=zh_CN 8 1.34 %
10 https://wx.qq.com/ 7 1.17 %
11 about:newtab

wading though a number of reports, LenovoTSF.ime 1.0.1.0 is the most common IME in use in these reports, though it's not the only one showing up in there.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Moving all open keyboard/IME handling bugs to DOM: UI Events & Focus Handling component.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1589497 - Make PendingAction of TSFStore copy of key message r=m_kato!

TSFTextStore::sHandlingKeyMsg refers pointer of struct, but referred via
TSFTextStore::PendingAction so that we should make it has a copy of
sHandlingKeyMsg because of for async handling.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46bf06a42ba6
Make PendingAction of TSFStore copy of key message r=m_kato

Comment 6

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/46bf06a42ba6