881504 - crash in mozilla::a11y::DocManager::RemoveListeners

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Scoobidiver (away)]

This bug tracks crashes not fixed by bug 849496.

Signature 	mozilla::a11y::DocManager::RemoveListeners(nsIDocument*) More Reports Search
UUID	e5ccbe3e-23d7-4d33-af58-fbff12130610
Date Processed	2013-06-10 19:55:19
Uptime	777
Last Crash	13.0 minutes before submission
Install Age	3.5 hours since version was first installed.
Install Time	2013-06-10 08:23:51
Product	Firefox
Version	24.0a1
Build ID	20130610031147
Release Channel	nightly
OS	Windows NT
OS Version	6.2.9200
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 58 stepping 9
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 	
AdapterVendorID: 0x8086, AdapterDeviceID: 0x0166, AdapterSubsysID: 00000000, AdapterDriverVersion: 9.17.10.2828
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ WebGL? EGL? EGL+ GL Context? GL Context+ WebGL+ 
Processor Notes 	sp-processor07_phx1_mozilla_com_3023:2012
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x0166
Total Virtual Memory	4294836224
Available Virtual Memory	3379986432
System Memory Use Percentage	63
Available Page File	4687994880
Available Physical Memory	1515302912
Accessibility	Active

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::a11y::DocManager::RemoveListeners 	accessible/src/base/DocManager.cpp:358
1 	xul.dll 	mozilla::a11y::DocManager::NotifyOfDocumentShutdown 	obj-firefox/dist/include/mozilla/a11y/DocManager.h:68
2 	xul.dll 	mozilla::a11y::DocAccessible::Shutdown 	accessible/src/generic/DocAccessible.cpp:630
3 	xul.dll 	mozilla::a11y::DocAccessible::Shutdown 	accessible/src/generic/DocAccessible.cpp:612
4 	xul.dll 	mozilla::a11y::DocManager::HandleEvent 	accessible/src/base/DocManager.cpp:285
5 	xul.dll 	nsEventListenerManager::HandleEventSubType 	content/events/src/nsEventListenerManager.cpp:937
6 	xul.dll 	nsEventListenerManager::HandleEventInternal 	content/events/src/nsEventListenerManager.cpp:1009
7 	xul.dll 	nsEventListenerManager::HandleEvent 	content/events/src/nsEventListenerManager.h:328
8 	xul.dll 	nsEventTargetChainItem::HandleEvent 	content/events/src/nsEventDispatcher.cpp:203
9 	xul.dll 	nsEventTargetChainItem::HandleEventTargetChain 	content/events/src/nsEventDispatcher.cpp:302
10 	xul.dll 	nsEventDispatcher::Dispatch 	content/events/src/nsEventDispatcher.cpp:635
11 	xul.dll 	nsEventDispatcher::DispatchDOMEvent 	content/events/src/nsEventDispatcher.cpp:693
12 	xul.dll 	nsDocument::DispatchPageTransition 	content/base/src/nsDocument.cpp:7994
13 	xul.dll 	nsDocument::OnPageHide 	content/base/src/nsDocument.cpp:8118
14 	xul.dll 	nsDocumentViewer::PageHide 	layout/base/nsDocumentViewer.cpp:1261
15 	xul.dll 	nsDocShell::FirePageHideNotification 	docshell/base/nsDocShell.cpp:1623
16 	xul.dll 	nsDocShell::Destroy 	docshell/base/nsDocShell.cpp:4909
17 	xul.dll 	nsXULWindow::Destroy 	xpfe/appshell/src/nsXULWindow.cpp:474
18 	xul.dll 	nsWebShellWindow::Destroy 	xpfe/appshell/src/nsWebShellWindow.cpp:758
19 	xul.dll 	nsWebShellWindow::RequestWindowClose 	xpfe/appshell/src/nsWebShellWindow.cpp:315
20 	xul.dll 	nsWindow::ProcessMessage 	widget/windows/nsWindow.cpp:4701
21 	xul.dll 	nsWindow::WindowProcInternal 	widget/windows/nsWindow.cpp:4323
22 	xul.dll 	CallWindowProcCrashProtected 	xpcom/base/nsCrashOnException.cpp:32
23 	xul.dll 	nsWindow::WindowProc 	widget/windows/nsWindow.cpp:4275
24 	user32.dll 	InternalCallWinProc 	
...

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Aa11y%3A%3ADocManager%3A%3ARemoveListeners%28nsIDocument*%29

Comment 1

[alexander :surkov (:asurkov)]

Null check again?

Is it ok that when nsDocument::OnPageHide triggers then

nsPIDOMWindow* window = aDocument->GetWindow();
EventTarget* target = window->GetChromeEventHandler();
nsEventListenerManager* elm = target->GetListenerManager(true);

elm is null?

(stack crash: https://crash-stats.mozilla.com/report/index/e5ccbe3e-23d7-4d33-af58-fbff12130610)

Boris, do you know?

Comment 2

[Boris Zbarsky [:bzbarsky]]

No idea, but I bet smaug would!

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

That crash stat says GetChromeEventHandler() returns null, and as the Get* prefix hints, it can return
null. And it is not surprising that it can be null in this kind of case.

Comment 4

[poiru]

Attachment: First patch

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 766249 [details] [diff] [review]
First patch

Review of attachment 766249 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/DocManager.cpp
@@ +356,5 @@
>  
>    EventTarget* target = window->GetChromeEventHandler();
>    nsEventListenerManager* elm = target->GetListenerManager(true);
> +  if (!elm)
> +    return;

it seems you need to check target instead

Comment 6

[poiru]

Attachment: Patch v2

(In reply to alexander :surkov from comment #5)
> it seems you need to check target instead

Ah, I must have missed Olli's comment. Updated patch.

Comment 7

[poiru]

Attachment: Patch v3

Comment 8

[poiru]

Attachment: Patch v4

Well, this is embarrassing. Seems like I uploaded the wrong patch previously. Fixed now.

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 767239 [details] [diff] [review]
Patch v4

Review of attachment 767239 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks!

Comment 10

[Marco Zehe (:MarcoZ)]

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc6b2673c2a9

Thank you for the patch!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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