Closed
Bug 1388647
Opened 7 years ago
Closed 7 years ago
[e10s] Crash in mozilla::ContentCacheInParent::OnEventNeedingAckHandled on macOS
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | fixed |
firefox57 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
(Keywords: crash, inputmethod)
Crash Data
Attachments
(2 files)
(deleted),
text/x-review-board-request
|
m_kato
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
(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-79fe859f-d8b3-4f50-bd5a-a9b8c0170809.
=============================================================
> mozilla::ContentCacheInParent::OnEventNeedingAckHandled(nsIWidget*, mozilla::EventMessage) widget/ContentCache.cpp:1222
> mozilla::dom::TabParent::RecvOnEventNeedingAckHandled(mozilla::EventMessage const&) dom/ipc/TabParent.cpp:1862
> mozilla::dom::PBrowserParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PBrowserParent.cpp:2251
> mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp:2092
> mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) ipc/glue/MessageChannel.cpp:2018
> mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1920
> nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1446
> NS_ProcessPendingEvents(nsIThread*, unsigned int) xpcom/threads/nsThreadUtils.cpp:422
> nsBaseAppShell::NativeEventCallback() widget/nsBaseAppShell.cpp:97
> nsAppShell::ProcessGeckoEvents(void*) widget/cocoa/nsAppShell.mm:409
STR:
1. Open two windows.
2. Start composition with IME in a tab (do not commit the composition).
3. Activate another application, e.g., click on desktop.
4. Click the other window's title bar.
I cannot reproduce this crash with this STR on the other platforms. However, this is similar to bug 1387357.
Assignee | ||
Comment 1•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
I checked the native IME behavior on OS X 10.9 (oldest one which we support) and macOS 10.12 (current).
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8895747 [details]
Bug 1388647 - part1: IMEStateManager::OnChangeFocusInternal() shouldn't request to commit composition with sFocusedIMETabParent
https://reviewboard.mozilla.org/r/166506/#review173206
Attachment #8895747 -
Flags: review?(m_kato) → review+
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8895748 [details]
Bug 1388647 - part2: Make IMEInputHandler of Cocoa widget handle request to commit/cancel composition synchronously
https://reviewboard.mozilla.org/r/166508/#review173208
::: commit-message-c06c1:3
(Diff revision 1)
> +Bug 1388647 - part2: Make IMEInputHandler of Cocoa widget handle request to commit/cancel composition synchronously r?m_kato
> +
> +When Gecko starts to support Cocoa widget, we needed to use NSInputManager.
When Gecko started ...
Attachment #8895748 -
Flags: review?(m_kato) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/8f09ad78575d
part1: IMEStateManager::OnChangeFocusInternal() shouldn't request to commit composition with sFocusedIMETabParent r=m_kato
https://hg.mozilla.org/integration/autoland/rev/91b531826640
part2: Make IMEInputHandler of Cocoa widget handle request to commit/cancel composition synchronously r=m_kato
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8f09ad78575d
https://hg.mozilla.org/mozilla-central/rev/91b531826640
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 11•7 years ago
|
||
Looks like this affects 56 as well. Should we request uplift to Beta?
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(masayuki)
Assignee | ||
Comment 12•7 years ago
|
||
Comment on attachment 8895747 [details]
Bug 1388647 - part1: IMEStateManager::OnChangeFocusInternal() shouldn't request to commit composition with sFocusedIMETabParent
Approval Request Comment
[Feature/Bug causing the regression]:
Regression of e10s mode (no occurs without e10s mode).
[User impact if declined]:
If user switches windows of Firefox when there is IME composition, composition events are handled in wrong TabParent and crashes with MOZ_RELEASE_ASSERT.
[Is this code covered by automated tests?]:
No.
[Has the fix been verified in Nightly?]:
Yes.
[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?]:
Making IMEStateManager::OnChangeFocusInternal() request IME to commit composition only when there is composition and:
* focus is moving to another element or
* all windows are being inactivated and active IME doesn't want us to keep composition during deactive.
The old behavior is, in the latter case, it puts off to request commit composition until a window is being activated later. The reason is ancient Cocoa API limitation.
[String changes made/needed]:
No.
Flags: needinfo?(masayuki)
Attachment #8895747 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8895748 [details]
Bug 1388647 - part2: Make IMEInputHandler of Cocoa widget handle request to commit/cancel composition synchronously
Approval Request Comment
[Is this code covered by automated tests?]:
No.
[Has the fix been verified in Nightly?]:
Yes.
[Needs manual test from QE? If yes, steps to reproduce]:
No.
[List of other uplifts needed for the feature/fix]:
The "part1".
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
This makes the behavior of REQUEST_IME_TO_COMMIT_COMPOSITION on macOS same as the other platforms. Due to the ancient Cocoa API limitation, we couldn't commit composition when window is being inactivated. However, currently, it's possible. Therefore, this patch removes the focus check and async commit request handlers which are for the cases if the request is posted during deactive.
So, even though this patch changes a lot of lines, but the changes are very simple.
[String changes made/needed]:
No.
Attachment #8895748 -
Flags: approval-mozilla-beta?
Comment on attachment 8895747 [details]
Bug 1388647 - part1: IMEStateManager::OnChangeFocusInternal() shouldn't request to commit composition with sFocusedIMETabParent
Crash fix, let's take this for beta 5.
Attachment #8895747 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8895748 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•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
•