783139 - Crash [@ nsIView::GetViewFor]

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(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.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Restore setting .widget on nsGUIEvent

We assert in nsView.cpp because this is null, so I think this patch is correct.

Comment 2

[Doug Sherk (:drs) (inactive)]

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?

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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 4

[Doug Sherk (:drs) (inactive)]

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

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

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

[Michael Wu [:mwu]]

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

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ac8287aea1fb