Closed Bug 597963 Opened 15 years ago Closed 15 years ago

crash [@ nsIDocument::GetContainer() ]

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b8
Tracking Flags:
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: scoobidiver, Assigned: ginnchen+exoracle)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 2 obsolete files)

patch v2
1.20 KB, patch
surkov
: review+
Details | Diff | Splinter Review
Build : Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre This is a residual crash signature that exists in trunk builds. It is #43 top crasher for 4.0b6 for the last two weeks. See the crash with the only comment : Signature nsIDocument::GetContainer() UUID d4e0a44e-8a45-4dfa-be8a-5c4d42100919 Time 2010-09-19 07:37:37.609314 Uptime 1529 Install Age 338270 seconds (3.9 days) since version was first installed. Product Firefox Version 4.0b6 Build ID 20100914083612 Branch 2.0 OS Windows NT OS Version 6.1.7600 CPU x86 CPU Info AuthenticAMD family 16 model 4 stepping 2 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x30 User Comments Dictating using Word 7 dictation into the LMS Angel when the program crashed. App Notes AdapterVendorID: 10de, AdapterDeviceID: 0a20 Crashing Thread Frame Module Signature [Expand] Source 0 xul.dll nsIDocument::GetContainer obj-firefox/dist/include/nsIDocument.h:796 1 xul.dll nsAccDocManager::ShutdownDocAccessiblesInTree accessible/src/base/nsAccDocManager.cpp:98 2 xul.dll nsOuterDocAccessible::Shutdown accessible/src/base/nsOuterDocAccessible.cpp:173 3 xul.dll ClearCacheEntry<nsAccessible> accessible/src/base/nsAccCache.h:59 4 xul.dll nsBaseHashtable<nsURIHashKey,nsRefPtr<nsCSSStyleSheet>,nsCSSStyleSheet*>::s_EnumStub obj-firefox/dist/include/nsBaseHashtable.h:364 5 xul.dll PL_DHashTableEnumerate obj-firefox/xpcom/build/pldhash.c:754 6 xul.dll nsBaseHashtable<nsPtrHashKey<void const >,nsRefPtr<nsAccessible>,nsAccessible*>::Enumerate obj-firefox/dist/include/nsBaseHashtable.h:239 7 xul.dll ClearCache<nsAccessible> accessible/src/base/nsAccCache.h:71 8 xul.dll nsAccDocManager::ShutdownDocAccessible accessible/src/base/nsAccDocManager.cpp:148 9 xul.dll nsAccDocManager::ShutdownDocAccessiblesInTree accessible/src/base/nsAccDocManager.cpp:541
Interesting, I'm just about to fire this bug.
Attached patch patch (obsolete) — — Splinter Review
The second part of the patch fixes another crash caused by null check. To reproduce: Enable a11y on Linux or OpenSolaris. Run make reftest make mochitest-plain
Assignee: nobody → ginn.chen
Attachment #476741 - Flags: review?(surkov.alexander)
OS: Windows 7 → All
Hardware: x86 → All
Sorry, the second part of the patch is for this bug, the first part of the patch is for Bug 597343.
(In reply to comment #3) > Sorry, the second part of the patch is for this bug, the first part of the > patch is for Bug 597343. that's ok to keep them together, but perhaps the first part makes unnecessary the second part.
No, they're different crashes.
(In reply to comment #5) > No, they're different crashes. you're right, though it sounds we need to have something else than just null check for shutdown process, otherwise we might keep sub documents in the cache. (perhaps we could be based on our document hierarchy cache). Ginn, once you're on these bugs, could you please figure out why this is happen and what should we do? You always can get help from Boris if you have specific questions about docshell stuffs.
blocking2.0: --- → ?
Approving blocking. The deeper problem is worth the attention and we can always take the null checks for release (assuming a landing window).
blocking2.0: ? → final+
Weird, I cannot reproduce the crash with mochitest-plain at home. It wascrashed at nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocShellTreeItem *aTreeItem, nsIDocument *aDocument) I think we missed another null check at nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocument *aDocument)
On Gnome 2.32, run TEST_PATH=dom/tests/mochitest/bugs gmake mochitest-plain I got crash with stack: Note: ShutdownDocAccessiblesInTree is re-entered with same document. fd777248 void nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocument*) (9682d40, 0, 8043a18, fd787e09) + 1c fd787e51 void nsOuterDocAccessible::Shutdown() (acfab40, fdfbdc2c, 8043a78, fd7b2368) + 55 fd7877f0 PLDHashOperator ClearCacheEntry<nsAccessible>(const void*,nsRefPtr<__type_0>&,void*) (a315308, 9a95b24, 0, fd787930, a5c7 f40, c) + 18 fd7879cb PLDHashOperator nsBaseHashtable<nsPtrHashKey<const void>,nsRefPtr<nsAccessible>,nsAccessible*>::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*,unsigned,void*) (a5c7fb0, 9a95b1c, 4, 8043af8) + 1b fd871f12 PL_DHashTableEnumerate (a5c7fb0, fd7879b0, 8043af8, fd781bb6) + 8a fd781dc6 void nsDocAccessible::Shutdown() (a5c7f40, 0, 8043b80, fd7af5f0) + 21e fd7af61d void nsRootAccessible::Shutdown() (a5c7f40, b5f8d38, 0) + 39 fd77894c void nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocShellTreeItem*,nsIDocument*) (9682d40) + 398 fd77729b void nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocument*) (9682d40) + 6f fd777c53 unsigned nsAccDocManager::HandleEvent(nsIDOMEvent*) (9682d40, ab32cd0, 8043d38, fcbe7091) + 12f fcbe7818 unsigned nsEventListenerManager::HandleEventInternal(nsPresContext*,nsEvent*,nsIDOMEvent**,nsPIDOMEventTarget*,unsigned,n sEventStatus*,nsCxPusher*) (8b4f768, 0, a3987a8, 8044078, a592f1c, 4) + 798 fcc13c8e unsigned nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&,unsigned,nsDispatchingCallback*,int,nsCx Pusher*) (88a0450, 8044070, 6, 0, 0, 8044090) + 1e2 fcc14b99 unsigned nsEventDispatcher::Dispatch(nsISupports*,nsPresContext*,nsEvent*,nsIDOMEvent*,nsEventStatus*,nsDispatchingCallba ck*,nsCOMArray<nsPIDOMEventTarget>*) (8b4fb48, 0, a3987a8, ab32cd0, 0) + a09 fcc15145 unsigned nsEventDispatcher::DispatchDOMEvent(nsISupports*,nsEvent*,nsIDOMEvent*,nsPresContext*,nsEventStatus*) (8b4fb48, 0, ab32cd0, 0, 0, b5f8e4c) + 115 fcb1bb60 void nsDocument::OnPageHide(int,nsIDOMEventTarget*) (b5f8d38, 0, 0, fc86da1a) + 44c fc86da53 unsigned DocumentViewerImpl::PageHide(int) (b605b40) + 47 fd41d2c1 unsigned nsDocShell::FirePageHideNotification(int) (927ca78, 1, 927cb48, 0) + 65 fd427b92 unsigned nsDocShell::Destroy() (927ca78, 927cb0c) + 126 fd4d0db5 unsigned nsXULWindow::Destroy() (c531c98) + 211 fd4df839 unsigned nsWebShellWindow::Destroy() (c531c98, 5, 8044564, fd4cf974) + 145 fd4cdab5 unsigned nsContentTreeOwner::Destroy() (a315790, 87340c8, 804459c, fce2182e) + 19 fce2199e void nsGlobalWindow::ReallyCloseWindow() (87340c8, 0, 2, fce33120) + 17e fce3313a unsigned nsCloseEvent::Run() (a089170, 1, 804461c, 1) + 26 fd8d3bf5 unsigned nsThread::ProcessNextEvent(int,int*) (80ef500, 1, 804465c, fd87cc71) + 151 fd87cca6 int NS_ProcessNextEvent_P(nsIThread*,int) (80ef500, 1, 80446c8, fd4d0b0c) + 42 fd4d0b22 unsigned nsXULWindow::ShowModal() (c531c98, fdfbdc2c, 8044ae8, fd4cf464) + 152 fd4cda00 unsigned nsContentTreeOwner::ShowAsModal() (a315790, 2, 0, fd48d2e2) + 1c fd48d52f unsigned nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow*,const char*,const char*,const char*,int,nsIArray*,int,nsIDOM Window**) (847fa80, 9074838, c488670, 0, 99dbe68, 1) + 1fd7 fd48b4d5 unsigned nsWindowWatcher::OpenWindow(nsIDOMWindow*,const char*,const char*,const char*,nsISupports*,nsIDOMWindow**) (847f a80, 9074838, c488670, 0, 99dbe68, 9fcbc18) + 3c1 fce29635 unsigned nsGlobalWindow::OpenInternal(const nsAString_internal&,const nsAString_internal&,const nsAString_internal&,int,i nt,int,int,nsIArray*,nsISupports*,nsIPrincipal*,JSContext*,nsIDOMWindow**) (9074838, 8045438, fe210200, 8044fd0, 0, 1) + ef5 fce230f4 unsigned nsGlobalWindow::ShowModalDialog(const nsAString_internal&,nsIVariant*,const nsAString_internal&,nsIVariant**) (9 074838, 8045438, 9fcbc18, c2e3930, 80451ac, 4) + 264 fd8e7eef NS_InvokeByIndex_P (9074838, 5b, 4, 804517c) + 51
(In reply to comment #9) > On Gnome 2.32, run TEST_PATH=dom/tests/mochitest/bugs gmake mochitest-plain > I got crash with stack: Note: the crash is not 100% reproducible. > > Note: ShutdownDocAccessiblesInTree is re-entered with same document. Apparently I was tired when I wrote this note and wrote this patch. It is not re-entered. The crash is caused by childAcc->GetDocumentNode() is null in nsOuterDocAccessible::Shutdown(). i.e. childAcc->mContent is null.
Attachment #476741 - Flags: review?(surkov.alexander)
Alexander, do you think bug 570275 will resolve this?
(In reply to comment #11) > Alexander, do you think bug 570275 will resolve this? No. It might be but I wouldn't expect this.
(In reply to comment #10) > The crash is caused by childAcc->GetDocumentNode() is null in > nsOuterDocAccessible::Shutdown(). > i.e. childAcc->mContent is null. does it mean the document accessible is shutdown? If so I wonder how it happens the document accessible was shutdown but it hadn't a parent since it didn't notified outer doc accessible that it's getting shutdown?
(In reply to comment #13) > (In reply to comment #10) > > > The crash is caused by childAcc->GetDocumentNode() is null in > > nsOuterDocAccessible::Shutdown(). > > i.e. childAcc->mContent is null. > > does it mean the document accessible is shutdown? yes, at least corresponding to the stack. the root document is going to shutdown, subdocuments are shutdown by docshell traversal, then the root document gets shutdown, its children cache is cleared, iframe shutdowns and tries to shutdown its subdocument that was shutdown already in docshell traversal stage. > If so I wonder how it happens > the document accessible was shutdown but it hadn't a parent since it didn't > notified outer doc accessible that it's getting shutdown? but this question stays open, if the subdocument was shutdown then it child reference to it should be removed from container iframe accessible. This doesn't happen due to some reason. In general we could remove shutdown document by docshell traversal because iframes takes care of this. But the above question makes be doubt because something wrong happens. Ginn, could you debug this please?
It's hard to debug because I can only reproduce it randomly with release build. :( I'll try in next week.
It always happen with test_bug504862.html on my machine. We got the crash because we're trying to Shutdown an already shutdown node in nsOuterDocAccessible::Shutdown(). For your question, why it doesn't remove itself from nsOuterDocAccesible's mChildren when it shuts down. The reason if nsOuterDocAccesible::Append/RemoveChild doesn't work, if it tries to add second child to one nsOuterDocAccesible. It will cause this crash, if the second child gets shutdown before the first one. We hit this problem because file_bug504862.html is loading a different url in the same dialog. Probably we need to make nsOuterDocAccesible::RemoveChild tolerant the wrong sequence.
Yes, it should be tolerant for additional protection but we need to get fixed two documents issue, I don't understand why we're going to create new document while the old one isn't shutdown. If you run this test in stand alone mode then do you get a crash?
Yes, I can get the crash, not 100% though. There're 2 stacks on the second document was added. 1) =>[1] nsOuterDocAccessible::AppendChild(0xa1d0700, 0xd0c8a28, 0x1, 0xdd5f5a8), at 0xfd7d3ee6 [2] nsAccDocManager::CreateDocOrRootAccessible(0x9537b98, 0xada27b8, 0x0, 0xfd7c400d), at 0xfd7c52ac [3] nsAccDocManager::GetDocAccessible(0x9537b98, 0xada27b8, 0x8042074, 0xfd7df286), at 0xfd7c404d [4] nsAccessibilityService::ContentRangeInserted(0x9537b98, 0x8ebde90, 0xc723d78, 0xb12a958, 0x0, 0xfdeb02d0, 0xfe097eb0, 0x9741b28), at 0xfd7df2a5 [5] nsCSSFrameConstructor::ContentAppended(0x91f47c0, 0xc723d78, 0xb12a958, 0x0), at 0xfc86af74 [6] nsCSSFrameConstructor::CreateNeededFrames(0x91f47c0, 0xc723d78, 0x8042238, 0xfc86a2d6), at 0xfc86a38b [7] nsCSSFrameConstructor::CreateNeededFrames(0x91f47c0), at 0xfc86a443 [8] nsCSSFrameConstructor::CreateNeededFrames(0x91f47c0), at 0xfc86a4e3 [9] PresShell::FlushPendingNotifications(0x8ebde90, 0x3, 0x8042380, 0xfcb4872a), at 0xfc8cdd1d [10] nsDocument::FlushPendingNotifications(0xada27b8, 0x3, 0xfe15f4f0, 0xfd4a0876), at 0xfcb48964 [11] nsDocLoader::DocLoaderIsEmpty(0xc61d4b0, 0x1, 0x2, 0xfd4a04ea), at 0xfd4a0998 [12] nsDocLoader::OnStopRequest(0xc61d4b0, 0xcf4cfa0, 0x0, 0x804b0002), at 0xfd4a075a [13] nsLoadGroup::RemoveRequest(0xbf32060, 0xcf4cfa0), at 0xfc6a652b [14] nsLoadGroup::Cancel(0xbf32060, 0x804b0002, 0x8042580, 0xfd49f6ee), at 0xfc6a5b18 [15] nsDocLoader::Stop(0xc61d4b0, 0xfe0787a4, 0x80425c0, 0xfd493f60, 0xc61d4b0), at 0xfd49f788 [16] nsDocShell::Stop(0xc61d4b0, 0xfe0787a4, 0x8042600, 0xfd4718a1), at 0xfd493f72 [17] nsDocShell::Stop(0xc61d4b0, 0x3, 0x0, 0x0), at 0xfd471e7e [18] nsDocShell::InternalLoad(0xc61d4b0, 0xa1c90e0, 0x8ebddd8, 0xaae1f00, 0x0, 0xca91f20, 0x0, 0x0, 0x0, 0x8800001, 0x0, 0x1, 0x0), at 0xfd47fe4e [19] nsDocShell::LoadURI(0xc61d4b0, 0xa1c90e0, 0xb58bcf0, 0x0, 0x1), at 0xfd4679c5 [20] nsLocation::SetURI(0xc788710), at 0xfce7a90f [21] nsLocation::SetHrefWithBase(0xc788710), at 0xfce7befc [22] nsLocation::SetHref(0xc788710), at 0xfce7b801 [23] nsWindowSH::SetProperty(0xccabf68, 0xc0dc520, 0x9e9d858, 0xe4673ea8, 0xf6816820, 0x8043350, 0x8042ff4, 0xfd37e166), at 0xfce8db31 ...... [76] DocumentViewerImpl::LoadComplete(0xa424da8, 0x0, 0xfe15f4f0, 0xfd4776a9), at 0xfc899ef9 [77] nsDocShell::EndPageLoad(0x8e32590), at 0xfd4778b7 [78] nsDocShell::OnStateChange(0x8e32590, 0x8e325a4, 0xc765a60, 0x20010, 0x0, 0x80468f0, 0xfe097f50, 0xfd4a1ef6), at 0xfd476854 [79] nsDocLoader::FireOnStateChange(0x8e32590, 0x8e325a4, 0xc765a60, 0x20010, 0x0), at 0xfd4a201d [80] nsDocLoader::DocLoaderIsEmpty(0x8e32590, 0x1, 0x2, 0xfd4a04ea), at 0xfd4a0aff [81] nsDocLoader::OnStopRequest(0x8e32590, 0x94c8be8, 0x0, 0x0), at 0xfd4a075a [82] nsLoadGroup::RemoveRequest(0x8e32a98, 0x94c8be8, 0x0, 0x0), at 0xfc6a652b [83] nsDocument::UnblockOnload(0x924e990), at 0xfcb4b8d9 [84] nsDocument::DispatchContentLoadedEvents(0x924e990, 0x0, 0x3, 0x8108500), at 0xfcb412b0 [85] nsRunnableMethodImpl<void(nsDocument::*)(),true>::Run(0xac74e38, 0x1, 0x8046e4c, 0x0), at 0xfcb51efa 2) current thread: t@1 =>[1] nsOuterDocAccessible::AppendChild(0xc5ac688, 0xa931168, 0x1, 0x955a308), at 0xfd7d3ee6 [2] nsAccDocManager::CreateDocOrRootAccessible(0x9537b98, 0xd05d858, 0x0, 0xfd7c400d), at 0xfd7c52ac [3] nsAccDocManager::GetDocAccessible(0x9537b98, 0xd05d858, 0x80429b8, 0xfd7e53be), at 0xfd7c404d [4] nsAccessibilityService::GetAccessibleByRule(0x9537b98, 0xd05d858, 0x955a308), at 0xfd7e53f9 [5] nsRootAccessible::HandleEvent(0xabb5c70, 0xa931130, 0xfe2cefb8, 0x8043318), at 0xfd7fa4f3 [6] nsEventListenerManager::HandleEventInternal(0xd0eb4e0, 0x9ef2c28, 0x8043380, 0x8043318, 0xa4e1ec0, 0x4, 0x804331c, 0x8043330), at 0xfcc297b4 [7] nsEventTargetChainItem::HandleEventTargetChain(0x8721e58, 0x8043310, 0x6, 0x0, 0x0, 0x8043330, 0xfe097f00, 0xfcc563f9), at 0xfcc55ee6 [8] nsEventDispatcher::Dispatch(0xd05d858, 0x9ef2c28, 0x8043380, 0x0, 0x0), at 0xfcc56df1 [9] FocusBlurEvent::Run(0xd0feda8), at 0xfce4c620 [10] nsContentUtils::AddScriptRunner(0xd0feda8), at 0xfcb0ac3f [11] c1A___1cOnsFocusManagerUSendFocusOrBlurEvent6MIpnMnsIPresShell_pnLnsIDocument_pnLnsISupports_Iii_v_(0x840d680, 0xcd5a7d8, 0xd05d858, 0xd05d858), at 0xfce4c486 [12] nsFocusManager::Blur(0x840d680, 0x0, 0x0, 0x1), at 0xfce43bda [13] nsFocusManager::WindowLowered(0x840d680, 0xa536d60, 0xfe1aa6f0, 0xfd528809), at 0xfce4009c [14] nsWebShellWindow::HandleEvent(0x80438f0, 0x0, 0x0, 0xfd75f084), at 0xfd528909 [15] nsWindow::DispatchEvent(0x9e237f8, 0x80438f0, 0x80438e4, 0xfd75ef46), at 0xfd75f0a4 [16] nsWindow::DispatchDeactivateEvent(0x9e237f8), at 0xfd75efb2 [17] focus_out_event_cb(0xb8b9a48, 0x9559cf0), at 0xfd76ed11 ..... [36] gtk_widget_hide(0xb8b9a48, 0x80b3b08, 0x8044228, 0xfd76b13c), at 0xfbaa7213 [37] nsWindow::NativeShow(0x9e237f8, 0x0, 0xfe097f30, 0xfd760436), at 0xfd76b154 [38] nsWindow::Show(0x9e237f8, 0x0, 0xfe1ca630, 0xfd51b8ea), at 0xfd760579 [39] nsXULWindow::Destroy(0xa4078b0), at 0xfd51ba98 [40] nsWebShellWindow::Destroy(0xa4078b0, 0x5, 0x8044364, 0xfd51a6ac), at 0xfd52a7e9 [41] nsContentTreeOwner::Destroy(0xa691a88, 0xab20ec0, 0x8044394, 0xfce64cf6), at 0xfd5187ed [42] nsGlobalWindow::ReallyCloseWindow(0xab20ec0, 0x0, 0x3, 0xfce76c44), at 0xfce64ff8 [43] nsCloseEvent::Run(0xd9ff148, 0x1, 0x804442c, 0x1), at 0xfce76c5e ...... [69] DocumentViewerImpl::LoadComplete(0xa424da8, 0x0, 0xfe15f4f0, 0xfd4776a9), at 0xfc899ef9 [70] nsDocShell::EndPageLoad(0x8e32590), at 0xfd4778b7 [71] nsDocShell::OnStateChange(0x8e32590, 0x8e325a4, 0xc765a60, 0x20010, 0x0, 0x80468f0, 0xfe097f50, 0xfd4a1ef6), at 0xfd476854 [72] nsDocLoader::FireOnStateChange(0x8e32590, 0x8e325a4, 0xc765a60, 0x20010, 0x0), at 0xfd4a201d [73] nsDocLoader::DocLoaderIsEmpty(0x8e32590, 0x1, 0x2, 0xfd4a04ea), at 0xfd4a0aff [74] nsDocLoader::OnStopRequest(0x8e32590, 0x94c8be8, 0x0, 0x0), at 0xfd4a075a [75] nsLoadGroup::RemoveRequest(0x8e32a98, 0x94c8be8, 0x0, 0x0), at 0xfc6a652b [76] nsDocument::UnblockOnload(0x924e990), at 0xfcb4b8d9 [77] nsDocument::DispatchContentLoadedEvents(0x924e990, 0x0, 0x3, 0x8108500), at 0xfcb412b0 [78] nsRunnableMethodImpl<void(nsDocument::*)(),true>::Run(0xac74e38, 0x1, 0x8046e4c, 0x0), at 0xfcb51efa Should we allow 2 children of nsOuterDocAccessible or should we always remove the first one if the second one arrives?
In general if the old document removal is pending already then we could force things and remove it early. But we should understand why the old one wasn't removed when we create the new. Could you get a stack when we destroy the old document (if we do this at all) and ask Boris why this happens in this order?
It is at http://hg.mozilla.org/mozilla-central/file/0fed77757e88/layout/base/nsDocumentViewer.cpp#l1912 The problem is we get into nsAccDocManager::CreateDocOrRootAccessible() before DocumentViewerImpl::Show(). Adding new child stack: [19] nsDocShell::LoadURI(this = 0xae46678, aURI = 0x9fbdab8, aLoadInfo = 0xa558078, aLoadFlags = 0, aFirstParty = 1), line 1408 in "nsDocShell.cpp" [20] nsLocation::SetURI(this = 0xacfa620, aURI = 0x9fbdab8, aReplace = 1), line 316 in "nsLocation.cpp" [21] nsLocation::SetHrefWithBase(this = 0xacfa620, aHref = CLASS, aBase = 0xbeb1888, aReplace = 0), line 591 in "nsLocation.cpp" [22] nsLocation::SetHrefWithContext(this = 0xacfa620, cx = 0xa02f038, aHref = CLASS, aReplace = 0), line 538 in "nsLocation.cpp" [23] nsLocation::SetHref(this = 0xacfa620, aHref = CLASS), line 507 in "nsLocation.cpp" ....(javascript stacks) [53] nsHTMLScriptElement::MaybeProcessScript(this = 0x9536ce0), line 551 in "nsHTMLScriptElement.cpp" [54] nsHTMLScriptElement::DoneAddingChildren(this = 0x9536ce0, aHaveNotified = 1), line 480 in "nsHTMLScriptElement.cpp" [55] nsHtml5TreeOpExecutor::RunScript(this = 0x952e768, aScriptElement = 0x9536ce0), line 730 in "nsHtml5TreeOpExecutor.cpp" [56] nsHtml5TreeOpExecutor::RunFlushLoop(this = 0x952e768), line 525 in "nsHtml5TreeOpExecutor.cpp" [57] nsHtml5ExecutorFlusher::Run(this = 0xc9b42a8), line 153 in "nsHtml5StreamParser.cpp" [58] nsThread::ProcessNextEvent(this = 0x8115330, mayWait = 1, result = 0x8043ce8), line 547 in "nsThread.cpp" [59] NS_ProcessNextEvent_P(thread = 0x8115330, mayWait = 1), line 250 in "nsThreadUtils.cpp" [60] nsXULWindow::ShowModal(this = 0xc3aae38), line 419 in "nsXULWindow.cpp" Removing old child stack: [7] DocumentViewerImpl::Destroy(this = 0xc989340), line 1618 in "nsDocumentViewer.cpp" [8] DocumentViewerImpl::Show(this = 0xa47f958), line 1912 in "nsDocumentViewer.cpp" [9] nsPresContext::EnsureVisible(this = 0xab44ff0), line 1665 in "nsPresContext.cpp" [10] PresShell::UnsuppressAndInvalidate(this = 0xc9c1898), line 4582 in "nsPresShell.cpp" [11] PresShell::ProcessReflowCommands(this = 0xc9c1898, aInterruptible = 1), line 7932 in "nsPresShell.cpp" [12] PresShell::FlushPendingNotifications(this = 0xc9c1898, aType = Flush_InterruptibleLayout), line 4869 in "nsPresShell.cpp" [13] PresShell::HandlePostedReflowCallbacks(this = 0xc9c1898, aInterruptible = 1), line 4731 in "nsPresShell.cpp" [14] PresShell::DidDoReflow(this = 0xc9c1898, aInterruptible = 1), line 7614 in "nsPresShell.cpp" [15] PresShell::ProcessReflowCommands(this = 0xc9c1898, aInterruptible = 1), line 7902 in "nsPresShell.cpp" [16] PresShell::FlushPendingNotifications(this = 0xc9c1898, aType = Flush_InterruptibleLayout), line 4869 in "nsPresShell.cpp" [17] nsRefreshDriver::Notify(this = 0xc787b30, _ARG2 = 0xcf2e658), line 310 in "nsRefreshDriver.cpp" [18] nsTimerImpl::Fire(this = 0xcf2e658), line 428 in "nsTimerImpl.cpp" [19] nsTimerEvent::Run(this = 0xa103ca8), line 517 in "nsTimerImpl.cpp" [20] nsThread::ProcessNextEvent(this = 0x8115330, mayWait = 1, result = 0x8043ce8), line 547 in "nsThread.cpp" [21] NS_ProcessNextEvent_P(thread = 0x8115330, mayWait = 1), line 250 in "nsThreadUtils.cpp" [22] nsXULWindow::ShowModal(this = 0xc3aae38), line 419 in "nsXULWindow.cpp"
Ginn, could you please cc Boris and address the question to him?
Hi Boris, the question here is why we get nsOuterDocAccessible::AppendChild for the new child before nsOuterDocAccessible::RemoveChild for the old child. I can see SetPreviousViewer() being called before nsOuterDocAccessible::AppendChild for the new child. =>[1] DocumentViewerImpl::SetPreviousViewer(this = 0xa4eec70, aViewer = 0x8e84c68), line 1841 in "nsDocumentViewer.cpp" [2] nsDocShell::SetupNewViewer(this = 0x9cf3aa0, aNewViewer = 0xa4eec70), line 7576 in "nsDocShell.cpp" [3] nsDocShell::Embed(this = 0x9cf3aa0, aContentViewer = 0xa4eec70, aCommand = 0xfda33beb "", aExtraInfo = (nil)), line 5689 in "nsDocShell.cpp" [4] nsDocShell::CreateContentViewer(this = 0x9cf3aa0, aContentType = 0xaffc558 "text/html", request = 0xaccd384, aContentHandler = 0xaccd9c8), line 7380 in "nsDocShell.cpp" And DocumentViewerImpl::Destroy() is called for the prevViewer after that, which triggered nsOuterDocAccessible::RemoveChild. So should we do nsOuterDocAccessible::RemoveChild for the old one immediately when we get AppendChild for the new one? Thanks!
(In reply to comment #22) > So should we do nsOuterDocAccessible::RemoveChild for the old one immediately > when we get AppendChild for the new one? Not sure the question formed in a11y terms is in right form for Boris. Technically we need to know why do we get notifications about document destroy after document creation. And if that's "in low" and the notifications order may be random then we should deal with it on a11y side. Otherwise it should be fixed outside of a11y. So shortly we need to know where's the problem actually.
> Technically we need to know why do we get notifications about document destroy > after document creation. Because we keep showing the old document for a bit after creating the new one, and while building the new DOM and frame tree. That's done on purpose to avoid weird flashes of default background color. So yes, the old viewer will be destroyed after the new one is created, typically.
Thanks, Boris. Do I understand right there is no chance to "rollback" this operation (for example, when something goes wrong) when new document is created but old one is not destroyed yet so that if we run into the case when we have two documents then we can be sure that old document will die, new document will stay. Correct? Ginn, I think we should shutdown the old document early when we were requested to create new document. That should be safe (even if kind of rollbacks are allowed I think).
> Correct? At the moment, yes.
Attached patch patch (obsolete) — — Splinter Review
Attachment #476741 - Attachment is obsolete: true
Attachment #486017 - Flags: review?(surkov.alexander)
Comment on attachment 486017 [details] [diff] [review] patch > nsAccDocManager::ShutdownDocAccessiblesInTree(nsIDocument *aDocument) > { >+ NS_ASSERTION(aDocument, "Null document for ShutdownDocAccessiblesInTree"); >+ nit: empty line is not necessary here. >+ if (!aDocument) >+ return; this shouldn't happen, I'm fine with assertion but are you sure you want to add the null check? > PRBool > nsOuterDocAccessible::AppendChild(nsAccessible *aAccessible) > { >- NS_ASSERTION(!mChildren.Length(), >- "Previous child document of outerdoc accessible wasn't removed!"); perhaps assert when mChildren.Lenght() > 1? >+ // Previous child document of outerdoc accessible will be shut down soon. >+ // Remove it now. please extend the comment based on info from Boris. It's useful to say here why this happens. >+ if (mChildren.Length()) >+ RemoveChild(mChildren[0]); > > if (!nsAccessible::AppendChild(aAccessible)) > return PR_FALSE; >- >+ nit: line with whitespaces added
Attachment #486017 - Flags: review?(surkov.alexander) → review+
(In reply to comment #28) > Comment on attachment 486017 [details] [diff] [review] > this shouldn't happen, I'm fine with assertion but are you sure you want to add > the null check? OK, I'll remove the null check and the assertion. > > > PRBool > > nsOuterDocAccessible::AppendChild(nsAccessible *aAccessible) > > { > >- NS_ASSERTION(!mChildren.Length(), > >- "Previous child document of outerdoc accessible wasn't removed!"); > > perhaps assert when mChildren.Lenght() > 1? > I don't think it's necessary unless we have another entry for AppendChild. > >+ // Previous child document of outerdoc accessible will be shut down soon. > >+ // Remove it now. > > please extend the comment based on info from Boris. It's useful to say here why > this happens. Will do. > nit: line with whitespaces added Will fix.
Comment on attachment 486017 [details] [diff] [review] patch Ginn, I missed that point that we should remove old document from documents cache of parent accessible, see nsDocAccessible::Shutdown().
Attachment #486017 - Flags: review+
(In reply to comment #30) > Comment on attachment 486017 [details] [diff] [review] > patch > > Ginn, I missed that point that we should remove old document from documents > cache of parent accessible, see nsDocAccessible::Shutdown(). of parent document accessible
(In reply to comment #31) > > Ginn, I missed that point that we should remove old document from documents > > cache of parent accessible, see nsDocAccessible::Shutdown(). in other words we should shutdown it I think, at least if we can be sure there's no change it can be recreated (I hope this old document can't pass our check when we decide whether we should create or not the document accessible).
Would mChildren[0]->Shutdown(); be enough?
(In reply to comment #33) > Would mChildren[0]->Shutdown(); be enough? yeah, at least after bug 607882. But could you check please whether it can be recreated?
Attached patch patch v2 — — Splinter Review
Confirmed, the nsDocAccessible can not be recreated for the old document.
Attachment #486017 - Attachment is obsolete: true
Attachment #486887 - Flags: review?(surkov.alexander)
(In reply to comment #35) > Created attachment 486887 [details] [diff] [review] > patch v2 > > Confirmed, the nsDocAccessible can not be recreated for the old document. Ginn, just wondering what check in CreateDocOrRootAccessible filters old documents?
Comment on attachment 486887 [details] [diff] [review] patch v2 r=me while accessible for old document can't be recreated.
Attachment #486887 - Flags: review?(surkov.alexander) → review+
(In reply to comment #36) > (In reply to comment #35) > > Created attachment 486887 [details] [diff] [review] [details] > > patch v2 > > > > Confirmed, the nsDocAccessible can not be recreated for the old document. > > Ginn, just wondering what check in CreateDocOrRootAccessible filters old > documents? aDocument->IsInitialDocument() !aDocument->IsVisible() !aDocument->IsActive() These checkings can filter it.
Can we get blocking7 for this crash fix?
Let's get this in as soon as we can, but we are too late for beta7 as per discussion with beta7 drivers on #planning today.
blocking2.0: final+ → beta8+
http://hg.mozilla.org/mozilla-central/rev/11837c6ed4a0
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Crash Signature: [@ nsIDocument::GetContainer() ]
Blocks: 735666
No longer blocks: 735666
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: