557931 - Crash [@ mozilla::widget::WindowHook::Lookup] with createTaskbarTabPreview

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase, uses enhanced privileges

I was trying to use the nsIWinTaskbar service, but I get a crash when using it in the way provided with the testcase.

http://crash-stats.mozilla.com/report/index/15ae59d0-4523-4759-974a-63e7f2100407
0  	xul.dll  	mozilla::widget::WindowHook::Lookup  	 widget/src/windows/WindowHook.cpp:99
1 	xul.dll 	mozilla::widget::WindowHook::LookupOrCreate 	widget/src/windows/WindowHook.cpp:109
2 	xul.dll 	mozilla::widget::WindowHook::AddMonitor 	widget/src/windows/WindowHook.cpp:77
3 	xul.dll 	mozilla::widget::TaskbarPreview::TaskbarPreview 	widget/src/windows/TaskbarPreview.cpp:107
4 	xul.dll 	mozilla::widget::TaskbarTabPreview::TaskbarTabPreview 	widget/src/windows/TaskbarTabPreview.cpp:62
5 	xul.dll 	mozilla::widget::WinTaskbar::CreateTaskbarTabPreview 	widget/src/windows/WinTaskbar.cpp:250
6 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
7 	xul.dll 	XPCWrappedNative::CallMethod 	js/src/xpconnect/src/xpcwrappednative.cpp:2728
8 	xul.dll 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1770
9 	mozjs.dll 	js_Invoke 	js/src/jsinterp.cpp:1364
10 	mozjs.dll 	js_Interpret 	js/src/jsops.cpp:2277
11 	mozjs.dll 	js_Invoke 	js/src/jsinterp.cpp:1372
12 	mozjs.dll 	js_InternalInvoke 	js/src/jsinterp.cpp:1429
13 	mozjs.dll 	JS_CallFunctionValue 	js/src/jsapi.cpp:4957
14 	xul.dll 	nsJSContext::CallEventHandler 	dom/base/nsJSEnvironment.cpp:2161
15 	xul.dll 	nsJSEventListener::HandleEvent 	dom/src/events/nsJSEventListener.cpp:228
16 	xul.dll 	nsEventListenerManager::HandleEventSubType 	content/events/src/nsEventListenerManager.cpp:1082
17 	xul.dll 	nsEventListenerManager::HandleEvent 	content/events/src/nsEventListenerManager.cpp:1198
18 	xul.dll 	nsEventTargetChainItem::HandleEvent 	content/events/src/nsEventDispatcher.cpp:201
19 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:326
20 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:604
21 	xul.dll 	DocumentViewerImpl::LoadComplete 	layout/base/nsDocumentViewer.cpp:1027
22 	xul.dll 	nsDocShell::EndPageLoad 	docshell/base/nsDocShell.cpp:5746
23 	xul.dll 	nsCOMPtr_base::assign_from_qi 	obj-firefox/xpcom/build/nsCOMPtr.cpp:96
24 	xul.dll 	nsDocShell::OnStateChange 	docshell/base/nsDocShell.cpp:5624

Comment 1

[Jim Mathies [:jimm]]

GetHWNDFromDocShell for the doc shell is failing, returning a null hwnd, which in turn retrieves a null nsWindow object which we invoke in GetWindowHook().

We should add a null check in CreateTaskbarTabPreview on toplevelHWND, and fail if that fails.

Comment 2

[Jim Mathies [:jimm]]

(In reply to comment #1)
> GetHWNDFromDocShell for the doc shell is failing, returning a null hwnd, which
> in turn retrieves a null nsWindow object which we invoke in GetWindowHook().
> 
> We should add a null check in CreateTaskbarTabPreview on toplevelHWND, and fail
> if that fails.

We return NULL in some cases from GetHWNDFromDocShell if we can't QI the nsIBaseWindow. Patch coming up.

Comment 3

[Jim Mathies [:jimm]]

Attachment: fix

GetAncestor doesn't seem to mind the null value for the hwnd, so this should suffice.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 437707 [details] [diff] [review]
fix

we need this in GetTaskbarWindowPreview as well.

Comment 5

[Jim Mathies [:jimm]]

Attachment: fix

Somewhat overkill since GetNSWindowPtr should return null for a null hwnd, but just to be safe.

Comment 6

[Jim Mathies [:jimm]]

http://hg.mozilla.org/mozilla-central/rev/549329a134b8

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 437709 [details] [diff] [review]
fix

This crash exists in 1.9.2 as well. See bug 618240. Requesting blocking, it's a simple no risk patch.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 437709 [details] [diff] [review]
fix

Approved for 1.9.2.15, a=dveditz for release-drivers

Comment 9

[Brandon Sterne (:bsterne)]

Comment on attachment 437709 [details] [diff] [review]
fix

Moving the flag to the correct release.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 437709 [details] [diff] [review]
fix

Didn't make the code-freeze for non-blockers, you can try again next time.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 437709 [details] [diff] [review]
fix

Please land this for 1.9.2.18

Comment 12

[Daniel Veditz [:dveditz]]

Not going to track this one. If it makes it today it makes it, if not that's fine too.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 437709 [details] [diff] [review]
fix

Didn't make 3.6.18