Crash [@ nsIView::GetViewFor]

RESOLVED FIXED in mozilla17

Status

()

Core
Widget: Gonk
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cjones, Assigned: cjones)

Tracking

({crash})

Trunk
mozilla17
ARM
Gonk (Firefox OS)
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

(gdb) bt
#0  0x40ed8b06 in nsIView::GetViewFor (aWidget=0x0) at /home/cjones/mozilla/inbound/view/src/nsView.cpp:137
#1  0x40ed8656 in nsView::GetViewFor (aWidget=0x0) at /home/cjones/mozilla/inbound/view/src/nsView.h:80
#2  0x40edaeaa in nsView::HandleEvent (this=0x1495640, aEvent=0xbe8f4fc0, aUseAttachedEvents=false) at /home/cjones/mozilla/inbound/view/src/nsView.cpp:1054
#3  0x41784b74 in nsWindow::DispatchEvent (this=0x14956a0, aEvent=0xbe8f4fc0, aStatus=@0xbe8f4fa4) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:483
#4  0x41784644 in nsWindow::DispatchInputEvent (aEvent=...) at /home/cjones/mozilla/inbound/widget/gonk/nsWindow.cpp:288
#5  0x41780b9c in sendTouchEvent (data=...) at /home/cjones/mozilla/inbound/widget/gonk/nsAppShell.cpp:193
#6  0x417812d0 in GeckoInputDispatcher::dispatchOnce (this=0x1448868) at /home/cjones/mozilla/inbound/widget/gonk/nsAppShell.cpp:350
#7  0x41781efc in nsAppShell::ProcessNextNativeEvent (this=0x10a42d0, mayWait=false) at /home/cjones/mozilla/inbound/widget/gonk/nsAppShell.cpp:595

mUseAttachedEvents is false in nsWindow, so we go looking for a view for the widget, but the widget isn't set in the nsGUIEvent so we can't find one.
Created attachment 652285 [details] [diff] [review]
Restore setting .widget on nsGUIEvent

We assert in nsView.cpp because this is null, so I think this patch is correct.
Assignee: nobody → jones.chris.g
Attachment #652285 - Flags: review?(mwu)
Comment on attachment 652285 [details] [diff] [review]
Restore setting .widget on nsGUIEvent

Review of attachment 652285 [details] [diff] [review]:
-----------------------------------------------------------------

*puts on his mwu mask*

::: widget/gonk/nsWindow.cpp
@@ +284,5 @@
>  
>      gFocusedWindow->UserActivity();
>  
>      nsEventStatus status;
> +    aEvent.widget = gFocusedWindow;

Why not null check first? It seems like we're only doing this to target the focused window if there's no other widget. If new code is added which uses this, will it not have its widget overwritten?
It doesn't make sense to dispatch an input event without a focused widget.  This is restoring a line of code that was removed.  If it's null we can't do anything with the event and want to crash so we know we're in error.
Comment on attachment 652285 [details] [diff] [review]
Restore setting .widget on nsGUIEvent

Review of attachment 652285 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Chris Jones [:cjones] [:warhammer] from comment #3)
> It doesn't make sense to dispatch an input event without a focused widget. 
> This is restoring a line of code that was removed.  If it's null we can't do
> anything with the event and want to crash so we know we're in error.

Ok
Attachment #652285 - Flags: review?(mwu) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8287aea1fb

Neil, had to push this as an emergency fix; let me know if this isn't correct.

Comment 6

5 years ago
Urph, the removal of that line was obviously not correct. r=me too.

Updated

5 years ago
Crash Signature: [@ nsIView::GetViewFor]
Keywords: crash

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ac8287aea1fb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.