741053 - crash in nsIDocument::GetRootElement

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Scoobidiver (away)]

It first appeared in 14.0a1/20120330. The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1965a2c89d61&tochange=92fe907ddac8
It's likely caused by bug 737724.

In addition to the comment in the crash report below, there's another comment:
"Trying to attach a file in gmail. This is a serious bug."

Signature 	nsIDocument::GetRootElement More Reports Search
UUID	986b381b-bfc8-4d19-84e9-348d62120331
Date Processed	2012-03-31 03:09:00
Uptime	71
Last Crash	10.4 minutes before submission
Install Age	35.3 minutes since version was first installed.
Install Time	2012-03-31 02:25:49
Product	Firefox
Version	14.0a1
Build ID	20120330031146
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux 2.6.32-38-generic #83-Ubuntu SMP Wed Jan 4 11:13:04 UTC 2012 i686
Build Architecture	x86
Build Architecture Info	GenuineIntel family 15 model 6 stepping 5
Crash Reason	SIGSEGV
Crash Address	0x60
User Comments	As of the latest nightly update, whenever a file browsing dialog opens up (if I try to upload or download a file, so I need to browse to the particular download folder or file to upload), Nightly crashes. It was working earlier today, but I also tested it against an old Nightly 11 I had, and it's working there. The following atk-bridge-WARNING are given to me by the terminal if I open firefox from it: AT_SPI_REGISTRY was not started at session startup. IOR not set. Could not locate registry
App Notes 	
GLXtest process failed (exited with status 1): GLX version older than the required 1.3
EMCheckCompatibility	True

Frame 	Module 	Signature 	Source
0 	libxul.so 	nsIDocument::GetRootElement 	content/base/src/nsDocument.cpp:3451
1 	libxul.so 	nsRootAccessible::NativeRole 	accessible/src/base/nsRootAccessible.cpp:150
2 	libxul.so 	nsAccessible::Role 	accessible/src/base/nsAccessible.h:181
3 	libxul.so 	nsAccessible::GetRole 	accessible/src/base/nsAccessible.cpp:1260
4 	libxul.so 	nsAccUtils::IsEmbeddedObject 	accessible/src/base/nsAccUtils.h:269
5 	libxul.so 	nsAccessible::AppendChild 	accessible/src/base/nsAccessible.cpp:2623
6 	libxul.so 	nsApplicationAccessibleWrap::AppendChild 	accessible/src/atk/nsApplicationAccessibleWrap.cpp:750
7 	libxul.so 	nsAccessibilityService::AddNativeRootAccessible 	accessible/src/base/nsAccessibilityService.cpp:1752
8 	libxul.so 	toplevel_event_watcher 	accessible/src/atk/nsApplicationAccessibleWrap.cpp:594
9 	libgobject-2.0.so.0.2400.1 	signal_emit_unlocked_R 	gsignal.c:3214
10 	libgobject-2.0.so.0.2400.1 	IA__g_signal_emit_valist 	gsignal.c:2981
11 	libgobject-2.0.so.0.2400.1 	IA__g_signal_emit 	gsignal.c:3038
12 	libgtk-x11-2.0.so.0.2000.1 	IA__gtk_widget_show 	gtkwidget.c:3193
13 	libgtk-x11-2.0.so.0.2000.1 	IA__gtk_dialog_run 	gtkdialog.c:1060
14 	libxul.so 	nsFilePicker::Show 	widget/gtk2/nsFilePicker.cpp:511
15 	libxul.so 	AsyncClickHandler::Run 	content/html/content/src/nsHTMLInputElement.cpp:374
16 	libxul.so 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:656
17 	libxul.so 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:245
18 	libxul.so 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:110
19 	libxul.so 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:208
20 	libxul.so 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:189
21 	libxul.so 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:295
22 	libxul.so 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3770
23 	libxul.so 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3847
24 	libxul.so 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3923

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsIDocument%3A%3AGetRootElement

Comment 1

[Trevor Saunders (:tbsaunde)]

ok, so, I'm not sure how I want ot fix this, but here's what's going on.

we create a gtk window for choosing a file or probably some other things.
the gtk window creates an atk object for itself.
we grab that atk object wrap it in a nsNativeRootAccessibleWrap, and try to append it as a child of the application accessibel.
Appending a child to the application accessible ends up in nsAccessible::AppendChild() which wants to know if the accessible is an embeded object.
to see if the child is a embeded object we get its role.
while getting the role we end up in nsRootAccessible::NativeRole() which assumes we have a real root accessible, not a fake one with no dom document / content etc.

It used to be we decided the fake root accessible was defunct since it had no mContent.  Which was sort of convenient, but only sort of true.

preferences on how to fix?

Comment 3

[Scoobidiver (away)]

When there's a known regression, the version field allows to know where it first appeared and help determine the versions where the patch should land.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #1)

> preferences on how to fix?

short term is to override NativeRole I think. Long term is to not work with content for accessibles that are not IsPrimaryForNode. Ideally we need to have Accessible interface that is implemented by accessible with content and without content.

Comment 5

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #4)
> (In reply to Trevor Saunders (:tbsaunde) from comment #1)
> 
> > preferences on how to fix?
> 
> short term is to override NativeRole I think. Long term is to not work with

I think that will work, but I'm a little concerned we might have forgotten something else that gets called.  I'd sort of been thinking of overriding AppendChild(), or defining an AddWindow() on nsApplicationAccessible.

> content for accessibles that are not IsPrimaryForNode. Ideally we need to
> have Accessible interface that is implemented by accessible with content and
> without content.

how would that work with NativeRole() we want to be able to call it on both things with content, and some things without like the application accessible, and maybe some of the other things that aren't primary for node.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> I think that will work, but I'm a little concerned we might have forgotten
> something else that gets called.  I'd sort of been thinking of overriding
> AppendChild(), or defining an AddWindow() on nsApplicationAccessible.

if you like, maybe AppendNativeWindow or AppendNativeChild.

> > content for accessibles that are not IsPrimaryForNode. Ideally we need to
> > have Accessible interface that is implemented by accessible with content and
> > without content.
> 
> how would that work with NativeRole() we want to be able to call it on both
> things with content, and some things without like the application
> accessible, and maybe some of the other things that aren't primary for node.

the idea is to make sure we don't use mContent or mDocument (in case of nsDocAccessible) when IsPrimaryForNode is false.

Comment 7

[alexander :surkov (:asurkov)]

Trevor, are you taking it?

Comment 8

[alexander :surkov (:asurkov)]

Trevor, how about to take an approach from bug 740958?

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch

It works until we dexpcominate nsAccUtils::IsEmbeddedObject.

I'll file meta bug to provide a correct solution for cases like this.

Comment 10

[Trevor Saunders (:tbsaunde)]

Comment on attachment 611470 [details] [diff] [review]
patch

yeah, r=me for same reason as bug 740958 :/

Comment 11

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8701e9dd7715

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/8701e9dd7715