Closed Bug 179089 Opened 22 years ago Closed 21 years ago

crash in selection stuff loading a newsgroup message [@JS_GetPrivate]

Categories

(Core :: DOM: Selection, defect)

x86
Windows 2000
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: mjudge)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

I was reading news.openwatcom.org/openwatcom.contributors i use n/space to advance messages and i also double click in the message body sometimes for fear that focus is in the wrong place. the response time for this session was bad, i'm not sure if that contributed or why it would be the case.
Keywords: crash
JS_GetPrivate [d:/builds/seamonkey/mozilla/js/src/jsapi.c, line 1928] nsScriptSecurityManager::GetFunctionObjectPrincipal [d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1808] nsScriptSecurityManager::GetFramePrincipal [d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1842] nsScriptSecurityManager::GetPrincipalAndFrame [d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1855] nsScriptSecurityManager::GetSubjectPrincipal [d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1895] nsScriptSecurityManager::GetSubjectPrincipal [d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1607] nsContentUtils::CanCallerAccess [d:/builds/seamonkey/mozilla/content/base/src/nsContentUtils.cpp, line 531] nsRange::SetStart [d:/builds/seamonkey/mozilla/content/base/src/nsRange.cpp, line 1030] nsTypedSelection::SetOriginalAnchorPoint [d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 6614] nsTypedSelection::Collapse [d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 6151] nsSelection::TakeFocus [d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 2853] nsSelection::HandleClick [d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 2643] nsFrame::HandlePress [d:/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 1499] nsFrame::HandleEvent [d:/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 981] PresShell::HandleEventInternal [d:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6263] PresShell::HandleEvent [d:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6169] nsViewManager::HandleEvent [d:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 2209] nsView::HandleEvent [d:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 304] nsViewManager::DispatchEvent [d:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 1949] HandleEvent [d:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 83] nsWindow::DispatchEvent [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1074] nsWindow::DispatchWindowEvent [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1091] nsWindow::DispatchMouseEvent [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5208] nsWindow::DispatchFocus [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5462] nsWindow::ProcessMessage [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 4060] nsWindow::WindowProc [d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1339] USER32.DLL + 0x3eb0 (0x77e13eb0) USER32.DLL + 0x401a (0x77e1401a) USER32.DLL + 0x92da (0x77e192da) nsAppShellService::Run [d:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 472] main1 [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1538] main [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1886] WinMain [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1906] WinMainCRTStartup() KERNEL32.DLL + 0x7903 (0x77e87903)
1927 brendan 3.114 v = GC_AWARE_GET_SLOT(cx, obj, JSSLOT_PRIVATE); (cc for help and verification of analysis, not blame) I'm guessing the crash is a destroyed obj. nsScriptSecurityManager::GetFramePrincipal appears to verify that obj isn't null wow, no mailnews code at all. well here's what i think happened: mozilla was slowed down (and handling events slowly). so a mouse double click event fired by the old mailnews message was processed after the message was replaced.
Summary: crash in selection stuff loading a newsgroup message [@JS_GetPrivate] tb13679222k → crash in selection stuff loading a newsgroup message [@JS_GetPrivate]
I think we should move this bug out from under browser / selection. I don't think it has to to with selection. You can reproduce this in the latest thunderbird builds very easily. 1) bring up mail compose 2) Attach a web page, type in a url 3) hit the send button 4) we crash with this same stack trace. Although I just noticed this bug was reported back in 2002. Maybe thunderbird just introduced a new scenario that leads back to this same crash.
bz apparently fixed this problem for the insert case, but the remove case wasn't addressed. The code is nearly identical for the two functions so I moved the common code to a separate local function to help prevent future maintenance headaches.
Oh, I guess some explanation would be good too. The NS_RELEASE was causing problems because the pointer can be null, as indicated by surrounding code. This referencing of a null pointer was getting caught by Win32's structured exception handling. Fixing this, I couldn't reproduce There's two possibilities from here. 1. The caught exception left objects in an odd state and this was the source of the faults in JS 2. Correcting this problem effectively hides what's causing these crashes There's still a lot of asserts firing, but I was getting those before.
That's great news David. Thanks a lot for the quick turn around on this. I'll try your patch out in thunderbird today and verify that it also works for me.
Comment on attachment 127462 [details] [diff] [review] Fixes the null pointer and consolidates the code - In NotifyListBoxBody(nsIPresContext* aPresContext, + nsIContent* aContainer, + nsIContent* aChild, + PRInt32 aIndexInContainer, + nsIDocument* aDocument, + nsIFrame* aChildFrame, + PRBool useXBLForms, + content_operation operation) Maybe rename useXBLForms to aUseXBLForms and operation to aOperation to be consistent with the other arguments? +{ + if (aContainer) { ... } + return PR_FALSE; How about: +{ + if (!aContainer) { + return PR_FALSE; + } + ... to avoid indenting the whole function? + PRBool listitem = tag && tag.get() == nsXULAtoms::listitem; + if (listitem) { |listitem| isn't used for anything except that if check, maybe loose it, and change the above to: + if (tag == nsXULAtoms::listitem; (no need for the .get(), and nsXULAtoms::listitem isn't null unless something went really wrong during startup, in which case we should never get here). - In nsCSSFrameConstructor::ContentInserted(): + if (aContainer && NotifyListBoxBody(aPresContext, aContainer, aChild, + aIndexInContainer, mDocument, nsnull, + UseXBLForms(), CONTENT_INSERTED)) NotifyListBoxBody returns false if (!aContainer), so no need to check it here before making the call. - In nsCSSFrameConstructor::ContentRemoved(): + if (aContainer && NotifyListBoxBody(aPresContext, aContainer, aChild, + aIndexInContainer, mDocument, childFrame, + UseXBLForms(), CONTENT_REMOVED)) Same thing, no need to check aContainer. sr=jst
Attachment #127462 - Flags: superreview+
Who will r=? Should we get this into 1.5a for some widely-distributed testing? /be
Comment on attachment 127462 [details] [diff] [review] Fixes the null pointer and consolidates the code What jst said, and... >+static >+PRBool NotifyListBoxBody(nsIPresContext* aPresContext, .... >+ } else { >+ // There is no frame for the removed/inserted content, so we need to dig >+ // further for the body frame. XBL insertion may not yet have taken >+ // place here (in the case of replacements), so our only connection Nit: Could you remove the weird double space in between "of" and "replacements"? >+ // to the listbox content is through aContainer. Use the boxObject >+ // to get to the listboxbody frame so we can notify it of the removal. This comment applies to both insertion and removal cases. Please reflect that in the last line of the comment (it currently pertains to only removal).
Attachment #127462 - Flags: review+
Should I seek approval for 1.5a? Given that this is blocking a feature for Thunderbird and might be responsible for some other similiar screwy crashes it seems like a decent win. I also noticed about a crash a day listed for the ContentRemoved method as well.
Status: NEW → ASSIGNED
Patch checked in
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Crash Signature: [@JS_GetPrivate]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: