Closed
Bug 1396302
Opened 7 years ago
Closed 7 years ago
Crash in nsIWidget::IMENotificationRequestsRef
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
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)
(deleted),
text/x-review-board-request
|
m_kato
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
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.
Assignee | ||
Comment 2•7 years ago
|
||
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)
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
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
Comment 6•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 7•7 years ago
|
||
Please request Beta approval on this when you get a chance. It grafts cleanly from m-c.
Flags: needinfo?(masayuki)
Assignee | ||
Comment 8•7 years ago
|
||
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+
Comment 10•7 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•