Closed Bug 1396302 Opened 7 years ago Closed 7 years ago

Crash in nsIWidget::IMENotificationRequestsRef

Categories

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

56 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: philipp, Assigned: masayuki)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is report bp-ebb8a6be-17fb-4c9a-952d-d18140170902. ============================================================= Crashing Thread (0) Frame Module Signature Source 0 xul.dll nsIWidget::IMENotificationRequestsRef() widget/nsBaseWidget.cpp:2361 1 xul.dll mozilla::IMEStateManager::OnChangeFocusInternal(nsPresContext*, nsIContent*, mozilla::widget::InputContextAction) dom/events/IMEStateManager.cpp:489 2 xul.dll nsFocusManager::Blur(nsPIDOMWindowOuter*, nsPIDOMWindowOuter*, bool, bool, nsIContent*) dom/base/nsFocusManager.cpp:1673 3 xul.dll nsFocusManager::WindowLowered(mozIDOMWindowProxy*) dom/base/nsFocusManager.cpp:810 4 xul.dll nsWebShellWindow::WindowDeactivated() xpfe/appshell/nsWebShellWindow.cpp:500 5 xul.dll nsWindow::DispatchFocusToTopLevelWindow(bool) widget/windows/nsWindow.cpp:4641 6 xul.dll nsWindow::ProcessMessage(unsigned int, unsigned __int64&, __int64&, __int64*) widget/windows/nsWindow.cpp:5950 7 xul.dll nsWindow::WindowProcInternal(HWND__*, unsigned int, unsigned __int64, __int64) widget/windows/nsWindow.cpp:4953 8 xul.dll CallWindowProcCrashProtected xpcom/base/nsCrashOnException.cpp:35 9 xul.dll nsWindow::WindowProc(HWND__*, unsigned int, unsigned __int64, __int64) widget/windows/nsWindow.cpp:4905 10 user32.dll UserCallWinProcCheckWow 11 user32.dll DispatchClientMessage 12 user32.dll _fnDWORD 13 ntdll.dll KiUserCallbackDispatch 14 user32.dll ZwUserPeekMessage 15 user32.dll PeekMessageW 16 msctf.dll CThreadInputMgr::PeekMessageW(tagMSG*, HWND__*, unsigned int, unsigned int, unsigned int, int*) 17 msctf.dll CThreadInputMgr::PeekMessageW(tagMSG*, HWND__*, unsigned int, unsigned int, unsigned int, int*) 18 xul.dll mozilla::widget::WinUtils::PeekMessageW(tagMSG*, HWND__*, unsigned int, unsigned int, unsigned int) widget/windows/WinUtils.cpp:713 19 xul.dll nsAppShell::ProcessNextNativeEvent(bool) widget/windows/nsAppShell.cpp:319 20 xul.dll nsBaseAppShell::DoProcessNextNativeEvent(bool) widget/nsBaseAppShell.cpp:140 21 xul.dll nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) widget/nsBaseAppShell.cpp:291 22 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:952 23 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:521 24 xul.dll mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:125 25 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:319 26 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:299 27 xul.dll nsBaseAppShell::Run() widget/nsBaseAppShell.cpp:158 28 xul.dll nsAppShell::Run() widget/windows/nsAppShell.cpp:230 29 xul.dll nsAppStartup::Run() toolkit/components/startup/nsAppStartup.cpp:288 30 xul.dll XREMain::XRE_mainRun() toolkit/xre/nsAppRunner.cpp:4635 31 xul.dll XREMain::XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4799 32 xul.dll XRE_main(int, char** const, mozilla::BootstrapConfig const&) toolkit/xre/nsAppRunner.cpp:4894 33 firefox.exe NS_internal_main(int, char**, char**) browser/app/nsBrowserApp.cpp:309 34 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:115 35 firefox.exe __scrt_common_main_seh f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253 36 kernel32.dll BaseThreadInitThunk 37 ntdll.dll RtlUserThreadStart this crash signature is newly appearing on 57.0a1 and 56.0b5 after the patch for bug 1388647 has landed there.
Masayuki might know the best about this.
Flags: needinfo?(masayuki)
Yeah, looks like that this is simple mistake: https://hg.mozilla.org/mozilla-central/annotate/73e8f351b28f/dom/events/IMEStateManager.cpp#l490 > nsCOMPtr<nsIWidget> oldWidget = sWidget; > nsCOMPtr<nsIWidget> newWidget = > aPresContext ? aPresContext->GetRootWidget() : nullptr; > bool focusActuallyChanging = > (sContent != aContent || sPresContext != aPresContext || > oldWidget != newWidget || sActiveTabParent != newTabParent); > > // If old widget has composition, we may need to commit composition since > // a native IME context is shared on all editors on some widgets or all > // widgets (it depends on platforms). > if (oldWidget && focusActuallyChanging && sTextCompositions) { > RefPtr<TextComposition> composition = > sTextCompositions->GetCompositionFor(oldWidget); > if (composition) { > // However, don't commit the composition if we're being inactivated > // but the composition should be kept even during deactive. > if (aPresContext || > !sFocusedIMEWidget->IMENotificationRequestsRef(). > WantDuringDeactive()) { sFocusedIMEWidget may be nullptr here.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
Comment on attachment 8904170 [details] Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget https://reviewboard.mozilla.org/r/175944/#review180968
Attachment #8904170 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/3dbc8ba3dc09 IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please request Beta approval on this when you get a chance. It grafts cleanly from m-c.
Flags: needinfo?(masayuki)
Comment on attachment 8904170 [details] Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget Approval Request Comment [Feature/Bug causing the regression]: Regression of bug 1388647. [User impact if declined]: I'm still not sure of the STR, but a lot of users meet this crash everyday. [Is this code covered by automated tests?]: No, since not sure the STR. [Has the fix been verified in Nightly?]: The latest build ID is 9/4's. https://crash-stats.mozilla.com/signature/?product=Firefox&signature=nsIWidget%3A%3AIMENotificationRequestsRef&date=%3E%3D2017-08-30T05%3A01%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-build_id&_sort=-date&page=1 [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: No. [Is the change risky?]: No. [Why is the change risky/not risky?]: Just checking right pointer. [String changes made/needed]: No.
Flags: needinfo?(masayuki)
Attachment #8904170 - Flags: approval-mozilla-beta?
Comment on attachment 8904170 [details] Bug 1396302 - IMEStateManager::OnChangeFocusInternal() should check oldWidget's IME notification requests rather than sFocusedIMEWidget Fix for new crash in 56, let's uplift for beta 10.
Attachment #8904170 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: