Closed
Bug 1066539
Opened 10 years ago
Closed 6 years ago
crash in mozilla::a11y::DocManager::AddListeners(nsIDocument*, bool)
Categories
(Core :: Disability Access APIs, defect, P3)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: wsmwk, Unassigned)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
831 bytes,
patch
|
Details | Diff | Splinter Review |
This signature is more common in Thunderbird 31 than TB24 This bug was filed from the Socorro interface and is report bp-0aad59d5-3687-4996-afd8-493672140821. ============================================================= 0 xul.dll mozilla::a11y::DocManager::AddListeners(nsIDocument*, bool) accessible/src/base/DocManager.cpp 1 xul.dll mozilla::a11y::DocManager::CreateDocOrRootAccessible(nsIDocument*) accessible/src/base/DocManager.cpp 2 xul.dll mozilla::a11y::DocManager::GetDocAccessible(nsIDocument*) accessible/src/base/DocManager.cpp 3 xul.dll mozilla::a11y::DocManager::GetDocAccessible(nsIPresShell const*) objdir-tb/mozilla/dist/include/mozilla/a11y/DocManager.h 4 xul.dll nsAccessibilityService::ContentRangeInserted(nsIPresShell*, nsIContent*, nsIContent*, nsIContent*) accessible/src/base/nsAccessibilityService.cpp http://hg.mozilla.org/releases/mozilla-esr31/annotate/51a9591df62f/accessible/src/base/DocManager.cpp#l326 surkov.alexander@43310 321 void surkov.alexander@122870 322 DocManager::AddListeners(nsIDocument* aDocument, surkov.alexander@122870 323 bool aAddDOMContentLoadedListener) surkov.alexander@43310 324 { dzbarsky@140331 325 nsPIDOMWindow* window = aDocument->GetWindow(); dzbarsky@140331 326 EventTarget* target = window->GetChromeEventHandler(); In firefox, 50% are startup crashes eg bp-8c8d7340-512f-4c17-9135-2624d2140908 0 xul.dll mozilla::a11y::DocManager::AddListeners(nsIDocument*, bool) accessible/src/base/DocManager.cpp 1 xul.dll mozilla::a11y::DocManager::CreateDocOrRootAccessible(nsIDocument*) accessible/src/base/DocManager.cpp 2 xul.dll mozilla::a11y::DocManager::HandleDOMDocumentLoad(nsIDocument*, unsigned int) accessible/src/base/DocManager.cpp 3 xul.dll mozilla::a11y::DocManager::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) accessible/src/base/DocManager.cpp 4 xul.dll nsDocLoader::DoFireOnStateChange(nsIWebProgress* const, nsIRequest* const, int&, tag_nsresult) uriloader/base/nsDocLoader.cpp 5 xul.dll nsDocLoader::doStopDocumentLoad(nsIRequest*, tag_nsresult) uriloader/base/nsDocLoader.cpp 6 xul.dll nsDocLoader::DocLoaderIsEmpty(bool) uriloader/base/nsDocLoader.cpp
Reporter | ||
Updated•10 years ago
|
Component: Disability Access → Disability Access APIs
Product: Thunderbird → Core
Version: 31 → 31 Branch
Comment 2•10 years ago
|
||
Comment on attachment 8488628 [details] [diff] [review] patch Review of attachment 8488628 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: accessible/base/DocManager.cpp @@ +323,5 @@ > bool aAddDOMContentLoadedListener) > { > nsPIDOMWindow* window = aDocument->GetWindow(); > + if (!window) > + return; OK. Can you make a comment something like: // We need to null check here because Thunderbird is doing something we don't have time to debug.
Attachment #8488628 -
Flags: review?(dbolter) → review+
Comment 3•10 years ago
|
||
If this is a regression, let's get a window.
Comment 4•10 years ago
|
||
Comment on attachment 8488628 [details] [diff] [review] patch Review of attachment 8488628 [details] [diff] [review]: ----------------------------------------------------------------- Oh! This also happens in FF: https://crash-stats.mozilla.com/report/index/2073b8a0-012f-4f8c-8f10-060ab2140908 Cancelling r+ for this and as there is further discussion.
Attachment #8488628 -
Flags: review+
Comment 5•10 years ago
|
||
Boris, we're not getting a window from an nsIDocument in these stacks. Do you happen to know if that should be expected (sometimes)? (and when etc)
Flags: needinfo?(bzbarsky)
Comment 7•10 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #6) > Yes, that should be expected. Would it be expected on nsGlobalWindow::OpenInternal (stack from comment #0)?
Comment 8•10 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #6) > Yes, that should be expected. O_O why?
Comment 9•10 years ago
|
||
I don't know. For one thing, mWindow on nsDocument will be null before the SetScriptGlobalObject call from nsGlobalWindow::SetNewDocument. Code inspection should tell you if that is what happens here. If mWindow is null, there is a ton of ways in which getting a window might fail. Look at all of the early returns in nsDocument::GetWindowInternal and its callees.
Comment 10•10 years ago
|
||
(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #9) > I don't know. For one thing, mWindow on nsDocument I'd say you should have some reason it should be expected, because off hand having a document and a presShell for it without a window sounds kind of insane. will be null before the > SetScriptGlobalObject call from nsGlobalWindow::SetNewDocument. Code > inspection should tell you if that is what happens here. If mWindow is > null, there is a ton of ways in which getting a window might fail. Look at > all of the early returns in nsDocument::GetWindowInternal and its callees. Well, never actually returns early, and at least to me its unclear if it expects its callees to fail or not.
Comment 11•10 years ago
|
||
Do you have a way to reproduce this? Because that would make it way easier to answer some of these questions.
> having a document and a presShell for it without a window sounds kind of insane.
You just described SVG images and SVG resource documents. ;)
However, those should not have a docshell, and the stacks certainly look like a docshell is involved.
Furthermore, the document in question came from a GetDocument() on a window, right?
Reporter | ||
Comment 12•10 years ago
|
||
regression-wise: thunderbird is no help - no crashes in 2014 for aurora, nightly or beta firefox signature summary https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Aa11y%3A%3ADocManager%3A%3AAddListeners%28nsIDocument*%2C+bool%29&product=Firefox&query_type=is_exactly&range_unit=weeks&process_type=any&hang_type=any&date=2014-09-12+15%3A00%3A00&range_value=4#reports claims crashes exist for version as old as 25 and 26. examples bp-740662e5-ff18-4a74-94a6-717d82140220 Build ID 20131205075310 version 26 bp-17bca82c-d0cc-4d04-ac33-7b0432140528 Build ID 20131112160018 version 25.0.1 there are also a few crashes that call out the next source line bp-37d3225b-45eb-4b38-b606-1d8532140223 332 nsEventListenerManager* elm = target->GetOrCreateListenerManager();
Comment 13•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #11) > Do you have a way to reproduce this? Because that would make it way easier > to answer some of these questions. Definitely. > > having a document and a presShell for it without a window sounds kind of insane. > > You just described SVG images and SVG resource documents. ;) > > However, those should not have a docshell, and the stacks certainly look > like a docshell is involved. > > Furthermore, the document in question came from a GetDocument() on a window, > right? I'm not sure. I think in this case we're getting it from the presshell here: http://hg.mozilla.org/releases/mozilla-esr31/annotate/51a9591df62f/accessible/src/base/DocManager.h#l52
Comment 14•10 years ago
|
||
Hmm. I was looking at https://crash-stats.mozilla.com/report/index/2073b8a0-012f-4f8c-8f10-060ab2140908 which doesn't go through that function, right?
Comment 15•10 years ago
|
||
Seems my comment friday didn't go through. (In reply to David Bolter [:davidb] from comment #13) > (In reply to Boris Zbarsky [:bz] from comment #11) > > Do you have a way to reproduce this? Because that would make it way easier > > to answer some of these questions. > > Definitely. I'm not aware of anything. > > > having a document and a presShell for it without a window sounds kind of insane. > > > > You just described SVG images and SVG resource documents. ;) > > > > However, those should not have a docshell, and the stacks certainly look > > like a docshell is involved. yeah, the stacks do seem different, but seem to all involve a docshell somehow. > > Furthermore, the document in question came from a GetDocument() on a window, > > right? > > I'm not sure. I think in this case we're getting it from the presshell here: > http://hg.mozilla.org/releases/mozilla-esr31/annotate/51a9591df62f/ > accessible/src/base/DocManager.h#l52 yeah, I think the way it generally goes in these crashes is window -> docShell -> presShell -> document
Comment 16•10 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #14) > Hmm. I was looking at > https://crash-stats.mozilla.com/report/index/2073b8a0-012f-4f8c-8f10- > 060ab2140908 which doesn't go through that function, right? err, yeah, that one is definitely just window -> document and then document->window fails
Comment 17•10 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #16) > (In reply to Boris Zbarsky [:bz] from comment #14) > > Hmm. I was looking at > > https://crash-stats.mozilla.com/report/index/2073b8a0-012f-4f8c-8f10- > > 060ab2140908 which doesn't go through that function, right? > > err, yeah, that one is definitely just window -> document and then > document->window fails Yeah weird. Is the QI failing somehow here? http://hg.mozilla.org/releases/mozilla-release/annotate/44234f451065/accessible/src/base/DocManager.cpp#l148
Comment 18•10 years ago
|
||
(In reply to David Bolter [:davidb] from comment #17) > (In reply to Trevor Saunders (:tbsaunde) from comment #16) > > (In reply to Boris Zbarsky [:bz] from comment #14) > > > Hmm. I was looking at > > > https://crash-stats.mozilla.com/report/index/2073b8a0-012f-4f8c-8f10- > > > 060ab2140908 which doesn't go through that function, right? > > > > err, yeah, that one is definitely just window -> document and then > > document->window fails > > Yeah weird. Is the QI failing somehow here? > http://hg.mozilla.org/releases/mozilla-release/annotate/44234f451065/ > accessible/src/base/DocManager.cpp#l148 no, that would cause a crash much sooner the first time we call something on the document.
Updated•9 years ago
|
Crash Signature: [@ mozilla::a11y::DocManager::AddListeners(nsIDocument*, bool)] → [@ mozilla::a11y::DocManager::AddListeners(nsIDocument*, bool)]
[@ mozilla::a11y::DocManager::AddListeners]
Comment 19•8 years ago
|
||
Crash volume for signature 'mozilla::a11y::DocManager::AddListeners': - nightly (version 50): 0 crash from 2016-06-06. - aurora (version 49): 0 crash from 2016-06-07. - beta (version 48): 1 crash from 2016-06-06. - release (version 47): 32 crashes from 2016-05-31. - esr (version 45): 0 crash from 2016-04-07. Crash volume on the last weeks: Week N-1 Week N-2 Week N-3 Week N-4 Week N-5 Week N-6 Week N-7 - nightly 0 0 0 0 0 0 0 - aurora 0 0 0 0 0 0 0 - beta 1 0 0 0 0 0 0 - release 5 4 2 9 7 5 0 - esr 0 0 0 0 0 0 0 Affected platform: Windows
status-firefox47:
--- → affected
status-firefox48:
--- → affected
Comment 20•6 years ago
|
||
the crash moved down the road and not it crashes at target->GetOrCreateListenerManager() apparently because target = window.GetChromeEventHandler(). since don't have steps to reproduce, are there any suggestions for diagnostic assertions to add to investigate the issue further?
Priority: -- → P3
Comment 21•6 years ago
|
||
I can see 56/52 crashes only. I guess we can close it as wontfix.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•