Last Comment Bug 489045 - nsXULWindow::GetParentNativeWindow crashes if GetParentWidget returns null
: nsXULWindow::GetParentNativeWindow crashes if GetParentWidget returns null
Status: RESOLVED FIXED
[ccbr]
: crash
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.9.3a1
Assigned To: Markus Stange [:mstange] [away until June 27]
:
Mentors:
Depends on: 522843
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-18 21:44 PDT by Lingfeng.Guan
Modified: 2009-10-16 20:35 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
null check (874 bytes, patch)
2009-10-05 20:54 PDT, Markus Stange [:mstange] [away until June 27]
roc: review+
Details | Diff | Review

Description Lingfeng.Guan 2009-04-18 21:44:46 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:1.9.0.8) Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:1.9.0.8) Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729)

Call nsIBaseWindow->GetParentNativeWindow() causes crash.
the crash report is here:
http://crash-stats.mozilla.com/report/index/8d49f73c-3ee9-449a-b5b7-82c9a2090418?p=1

quote Mook's analysis:
"http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsXULWindow.cpp#716 can return null, but http://mxr.mozilla.org/mozilla-central/source/xpfe/appshell/src/nsXULWindow.cpp?mark=737,739#731 assumes it won't"

Reproducible: Always

Steps to Reproduce:
1. QueryInterface to get a nsIBaseWindow from the window object.(the window is not main Window, and with attribute hidechrome="true")
2.call nsIBaseWindow->GetParentNativeWindow to get HWND 
3.
Actual Results:  
Firefox crashed. I used MSVC to debug and get these
First-chance exception at 0x103d98b6 in firefox.exe: 0xC0000005: Access violation reading location 0x00000000.
Unhandled exception at 0x103d98b6 in firefox.exe: 0xC0000005: Access violation reading location 0x00000000.
First-chance exception at 0x103d98b6 in firefox.exe: 0xC0000005: Access violation reading location 0x00000000.
Unhandled exception at 0x103d98b6 in firefox.exe: 0xC0000005: Access violation reading location 0x00000000. 
happened

Expected Results:  
should return the HWND of the window
Comment 1 Markus Stange [:mstange] [away until June 27] 2009-09-19 23:20:55 PDT
Looks like GetParentNativeWindow is unused in Firefox, so maybe we could just remove it. What's mediacenter-com.dll?
Comment 2 Lingfeng.Guan 2009-09-20 19:51:58 PDT
Please do not do that:) I'm using it right now. And Mook's minimizeToTray is using that too. Right now it is the best way to get HWND for a window.
mediacenter-com.dll is a component in my extension.
Btw: As well as GetParentNativeWindow, I suggest that GetNativeWindow is added to the interface, so that when I want the HWND of the window, I do not have to find it's child first.
Comment 3 Markus Stange [:mstange] [away until June 27] 2009-09-20 19:54:10 PDT
Would only GetNativeWindow be enough for your purposes, i.e. is there a different quick way to get the parent window?
Comment 4 Lingfeng.Guan 2009-09-20 21:04:21 PDT
There is another way, but by that way, I have to use nsIWidget as well, and there for have to download couple of .idl and .h files of several unfrozen interfaces, which are not included in the Gecko-SDK, and could be changed anytime.
By this way, I only need nsIBaseWindow, although it's also unfrozen, but it's only one interface, which is much much better.
In fact, except for the bug, this is currently to best way to do what I want, and that's why I added it to the MDC, see below.
https://developer.mozilla.org/en/Code_snippets/Finding_Window_Handles
Comment 5 Markus Stange [:mstange] [away until June 27] 2009-09-23 20:42:49 PDT
> Btw: As well as GetParentNativeWindow, I suggest that GetNativeWindow is added
> to the interface, so that when I want the HWND of the window, I do not have to
> find it's child first.

For what reasons do you need the HWND of the child window? We'll get rid of all HWNDs except for the top level window one at some point, so I don't want to encourage using such an API now.
Comment 6 Markus Stange [:mstange] [away until June 27] 2009-10-05 20:54:46 PDT
Created attachment 404766 [details] [diff] [review]
null check

I haven't been able to figure out how to invoke GetParentNativeWindow from a js-only test. window.[...].getInterface(Ci.nsIBaseWindow).parentNativeWindow doesn't work.
Comment 7 Lingfeng.Guan 2009-10-06 10:33:54 PDT
I did not invoke that from js, if I recall correctly, seems like you cannot get a void * in js.
Comment 8 Lingfeng.Guan 2009-10-06 10:37:14 PDT
(In reply to comment #5)
> > Btw: As well as GetParentNativeWindow, I suggest that GetNativeWindow is added
> > to the interface, so that when I want the HWND of the window, I do not have to
> > find it's child first.
> 
> For what reasons do you need the HWND of the child window? We'll get rid of all
> HWNDs except for the top level window one at some point, so I don't want to
> encourage using such an API now.

I don't need the HWND of the child window right now, and even if I do, I think give me the parent HWND should be enough, as long as I can tell the differences between the children.
Comment 9 Markus Stange [:mstange] [away until June 27] 2009-10-09 00:24:07 PDT
http://hg.mozilla.org/mozilla-central/rev/5bac33739a83
Comment 10 :Mook 2009-10-10 09:01:17 PDT
Note that this will silently return garbage (more precisely, not clear aParentNativeWindow but return NS_OK) where things used to crash.
Comment 11 Jesse Ruderman 2009-10-16 20:35:12 PDT
Filed bug 522843 on that.

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