Last Comment Bug 881504 - crash in mozilla::a11y::DocManager::RemoveListeners
: crash in mozilla::a11y::DocManager::RemoveListeners
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: crash, regression
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: 22 Branch
: All Windows 7
: -- critical (vote)
: mozilla25
Assigned To: Birunthan Mohanathas [:poiru]
:
Mentors:
Depends on:
Blocks: 846204
  Show dependency treegraph
 
Reported: 2013-06-10 15:21 PDT by Scoobidiver (away)
Modified: 2013-06-26 13:42 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
affected
affected
affected


Attachments
First patch (986 bytes, patch)
2013-06-22 00:35 PDT, Birunthan Mohanathas [:poiru]
no flags Details | Diff | Splinter Review
Patch v2 (1016 bytes, patch)
2013-06-25 08:10 PDT, Birunthan Mohanathas [:poiru]
no flags Details | Diff | Splinter Review
Patch v3 (4.46 KB, text/plain)
2013-06-25 08:20 PDT, Birunthan Mohanathas [:poiru]
no flags Details
Patch v4 (1015 bytes, patch)
2013-06-25 08:22 PDT, Birunthan Mohanathas [:poiru]
surkov.alexander: review+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2013-06-10 15:21:00 PDT
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 2013-06-10 17:46:44 PDT
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 [:bz] 2013-06-11 20:09:43 PDT
No idea, but I bet smaug would!
Comment 3 Olli Pettay [:smaug] (vacation Aug 25-28) 2013-06-12 01:46:20 PDT
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 Birunthan Mohanathas [:poiru] 2013-06-22 00:35:47 PDT
Created attachment 766249 [details] [diff] [review]
First patch
Comment 5 alexander :surkov 2013-06-25 04:24:22 PDT
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 Birunthan Mohanathas [:poiru] 2013-06-25 08:10:12 PDT
Created attachment 767230 [details] [diff] [review]
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 Birunthan Mohanathas [:poiru] 2013-06-25 08:20:13 PDT
Created attachment 767236 [details]
Patch v3
Comment 8 Birunthan Mohanathas [:poiru] 2013-06-25 08:22:53 PDT
Created attachment 767239 [details] [diff] [review]
Patch v4

Well, this is embarrassing. Seems like I uploaded the wrong patch previously. Fixed now.
Comment 9 alexander :surkov 2013-06-26 05:35:30 PDT
Comment on attachment 767239 [details] [diff] [review]
Patch v4

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

r=me, thanks!
Comment 10 Marco Zehe (:MarcoZ) 2013-06-26 06:06:12 PDT
Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc6b2673c2a9

Thank you for the patch!
Comment 11 Ryan VanderMeulen [:RyanVM] 2013-06-26 13:42:22 PDT
https://hg.mozilla.org/mozilla-central/rev/fc6b2673c2a9

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