Closed
Bug 140218
Opened 23 years ago
Closed 18 years ago
Crash dereferencing box the QI result of a null mTreeBoxObject [@nsTreeBodyFrame::SetView] [@ nsTreeBodyFrame::GetMinSize]
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
VERIFIED
FIXED
People
(Reporter: timeless, Assigned: MatsPalmgren_bugz)
References
Details
(4 keywords)
Crash Data
Attachments
(4 files, 2 obsolete files)
(deleted),
text/xml
|
Details | |
(deleted),
text/xml
|
Details | |
(deleted),
patch
|
bzbarsky
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dveditz
:
approval1.8.1.5+
dveditz
:
approval1.8.0.13+
|
Details | Diff | Splinter Review |
cvs trunk w2k nondebug profile - box {...} \- nsCOMPtr_base {...} \+ mRawPtr 0x00000000 - mTreeBoxObject {...} \- nsCOMPtr_base {...} \+ mRawPtr 0x00000000 box->SetPropertyAsSupports(view.get(), mView); nsTreeBodyFrame::SetView(nsTreeBodyFrame * const 0x0273f000, nsITreeView * 0x00000000) line 654 + 9 bytes nsTreeBodyFrame::GetPrefSize(nsTreeBodyFrame * const 0x02775c48, nsBoxLayoutState & {...}, nsSize & {...}) line 442 nsBox::GetAscent(nsBox * const 0x0273eff0, nsBoxLayoutState & {...}, int &) line 1032 nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0273eff0, nsBoxLayoutState & {...}, int &) line 1521 nsContainerBox::GetAscent(nsContainerBox * const 0x0273ef1c, nsBoxLayoutState & {...}, int &) line 591 nsBoxFrame::GetAscent(nsBoxFrame * const 0x0273ef1c, nsBoxLayoutState & {...}, int & 0) line 1101 nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0273ef1c, nsBoxLayoutState & {...}, int &) line 1521 nsContainerBox::GetAscent(nsContainerBox * const 0x0273ee3c, nsBoxLayoutState & {...}, int &) line 591 nsBoxFrame::GetAscent(nsBoxFrame * const 0x0273ee3c, nsBoxLayoutState & {...}, int & 0) line 1101 nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0273ee3c, nsBoxLayoutState & {...}, int &) line 1521 nsContainerBox::GetAscent(nsContainerBox * const 0x0276532c, nsBoxLayoutState & {...}, int &) line 591 nsBoxFrame::GetAscent(nsBoxFrame * const 0x0276532c, nsBoxLayoutState & {...}, int & 0) line 1101 nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0276532c, nsBoxLayoutState & {...}, int &) line 1521 nsContainerBox::GetAscent(nsContainerBox * const 0x0275ab98, nsBoxLayoutState & {...}, int &) line 591 nsBoxFrame::GetAscent(nsBoxFrame * const 0x0275ab98, nsBoxLayoutState & {...}, int & 0) line 1101 nsSprocketLayout::Layout(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0275ab98, nsBoxLayoutState & {...}) line 245 nsContainerBox::DoLayout(nsContainerBox * const 0x0275ab98, nsBoxLayoutState & {...}) line 606 + 8 bytes nsBox::Layout(nsBox * const 0x0275ab98, nsBoxLayoutState & {...}) line 1052 nsPopupSetFrame::DoLayout(nsPopupSetFrame * const 0x0253c90c, nsBoxLayoutState & {...}) line 253 nsBox::Layout(nsBox * const 0x0253c90c, nsBoxLayoutState & {...}) line 1052 nsSprocketLayout::Layout(nsSprocketLayout * const 0x01adb530, nsIBox * 0x0253c7f8, nsBoxLayoutState & {...}) line 529 nsContainerBox::DoLayout(nsContainerBox * const 0x0253c7f8, nsBoxLayoutState & {...}) line 606 + 8 bytes nsBox::Layout(nsBox * const 0x0253c7f8, nsBoxLayoutState & {...}) line 1052 nsStackLayout::Layout(nsStackLayout * const 0x01bd0998, nsIBox * 0x0253c618, nsBoxLayoutState & {...}) line 331 nsContainerBox::DoLayout(nsContainerBox * const 0x0253c618, nsBoxLayoutState & {...}) line 606 + 8 bytes nsBox::Layout(nsBox * const 0x0253c618, nsBoxLayoutState & {...}) line 1052 nsBoxFrame::Reflow(nsBoxFrame * const 0x0253c5e4, nsIPresContext * 0x01b1dd98, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1001 nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x0253c5e4, nsIPresContext * 0x01b1dd98, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 243 nsContainerFrame::ReflowChild(nsContainerFrame * const 0x00000000, nsIFrame * 0x0253c5e4, nsIPresContext * 0x01b1dd98, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0, unsigned int 0, unsigned int & 0) line 807 ViewportFrame::Reflow(ViewportFrame * const 0x0253c5ac, nsIPresContext * 0x01b1dd98, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 588 nsHTMLReflowCommand::Dispatch(nsHTMLReflowCommand * const 0x00000000, nsIPresContext * 0x01b1dd98, nsHTMLReflowMetrics & {...}, const nsSize & {...}, nsIRenderingContext & {...}) line 224 PresShell::ProcessReflowCommand(PresShell * const 0x00000000, nsVoidArray & {...}, int 0, nsHTMLReflowMetrics & {...}, nsSize & {...}, nsIRenderingContext & {...}) line 6189 PresShell::ProcessReflowCommands(PresShell * const 0x00000000, int 0) line 6244 PresShell::FlushPendingNotifications(PresShell * const 0x00000001, int 0) line 5049 nsXULDocument::FlushPendingNotifications(nsXULDocument * const 0x01ae9968, int 1, int 0) line 2501 + 13 bytes nsXBLResourceLoader::NotifyBoundElements(nsXBLResourceLoader * const 0x00000000) line 281 nsXBLResourceLoader::StyleSheetLoaded(nsXBLResourceLoader * const 0x02702108, nsICSSStyleSheet * 0x02702108, int 1) line 207 CSSLoaderImpl::InsertSheetInDoc(CSSLoaderImpl * const 0x00000000, nsICSSStyleSheet * 0x0272b8d8, int 2, nsIContent * 0x00000000, int 1, nsICSSLoaderObserver * 0x027699a8) line 1194 + 10 bytes InsertPendingSheet(void * 0x02702038, void * 0x02683ba0) line 757 nsStringArray::EnumerateForwards(nsStringArray * const 0x00000000, int (nsString &, void *)* 0x01e03876 InsertPendingSheet(void *, void *), void * 0x02683ba0) line 660 + 11 bytes CSSLoaderImpl::Cleanup(CSSLoaderImpl * const 0x00000000, URLKey & {...}, SheetLoadData * 0x0276eed0) line 821 CSSLoaderImpl::SheetComplete(CSSLoaderImpl * const 0x00000000, nsICSSStyleSheet * 0x00000000, SheetLoadData * 0x0276eed0) line 914 CSSLoaderImpl::ParseSheet(CSSLoaderImpl * const 0x00000000, nsIUnicharInputStream * 0x02731168, SheetLoadData * 0x00000000, int & 1, nsICSSStyleSheet * & 0x01eb96d8 const CSSParserImpl::`vftable') line 949 CSSLoaderImpl::DidLoadStyle(CSSLoaderImpl * const 0x00000000, nsIStreamLoader * 0x0276edd0, nsString * 0x00000001, SheetLoadData * 0x0272b9e8, unsigned int 41095528) line 985 SheetLoadData::OnStreamComplete(SheetLoadData * const 0x0276eed0, nsIStreamLoader * 0x0276edd0, nsISupports * 0x00000000, unsigned int 0, unsigned int 6675, const char * 0x0277e890) line 745 nsStreamLoader::OnStopRequest(nsStreamLoader * const 0x00000000, nsIRequest * 0x0276eed0, nsISupports * 0x00000000, unsigned int 0) line 163 nsJARChannel::OnStopRequest(nsJARChannel * const 0x02667f44, nsIRequest * 0x0274c55c, nsISupports * 0x00000000, unsigned int 0) line 606 + 28 bytes nsOnStopRequestEvent::HandleEvent(nsOnStopRequestEvent * const 0x00000000) line 213 PL_HandleEvent(PLEvent * 0x02727e94) line 597 PL_ProcessPendingEvents(PLEventQueue * 0x1003342f) line 526 + 6 bytes _md_EventReceiverProc(HWND__ * 0x00cb6128, unsigned int 4202408, unsigned int 13196728, long 2013534116) line 1078 nsAppShellService::Run(nsAppShellService * const 0x00c95db8) line 451 main1(int 3, char * * 0x00262ed8, nsISupports * 0x00262f20) line 1431 + 9 bytes main(int 3, char * * 0x00262ed8) line 1779 + 26 bytes WinMain(HINSTANCE__ * 0x00400000, HINSTANCE__ * 0x00400000, char * 0x001334b9, HINSTANCE__ * 0x00400000) line 1799 + 23 bytes MOZILLA! WinMainCRTStartup + 308 bytes KERNEL32! 77e97d08() The problem is in nsTreeBodyFrame::GetPrefSize(nsTreeBodyFrame * const 0x02775c48, nsBoxLayoutState & {...}, nsSize & {...}) line 442 The code is basically: if (!mView) { EnsureBoxObject(); nsCOMPtr<nsIBoxObject> box = do_QueryInterface(mTreeBoxObject); if (box) { ... } if (!mView) { ... if (view) SetView(view); // crashes because !box ... } }
Comment 2•22 years ago
|
||
related: bug 139656 ?
Comment 3•22 years ago
|
||
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and <http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss bugs are of critical or possibly higher severity. Only changing open bugs to minimize unnecessary spam. Keywords to trigger this would be crash, topcrash, topcrash+, zt4newcrash, dataloss.
Severity: normal → critical
Assignee: Jan.Varga → timeless
Status: NEW → ASSIGNED
Attachment #214654 -
Flags: superreview?(neil)
Attachment #214654 -
Flags: review?(neil)
Comment 8•19 years ago
|
||
Comment on attachment 214654 [details] [diff] [review] null check? Not sure that comment 6 counts as moa ;-)
Attachment #214654 -
Flags: superreview?(neil)
Attachment #214654 -
Flags: superreview+
Attachment #214654 -
Flags: review?(neil)
Comment 9•19 years ago
|
||
Comment on attachment 214654 [details] [diff] [review] null check? Sure does
Attachment #214654 -
Flags: review+
Reporter | ||
Comment 10•19 years ago
|
||
Comment on attachment 214654 [details] [diff] [review] null check? mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp 1.267
Attachment #214654 -
Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 11•19 years ago
|
||
I still see a crash with both testcases. (Testing on Mac with a tinderbox build and my own debug build.) New stack: 0 nsTreeBodyFrame::GetMinSize(nsBoxLayoutState&, nsSize&) + 44 1 nsBox::GetPrefSize(nsBoxLayoutState&, nsSize&) + 156 2 nsBox::GetAscent(nsBoxLayoutState&, int&) + 112 3 nsSprocketLayout::GetAscent(nsIFrame*, nsBoxLayoutState&, int&) + 156 4 nsBoxFrame::GetAscent(nsBoxLayoutState&, int&) + 156 5 nsSprocketLayout::Layout(nsIFrame*, nsBoxLayoutState&) + 432 6 nsBoxFrame::DoLayout(nsBoxLayoutState&) + 68 ...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 12•19 years ago
|
||
neil: any ideas? :(
Assignee | ||
Comment 13•18 years ago
|
||
GetBaseElement() return null if there is no <select> or <tree> ancestor: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp&rev=1.275&root=/cvsroot&mark=3873-3875#3865 This patch adds the missing null checks for all calls to GetBaseElement().
Attachment #221388 -
Flags: superreview?(neil)
Attachment #221388 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•18 years ago
|
OS: Windows 2000 → All
Hardware: PC → All
Comment 14•18 years ago
|
||
Comment on attachment 221388 [details] [diff] [review] Additional patch, rev. 1 r=bzbarsky. Good to have you back, Mats!
Attachment #221388 -
Flags: review?(bzbarsky) → review+
Updated•18 years ago
|
Attachment #221388 -
Flags: superreview?(neil) → superreview+
Updated•18 years ago
|
Summary: Crash [@nsTreeBodyFrame::SetView] dereferencing box the QI result of a null mTreeBoxObject → Crash dereferencing box the QI result of a null mTreeBoxObject [@nsTreeBodyFrame::SetView] [@ nsTreeBodyFrame::GetMinSize]
Updated•18 years ago
|
Assignee: timeless → mats.palmgren
Status: REOPENED → NEW
Assignee | ||
Comment 15•18 years ago
|
||
Checked in "Additional patch, rev. 1" to trunk at 2006-06-03 09:57 PDT: Checking in layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp; /cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp,v <-- nsTreeBodyFrame.cpp new revision: 1.278; previous revision: 1.277 done
Status: NEW → RESOLVED
Closed: 19 years ago → 18 years ago
Resolution: --- → FIXED
Comment 16•17 years ago
|
||
This is a combination of attachment 221388 [details] [diff] [review] and attachment 214654 [details] [diff] [review], minus the change to GetScrollParts(), because I don't understand why it's needed (GetPrimaryFrameFor seems to null check it's argument and return null as needed, on branch and trunk). Mats, this should be fine for the branch, right?
Attachment #264111 -
Flags: review?
Updated•17 years ago
|
Attachment #264111 -
Flags: review? → review?(mats.palmgren)
Assignee | ||
Comment 18•17 years ago
|
||
Comment on attachment 264111 [details] [diff] [review] combined branch patch (In reply to comment #16) > ... because I don't understand why it's needed One shouldn't call GetPrimaryFrameFor with null content - it will result in a warning in debug builds here: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsFrameManager.cpp&rev=1.245&root=/cvsroot&mark=324#320 > Mats, this should be fine for the branch, right? Yep, adding null-checks is pretty safe and this has baked a long time. r=mats, but please also include the change to GetScrollParts() - not because a warning more or less means much on branch, but rather to keep the code differences to a minimum. In general we should merge patches as is without changes (unless there's a specific reason not to). It also lowers the risk of accidental mistakes in the process.
Attachment #264111 -
Flags: review?(mats.palmgren) → review+
Comment 19•17 years ago
|
||
Ah, right, I forgot about the NS_ENSURE_* macro warnings. I'll port an updated patch and request review, thanks!
Comment 20•17 years ago
|
||
Attachment #264111 -
Attachment is obsolete: true
Attachment #264132 -
Flags: approval1.8.1.5?
Assignee | ||
Comment 21•17 years ago
|
||
Comment on attachment 264132 [details] [diff] [review] combined branch patch Good for 1.8.0 branch too
Attachment #264132 -
Flags: approval1.8.0.13?
Assignee | ||
Comment 22•17 years ago
|
||
Fixing bug 374102 is required before this goes on branch though...
Comment 23•17 years ago
|
||
(In reply to comment #22) > Fixing bug 374102 is required before this goes on branch though... Are you sure? Bug 374103 was filed because bug 374102 supposedly didn't affect the branch (I can't reproduce it, anyways).
Assignee | ||
Comment 24•17 years ago
|
||
Yes. See bug 374102 comment 7 and forward.
Comment 25•17 years ago
|
||
Comment on attachment 264132 [details] [diff] [review] combined branch patch approved for 1.8.1.5 and 1.8.0.13, a=dveditz for release-drivers
Attachment #264132 -
Flags: approval1.8.1.5?
Attachment #264132 -
Flags: approval1.8.1.5+
Attachment #264132 -
Flags: approval1.8.0.13?
Attachment #264132 -
Flags: approval1.8.0.13+
Assignee | ||
Comment 26•17 years ago
|
||
Comment on attachment 264132 [details] [diff] [review] combined branch patch Checked in to MOZILLA_1_8_BRANCH: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp: 1.240.10.10 Checked in to MOZILLA_1_8_0_BRANCH: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp: 1.240.10.1.2.9
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite?
Keywords: fixed1.8.0.13,
fixed1.8.1.5
Comment 27•17 years ago
|
||
Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070710 BonEcho/2.0.0.5pre. Tested using a build from 2007-06-29-04 without Mats's patch and crashed on the first testcase and hung on the second, but using a build with the patch I don't crash or hang with either testcase.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.5 → verified1.8.1.5
Comment 28•17 years ago
|
||
This is verified fixed on 1.8.0.13 on Linux
Keywords: fixed1.8.0.13 → verified1.8.0.13
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets
Comment 29•16 years ago
|
||
Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/54417ebbaea2
Flags: in-testsuite? → in-testsuite+
Updated•13 years ago
|
Crash Signature: [@nsTreeBodyFrame::SetView]
[@ nsTreeBodyFrame::GetMinSize]
You need to log in
before you can comment on or make changes to this bug.
Description
•