Closed Bug 1368554 Opened 9 years ago Closed 9 years ago

Crash in mozilla::ContentCacheInParent::OnCompositionEvent

Categories

(Core :: Widget: Win32, defect, P2)

Product:
Core
Component:
Widget: Win32
Version:
52 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: philipp, Assigned: masayuki)

References

Details

(Keywords: crash, inputmethod, regression, Whiteboard: tpi:+)

Crash Data

Attachments

(2 files)

Bug 1368554 ContentCacheInParent::mPendingCompositionCount should be decreased when TextCompositin which has dispatched composition events to corresponding remote process
59 bytes, text/x-review-board-request
m_kato
: review+
Details
Patch for ESR52
12.70 KB, patch
Details | Diff | Splinter Review
This bug was filed from the Socorro interface and is report bp-da39123c-cfce-48b9-a95c-b79650170521. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll mozilla::ContentCacheInParent::OnCompositionEvent(mozilla::WidgetCompositionEvent const&) widget/ContentCache.cpp:1106 1 xul.dll mozilla::dom::TabParent::SendCompositionEvent(mozilla::WidgetCompositionEvent&) dom/ipc/TabParent.cpp:2051 2 xul.dll mozilla::TextComposition::DispatchCompositionEvent(mozilla::WidgetCompositionEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, bool) dom/events/TextComposition.cpp:252 3 xul.dll mozilla::IMEStateManager::DispatchCompositionEvent(nsINode*, nsPresContext*, mozilla::WidgetCompositionEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, bool) dom/events/IMEStateManager.cpp:1171 4 xul.dll mozilla::PresShell::DispatchEventToDOM(mozilla::WidgetEvent*, nsEventStatus*, nsPresShellEventCB*) layout/base/PresShell.cpp:8179 5 xul.dll mozilla::PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) layout/base/PresShell.cpp:8056 6 xul.dll mozilla::PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) layout/base/PresShell.cpp:7758 7 xul.dll nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) view/nsViewManager.cpp:814 8 xul.dll nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) view/nsView.cpp:1125 9 xul.dll nsWindow::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) widget/windows/nsWindow.cpp:4102 10 xul.dll mozilla::widget::TextEventDispatcher::DispatchEvent(nsIWidget*, mozilla::WidgetGUIEvent&, nsEventStatus&) widget/TextEventDispatcher.cpp:186 11 xul.dll mozilla::widget::TextEventDispatcher::StartComposition(nsEventStatus&, mozilla::WidgetEventTime const*) widget/TextEventDispatcher.cpp:237 12 xul.dll mozilla::widget::TSFTextStore::FlushPendingActions() widget/windows/TSFTextStore.cpp:1775 13 xul.dll mozilla::widget::TSFTextStore::DidLockGranted() widget/windows/TSFTextStore.cpp:1685 14 xul.dll mozilla::widget::TSFTextStore::RequestLock(unsigned long, long*) widget/windows/TSFTextStore.cpp:1639 15 msctf.dll CInputContext::_QueueItem(_TS_QUEUE_ITEM*, int, long*) 16 msctf.dll CInputContext::RequestEditSession(unsigned long, ITfEditSession*, unsigned long, long*) 17 imetip.dll CTipFnEditSessionHandler::Invoke(__MIDL___MIDL_itf_tipfunc_0000_0009_0001, __MIDL___MIDL_itf_tipfunc_0000_0009_0002, ITipEditSessionProcedure*, IUnknown*, long*) 18 imetip.dll CTipFnCompose::OnApplyComposition(CTipComposition*, int) 19 imetip.dll CTipComposition::ApplyToContext(int) 20 imetip.dll CTipFnProductStringHandler::UpdateComposition(int) 21 imetip.dll CTipFnProductStringHandler::_OnProductObjectChanged(unsigned long, void*) 22 imetip.dll Imeapiutil::CImeReconversionEndNotify::OnReconversionEnded(__MIDL___MIDL_itf_imeapi_0000_0043_0001) 23 imjkapi.dll CImeProductObject_JK::EndUpdateProductObject(int) 24 imjpapi.dll CImeIPointCallBack::GenerateMessage() 25 imjpapi.dll CIImeIPoint::UpdateContext(int) 26 imjpapi.dll CCmdUpdateContext::ExecuteWithParam(IImeEMManager*, __int64) 27 imjpapi.dll CImeEMManager::Execute(_GUID const*) 28 imjpapi.dll CImeCommonAPI_JPN_Desktop_V1::EndBulkChange(int) 29 imjpapi.dll CImeKeyEventHandler_JPN_Desktop_V1::ProcessKey(__MIDL___MIDL_itf_imeapi_0000_0001_0001 const*, unsigned short) 30 imjpapi.dll CImeKeyEventHandler_JPN_Desktop_V1::OnKeyDown(__MIDL___MIDL_itf_imeapi_0000_0001_0001 const*, int*) 31 imetip.dll CTipFnKeyEventHandler::OnKeyDown(unsigned __int64, __int64, int*) 32 imetip.dll CTipContextEditorMgr::OnKeyboardEvent(unsigned int, ITfContext*, unsigned __int64, __int64, int*) 33 imetip.dll CTipContextEditorMgr::_OnKeyboardEvent(unsigned int, ITfContext*, unsigned __int64, __int64, int*, void*) 34 imetip.dll Tsfutil::CTfKeyEventSink::OnKeyDown(ITfContext*, unsigned __int64, __int64, int*) 35 msctf.dll CThreadInputMgr::_CallKeyEventSink(unsigned long, CInputContext*, KSEnum, unsigned __int64, __int64, int*) 36 msctf.dll CheckKnownModuleOnWinlogonDesktop() 37 msctf.dll CThreadInputMgr::KeyDown(unsigned __int64, __int64, int*) 38 xul.dll mozilla::widget::TSFTextStore::ProcessRawKeyMessage(tagMSG const&) widget/windows/TSFTextStore.cpp:5878 39 xul.dll nsAppShell::ProcessNextNativeEvent(bool) widget/windows/nsAppShell.cpp:369 40 xul.dll nsBaseAppShell::DoProcessNextNativeEvent(bool) widget/nsBaseAppShell.cpp:138 41 xul.dll nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) widget/nsBaseAppShell.cpp:289 42 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1213 43 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/glue/nsThreadUtils.cpp:390 44 xul.dll mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:124 45 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:231 46 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:211 47 xul.dll nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:156 48 xul.dll nsAppShell::Run() widget/windows/nsAppShell.cpp:262 49 xul.dll nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:283 50 xul.dll XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:4477 51 xul.dll XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4654 52 xul.dll XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4745 53 firefox.exe NS_internal_main(int, char**, char**) browser/app/nsBrowserApp.cpp:305 54 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:115 55 firefox.exe __scrt_common_main_seh f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253 Ø 56 kernel32.dll kernel32.dll@0x159cc Ø 57 ntdll.dll ntdll.dll@0x2a560 Ø 58 kernel32.dll kernel32.dll@0x9baaf Ø 59 kernel32.dll kernel32.dll@0x9baaf this cross-platform crash is appearing in firefox 52 and subsequent versions (next to en-us mostly in ja, zh & ko locales) with MOZ_RELEASE_ASSERT(mPendingCompositionCount < 0xffui8) that got added in bug 1304620.
not bad crash volume, something todo with text composition. Masayuki, care to take a look?
Flags: needinfo?(masayuki)
Priority: -- → P2
Whiteboard: tpi:+
Oh, mPendingCompositionCount reaches 255... it means that the main process dispatched 255 composition events were sent to a remote process but they are not set back yet. If it's true, the child process is really stopped. However, I bet that this is caused by other bug. Perhaps, mPendingCompositionCount isn't reset in some cases or decreased from 0?
Flags: needinfo?(masayuki)
Keywords: inputmethod
Ah, I see that when composition is committed forcibly in a remote process, it sends a request to commit to the parent process. However, the parent process won't send composition events to the remote process. Therefore, mContentCacheInParent in TabParent doesn't have a chance to reset mPendingCompositionCount.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Comment on attachment 8876327 [details] Bug 1368554 ContentCacheInParent::mPendingCompositionCount should be decreased when TextCompositin which has dispatched composition events to corresponding remote process https://reviewboard.mozilla.org/r/147660/#review152222
Attachment #8876327 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/401127e8ba3b ContentCacheInParent::mPendingCompositionCount should be decreased when TextCompositin which has dispatched composition events to corresponding remote process r=m_kato
https://hg.mozilla.org/mozilla-central/rev/401127e8ba3b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Is this worth nominating for ESR52 uplift or should we let it ride the trains?
status-firefox-esr52: --- → affected
status-thunderbird_esr52: affected → ---
Flags: needinfo?(masayuki)
I think that the crash volume isn't so high but this might be reproduced a lot for some specific users. If you keep open a tab which supports autocomplete during composition during a browser session and you often use such input field with IME, you will meat this crash after autocomplete commits composition forcibly 255 times. I don't think we need to fix this bug urgently due to the rate, but I think that we should take this to 52ESR: https://crash-stats.mozilla.com/signature/?product=Firefox&version=52.0esr&version=52.0.1esr&version=52.0.2esr&version=52.1.0esr&version=52.1.1esr&version=52.1.2esr&version=52.2.0esr&signature=mozilla%3A%3AContentCacheInParent%3A%3AOnCompositionEvent&date=%3E%3D2016-12-14T05%3A13%3A16.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports I'll check if they can be uplifted cleanly.
Flags: needinfo?(masayuki)
Attached patch Patch for ESR52 — — Splinter Review
# Unfortunately, the patch conflicts a line in logging code. So, actual code isn't touched in this patch. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixing a crash bug of e10s. User impact if declined: If user or web page commits IME composition forcibly (e.g., clicking somewhere during composition or web page commits composition for autocomplete), TabParent's counter of number of composition won't be decreased. The counter is uint8_t. Therefore, when the counter reaches 254 and user tries to use IME, then, the main process crashes itself. So, this can be problem if a tab is opened a long time. E.g., some users may meet this bug in Gmail. Fix Landed on Version: 55 Risk to taking this patch (and alternatives if risky): Not so risky. The counter was managed with composition events (when TabParent sends compositionstart, incremented and when composiitonend, decremented). This patch makes that TextComposition class which creates for every composition notify TabParent. Therefore, TabParent can decrement the counter in any cases. String or UUID changes made by this patch: No.
Attachment #8880010 - Flags: approval-mozilla-esr52?
Comment on attachment 8880010 [details] [diff] [review] Patch for ESR52 Oops, I found a regression of this. This may cause setting current TextComposition instance to old composition's start offset. So, this patch is really buggy only when the child process is really slow.
Attachment #8880010 - Flags: approval-mozilla-esr52?
Is this something you still were hoping to land on ESR52 or should we wontfix?
Flags: needinfo?(masayuki)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #15) > Is this something you still were hoping to land on ESR52 or should we > wontfix? I think that we should take only part1 of bug 1376424 instead.
Flags: needinfo?(masayuki)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: