Last Comment Bug 783139 - Crash [@ nsIView::GetViewFor]
: Crash [@ nsIView::GetViewFor]
Status: RESOLVED FIXED
: crash
Product: Core
Classification: Components
Component: Widget: Gonk (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- critical (vote)
: mozilla17
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
Mentors:
Depends on:
Blocks: 743975
  Show dependency treegraph
 
Reported: 2012-08-15 17:20 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-16 06:18 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Restore setting .widget on nsGUIEvent (848 bytes, patch)
2012-08-15 17:28 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
bugzilla: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 17:20:14 PDT
(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 2012-08-15 17:28:46 PDT
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.
Comment 2 Doug Sherk (:drs) (inactive) 2012-08-15 17:50:21 PDT
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 2012-08-15 17:53:30 PDT
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) 2012-08-15 17:54:04 PDT
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 2012-08-15 17:55:36 PDT
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] 2012-08-15 18:20:56 PDT
Urph, the removal of that line was obviously not correct. r=me too.
Comment 7 Ed Morley [:emorley] 2012-08-16 06:18:25 PDT
https://hg.mozilla.org/mozilla-central/rev/ac8287aea1fb

Note You need to log in before you can comment on or make changes to this bug.