Crash when delayed keypress event is dispatched in e10s

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

See bug 1316330 comment 24.

Hitting 'MOZ_ASSERT(mNativeKeyCommandsValid)' in PuppetWidget::ExecuteNativeKeyBinding.

Actually, the same issue was found in Bug 993714, but it was fixed only for synthetic events. By doing the same for delayed events solves the issue. I'll provide a patch and see if it's okay.
Jessica, I know you are working on this, so assign to you. Thanks for taking this one.
Assignee: nobody → jjong
Posted patch patch, v1. (obsolete) — Splinter Review
Hi Olli, this is for fixing the crash. If the native key bindings list is no longer available when the delayed event is dispatched, we request it again.
Attachment #8816007 - Flags: review?(bugs)
Comment on attachment 8816007 [details] [diff] [review]
patch, v1.

So this gives a new meaning to mIsSuppressedOrDelayed. Atm it is set only for events which are suppressed or delayed so that later a new similar event is dispatched. But this sets the flag on those events which will be dispatched afterwards. At least document this new meaning in BasicEvents.h
Attachment #8816007 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8816007 [details] [diff] [review]
> patch, v1.
> 
> So this gives a new meaning to mIsSuppressedOrDelayed. Atm it is set only
> for events which are suppressed or delayed so that later a new similar event
> is dispatched. But this sets the flag on those events which will be
> dispatched afterwards. At least document this new meaning in BasicEvents.h

Right, I will document this in BasicEvents.h. Thanks.
Posted patch patch, v2. (obsolete) — Splinter Review
Added more documentation for mIsSuppressedOrDelayed in BasicEvents.h. Carrying r+.
Attachment #8816007 - Attachment is obsolete: true
Attachment #8816347 - Flags: review+
needs rebasing

applying bug-132145-crash-nativekeybindings.patch
unable to find 'layout/base/nsPresShell.cpp' for patching
(use '--prefix' to apply patch relative to the current directory)
1 out of 1 hunks FAILED -- saving rejects to file layout/base/nsPresShell.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug-132145-crash-nativekeybindings.patch
Flags: needinfo?(jjong)
Keywords: checkin-needed
Attachment #8816347 - Attachment is obsolete: true
Attachment #8816400 - Flags: review+
Flags: needinfo?(jjong)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0055028cd1f
Request native key bindings for delayed keypress events. r=smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d0055028cd1f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.