489045 - nsXULWindow::GetParentNativeWindow crashes if GetParentWidget returns null

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Lingfeng.Guan]

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]]

Looks like GetParentNativeWindow is unused in Firefox, so maybe we could just remove it. What's mediacenter-com.dll?

Comment 2

[Lingfeng.Guan]

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]]

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]

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]]

> 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]]

Attachment: 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]

I did not invoke that from js, if I recall correctly, seems like you cannot get a void * in js.

Comment 8

[Lingfeng.Guan]

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

http://hg.mozilla.org/mozilla-central/rev/5bac33739a83

Comment 10

[:Mook]

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]

Filed bug 522843 on that.