Closed Bug 377767 Opened 18 years ago Closed 18 years ago

Crash when closing Page Info dialog at the Media tab.

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: Aleksej, Assigned: aaronlev)

References

Details

(Keywords: crash)

Attachments

(3 files, 2 obsolete files)

Minefield 2007041704, Debian Etch. TB31271046E, TB31271212Y, TB31271351M, TB31271046E Also experienced bug 377123 / bug 377732 on this system.
Incident ID: 31271046 Stack Signature 0x00000029 640df0ac Product ID FirefoxTrunk Build ID 2007041704 Trigger Time 2007-04-17 05:16:51.0 Platform LinuxIntel Operating System Linux 2.6.18-4-k7 Module URL visited User Comments Alt-F4 in Page Info likely bug 377123 / bug 377123 that is supposed to have been fixed in this build :( Since Last Crash 0 sec Total Uptime 0 sec Trigger Reason SIGSEGV: Segmentation Fault: (signal 11) Source File, Line No. N/A Stack Trace 0x00000029 nsQueryInterface::operator() [mozilla/xpcom/build/nsCOMPtr.cpp, line 47] nsCOMPtr_base::assign_from_qi() [mozilla/xpcom/build/nsCOMPtr.cpp, line 96] nsAccessible::Shutdown() [mozilla/accessible/src/base/nsAccessible.cpp, line 897] nsAccessNode::ClearCacheEntry() [mozilla/accessible/src/base/nsAccessNode.cpp, line 693] nsBaseHashtable<nsVoidHashKey, nsCOMPtr<nsIAccessNode>, nsIAccessNode*>::s_EnumStub() PL_DHashTableEnumerate() [mozilla/xpcom/build/pldhash.c, line 724] nsAccessNode::ClearCache() [mozilla/accessible/src/base/nsAccessNode.cpp, line 218] nsDocAccessible::Shutdown() [mozilla/accessible/src/base/nsDocAccessible.cpp, line 712] nsDocAccessible::Destroy() [mozilla/accessible/src/base/nsDocAccessible.cpp, line 502] nsRootAccessible::HandleEventWithTarget() [mozilla/accessible/src/base/nsRootAccessible.cpp, line 846] nsRootAccessibleWrap::HandleEventWithTarget() [mozilla/accessible/src/atk/nsRootAccessibleWrap.cpp, line 62] nsRootAccessible::HandleEvent() [mozilla/accessible/src/base/nsRootAccessible.cpp, line 840] nsEventListenerManager::HandleEventSubType() [mozilla/content/events/src/nsEventListenerManager.cpp, line 1093] nsEventListenerManager::HandleEvent() [mozilla/content/events/src/nsEventListenerManager.cpp, line 1055] nsEventTargetChainItem::HandleEvent() [mozilla/content/events/src/nsEventDispatcher.cpp, line 1038] nsEventTargetChainItem::HandleEventTargetChain() [mozilla/content/events/src/nsEventDispatcher.cpp, line 238] nsEventDispatcher::Dispatch() [mozilla/content/events/src/nsEventDispatcher.cpp, line 477] nsDocument::DispatchEventToWindow() [mozilla/content/base/src/nsDocument.cpp, line 5649] nsDocument::OnPageHide() [mozilla/content/base/src/nsDocument.cpp, line 5704] DocumentViewerImpl::PageHide() [mozilla/layout/base/nsDocumentViewer.cpp, line 1237] nsDocShell::FirePageHideNotification() [mozilla/docshell/base/nsDocShell.cpp, line 62] nsDocShell::Destroy() [mozilla/docshell/base/nsDocShell.cpp, line 1055] nsXULWindow::Destroy() [mozilla/xpfe/appshell/src/nsXULWindow.cpp, line 712] nsWebShellWindow::Destroy() [mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 814] nsWebShellWindow::HandleEvent() [mozilla/xpfe/appshell/src/nsWebShellWindow.cpp, line 371] nsCommonWidget::DispatchEvent() [mozilla/widget/src/gtk2/nsCommonWidget.cpp, line 220] nsWindow::OnDeleteEvent() [mozilla/widget/src/gtk2/nsWindow.cpp, line 936] delete_event_cb() [mozilla/widget/src/gtk2/nsWindow.cpp, line 4325] libgtk-x11-2.0.so.0 + 0x12e250 (0xb7baf250) libgobject-2.0.so.0 + 0x998b (0xb792498b) libgobject-2.0.so.0 + 0x19f2d (0xb7934f2d) libgobject-2.0.so.0 + 0x1b208 (0xb7936208) libgobject-2.0.so.0 + 0x1b5d9 (0xb79365d9) libgtk-x11-2.0.so.0 + 0x217f64 (0xb7c98f64) libgtk-x11-2.0.so.0 + 0x128ebf (0xb7ba9ebf) libgdk-x11-2.0.so.0 + 0x42eea (0xb7a28eea) libglib-2.0.so.0 + 0x2b731 (0xb78b3731) libglib-2.0.so.0 + 0x2e7a6 (0xb78b67a6) libglib-2.0.so.0 + 0x2ed27 (0xb78b6d27) nsBaseAppShell::DoProcessNextNativeEvent() [mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp, line 138] nsBaseAppShell::OnProcessNextEvent() [mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp, line 247] nsThread::ProcessNextEvent() [mozilla/xpcom/threads/nsThread.cpp, line 846] NS_ProcessNextEvent_P() [mozilla/xpcom/build/nsThreadUtils.cpp, line 227] nsBaseAppShell::Run() [mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp, line 154] nsAppStartup::Run() [mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 172] XRE_main() [mozilla/toolkit/xre/nsAppRunner.cpp, line 2891] main() [mozilla/browser/app/nsBrowserApp.cpp, line 62] libc.so.6 + 0x14ea8 (0xb738fea8)
Keywords: crash, qawanted
Blocks: 377773
This is reproduceable on Linux, but not on Windows.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: qawanted
Ginn, do you have time to look at this?
Assignee: aaronleventhal → ginn.chen
It's not 100% reproducible, around 50% on my PC. I don't know if it's related, but I didn't reproduce it if I back out this change of bug 372367. RCS file: /cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v -NS_IMPL_ISUPPORTS_INHERITED2(nsXULTextFieldAccessible, nsAccessible, nsIAccessibleText, nsIAccessibleEditableText) +NS_IMPL_ISUPPORTS_INHERITED1(nsXULTextFieldAccessible, nsHyperTextAccessible, + nsIAccessibleText) surkov, can you take a look?
Assignee: ginn.chen → surkov.alexander
Not sure how soon I can because I'm new a bit in linux and I didn't setup it for the work completely yet.
The root cause of the problem is that we run Shutdown() from nsAccessNode::~nsAccessNode() (bug 377737). In this particular case we want to run nsAccessible::Shutdown() http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/accessible/src/base/nsAccessible.cpp&rev=1.256&root=/cvsroot&mark=562#545 but since we don't invoke that method our parent will have a mFirstChild which points to a destroyed node.
Assignee: surkov.alexander → mats.palmgren
Depends on: 377737
Thanks Mats, did you see my question in that bug?
This is unrelated to bug 377737. The real issue is that we have a nsHTMLTextFieldAccessible and a nsXULTextFieldAccessible that have the same 'mFirstChild' pointer. We will crash in Shutdown() for the node that did SetFirstChild() first since the last one "steals" the child (the child corresponds to the anonymous DIV inside the text control).
No longer depends on: 377737
Attached patch wip (obsolete) (deleted) — Splinter Review
This wallpapers over the problem, I'm not sure what the right fix is...
In case it may help, bug 366500 is another bug about nsXULTextFieldAccessible and nsHTMLTextFieldAccessible sharing the same mFirstChild
Mats, the problem is temporary accessible for html input, right?
(In reply to comment #11) > Mats, the problem is temporary accessible for html input, right? I guess you mean this one: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp&rev=1.79&root=/cvsroot&mark=744#730 Good guess, but no cigar, that's not the one...
Attached file Stack traces (deleted) —
Here's a few stack traces, the first two are when the accessible objects involved are created, the third trace is when SetFirstChild steals the child node, and the last trace is the crash. I've highlighted the accessible node addresses.
Attached patch Patch rev. 1 (deleted) — Splinter Review
It seems the root cause for creating the nsHTMLTextFieldAccessible is the InvalidateSubtreeFor() call from nsFrameManager::ReResolveStyleContext. Using the binding parent (if any) fixes the crash (and no assertion occurs). I've left the wallpaper in SetFirstChild() for now, I'm not sure if you want that, but we should at least leave the assertion (wrapped in NS_DEBUG), if not the actual InvalidateChildren() call...
Attachment #261988 - Attachment is obsolete: true
Attachment #262180 - Flags: review?(aaronleventhal)
So the HTML Input is setting its anonymous text as its first child, and that was already the XUL texbox's first child? So the HTML input is stealing it from the XUL textbox? Wouldn't that happen any time the HTML input is asked for a child? That would cause it to CacheChildren(). I'd prefer not to add code to layout. If we need to grab the binding parent we could be doing that inside nsAccessibilityService::InvalidateSubtreeFor(). In fact, that method might want to use nsAccessibilityService::GetRelevantContentNodeFor(), which will make sure we start from the right content node. However, that seems to fix only one situation where CacheChildren() occurs on the anon HTML input accessible.
Attached patch Slightly better (obsolete) (deleted) — Splinter Review
Mats, if you don't mind me writing the next version of this patch -- thanks a lot for figuring it out.
Attachment #262274 - Flags: review?(mats.palmgren)
Comment on attachment 262180 [details] [diff] [review] Patch rev. 1 I favor a slightly modified version of this, please see the next patch.
Attachment #262180 - Flags: review?(aaronleventhal) → review-
Attachment #262274 - Flags: review?(surkov.alexander)
Aaron, do we have a case when attached accessible doesn't allow accessible inside of anonymous content but it sets first child accessible from anonymous content. Is it valid case?
No that doesn't seem valid.
Aaron, should we check if node is relevant when we fire ally event in FireAccessible(Toolkit)Event and if this node is not relevant then to do not fire ally event. I just look at AttributeChanged code and I do not get why do we fire event for attached accessible if attribute was changed on its child that isn't allowed to be accessible.
I wasn't sure if we should fire event if accessible is not attached. I'll change it just to not fire event if node is not relevant.
Assignee: mats.palmgren → aaronleventhal
Attachment #262274 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #262656 - Flags: review?(surkov.alexander)
Attachment #262274 - Flags: review?(surkov.alexander)
Attachment #262274 - Flags: review?(mats.palmgren)
In the patch, it should be s/IsRelevantNode/IsNodeRelevant
Comment on attachment 262656 [details] [diff] [review] Just don't fire events for non-relevant accessibles r=me > >+PRBool nsAccessible::IsNodeRelevant(nsIDOMNode *aNode) >+{ >+ // Don't fire events for accessibles that aren't an attached descendant of the document Comment isn't clear. Here there is not any events. You may want to say this method is used when event is fired or something. Also do we want assertion because it looks it is not normal when this method return false. >+ nsCOMPtr<nsIAccessibilityService> accService = >+ do_GetService("@mozilla.org/accessibilityService;1"); Please use GetAccService()
Attachment #262656 - Flags: review?(surkov.alexander) → review+
GetAccService() wasn't available for static method.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Verified with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a4pre) Gecko/20070425 Minefield/3.0a4pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: