Closed Bug 269566 Opened 20 years ago Closed 20 years ago

[FIX]Crash after 2 or 3 reloads [@ nsCachedStyleData::GetStyleData]

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8beta1

People

(Reporter: dromaouira, Assigned: bzbarsky)

References

()

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(5 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041111 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041111 Firefox/0.9.1+ Talkback was unfortunately not available (why ?) at installation time. Reproducible: Always Steps to Reproduce: 1. Load http://www.tiscali.be/FR/home/home_center.asp . 2. Reload 2 or 3 times. Actual Results: Crash. Expected Results: No crash. Firefox 1.0 displays the page without problem.
Firefox does not show anything from the page (blank). This page is hosted by my ISP. I truly hope that they don't restrict its access to their IP range. Otherwise I'm out of luck and I'll have to hope that someone else with the same problem will provide a good testcase in another bug report.
Version: unspecified → Trunk
reporter: you can try selecting custom install, in order to avoid flooding the talkback servers, windows installer builds randomly decide not to install talkback. that said, here's a stack: > gklayout.dll!nsCachedStyleData::GetStyleData(const nsStyleStructID & aSID=eStyleStruct_Visibility) Line 210 + 0x3 C++ gklayout.dll!nsStyleContext::GetStyleData(nsStyleStructID aSID=eStyleStruct_Visibility) Line 247 + 0xf C++ gklayout.dll!nsIFrame::GetStyleData(nsStyleStructID aSID=eStyleStruct_Visibility) Line 616 C++ gklayout.dll!nsIFrame::GetStyleVisibility() Line 98 + 0x11 C++ gklayout.dll!nsImageFrame::FrameChanged(imgIContainer * aContainer=0x035d2260, gfxIImageFrame * aNewFrame=0x035f8480, nsRect * aDirtyRect=0x0012fc40) Line 702 + 0x8 C++ gklayout.dll!nsImageListener::FrameChanged(imgIContainer * aContainer=0x035d2260, gfxIImageFrame * newframe=0x035f8480, nsRect * dirtyRect=0x0012fc40) Line 2139 C++ gklayout.dll!nsImageLoadingContent::FrameChanged(imgIContainer * aContainer=0x035d2260, gfxIImageFrame * aFrame=0x035f8480, nsRect * aDirtyRect=0x0012fc40) Line 160 + 0x4f C++ imglib2.dll!imgRequestProxy::FrameChanged(imgIContainer * container=0x035d2260, gfxIImageFrame * newframe=0x035f8480, nsRect * dirtyRect=0x0012fc40) Line 366 C++ imglib2.dll!imgRequest::FrameChanged(imgIContainer * container=0x035d2260, gfxIImageFrame * newframe=0x035f8480, nsRect * dirtyRect=0x0012fc40) Line 375 C++ imglib2.dll!imgContainerGIF::Notify(nsITimer * timer=0x0373b878) Line 456 C++ xpcom_core.dll!nsTimerImpl::Fire() Line 387 C++ xpcom_core.dll!nsTimerManager::FireNextIdleTimer() Line 617 C++ gkwidget.dll!nsAppShell::Run() Line 142 C++ appcomps.dll!nsAppStartup::Run() Line 221 C++ mozilla.exe!main1(int argc=0x00000001, char * * argv=0x00347b80, nsISupports * nativeApp=0x0109ef38) Line 1321 + 0x20 C++ mozilla.exe!main(int argc=0x00000001, char * * argv=0x00347b80) Line 1813 + 0x25 C++ mozilla.exe!mainCRTStartup() Line 400 + 0x11 C kernel32.dll!TermsrvAppInstallMode() + 0x269 + info {mCachedStyleDataOffset=0x00000000 mInheritResetOffset=0x00000010 mIsReset=0x00000000 } const nsCachedStyleData::StyleStructInfo & info.mCachedStyleDataOffset 0x00000000 int + resetOrInherit 0x037499c0 "WôPT" char * + resetOrInheritSlot 0xddddddf9 <Bad Ptr> char * + this 0xddddddf9 {gInfo=0x01fec970 struct nsCachedStyleData::StyleStructInfo * nsCachedStyleData::gInfo mInheritedData=??? mResetData=??? } nsCachedStyleData * const Unhandled exception at 0x01b30e28 (gklayout.dll) in mozilla.exe: 0xC0000005: Access violation reading location 0xddddddf9. problem starts here: > gklayout.dll!nsImageListener::FrameChanged(imgIContainer * aContainer=0x035d2260, gfxIImageFrame * newframe=0x035f8480, nsRect * dirtyRect=0x0012fc40) Line 2139 C++ + aContainer 0x035d2260 {mRefCnt={mValue=0x00000002 } _mOwningThread={mThread=0x00345278 } mObserver={mRawPtr=0x03622668 {mRefCount=0x00000001 mReferent=0x035b7d1c {mRefCnt={...} _mOwningThread={...} mChannel={...} ...} } } ...} imgIContainer * + dirtyRect 0x0012fc40 {x=0x00000000 y=0x00000000 width=0x000001d4 ...} nsRect * + mFrame 0x0375a19c {mImageMap=0xdddddddd {mRefCnt={mValue=??? } _mOwningThread={mThread=??? } mPresShell=??? ...} mListener={mRawPtr=0xdddddddd } mComputedSize={width=0xdddddddd height=0xdddddddd } ...} nsImageFrame * + newframe 0x035f8480 {mRefCnt={mValue=0x00000001 } _mOwningThread={mThread=0x00345278 } mSize={width=0x000001d4 height=0x0000003c } ...} gfxIImageFrame * + this 0x0374ac08 {mRefCnt={mValue=0x00000002 } _mOwningThread={mThread=0x00345278 } mFrame=0x0375a19c {mImageMap=0xdddddddd {mRefCnt={mValue=??? } _mOwningThread={mThread=??? } mPresShell=??? ...} mListener={mRawPtr=0xdddddddd } mComputedSize={width=0xdddddddd height=0xdddddddd } ...} } nsImageListener * const
Assignee: firefox → dbaron
Component: General → Style System (CSS)
Keywords: crash
Product: Firefox → Browser
QA Contact: firefox.general → ian
Summary: Crash after 2 or 3 reloads → Crash after 2 or 3 reloads [@ nsCachedStyleData::GetStyleData]
fwiw there were no asserts of interest. debug build is trunk from probably about the middle of last week. i can check later if people care.
Attached image Image for testcase (deleted) —
Attached file Minimal testcase (deleted) —
The basic problem is that ConstructHTMLFrame doesn't get a pseudo-parent, I think. I'll do more debugging tomorrow...
Assignee: dbaron → nobody
Severity: major → critical
Status: UNCONFIRMED → NEW
Component: Style System (CSS) → Layout: Misc Code
Ever confirmed: true
OS: Windows XP → All
QA Contact: ian → core.layout.misc-code
Hardware: PC → All
not that it really matters, but loading the testcase (by clicking edit from this bug) and then clicking back in Camino crashed my poor Camino [Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20041110 Camino/0.8+]
The testcase kills my 1.0 branch build as well. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041108 Firefox/1.0 (MOOX M2)
(In reply to comment #8) > The testcase kills my 1.0 branch build as well. > > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041108 > Firefox/1.0 (MOOX M2) Ran reload about 10-12 times here with no problems. I didn't allow the popup window to load. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Guys, we know where the crash happens and why it happens. It's nondeterministic in release builds (so it'll only happen sometimes). In a debug build it will happen with 100% certainty when the testcase page is closed. Please stop spamming the bug long enough for us to work on a fix, ok? ;)
Keywords: testcase
Attached file Testcase2 (deleted) —
Some time ago, I also made a testcase of that ur, without knowing the existence of this bug. It's almost the same, but this one uses an iframe. It crashes always for me on closing the tab.
Blocks: 275625
Flags: blocking-aviary1.1?
Blocks: 275548
So I've been thinking about this, and I don't see a simple solution offhand, short of bailing out of ConstructHTMLFrame immediately if the parent is a table frame... That would not show content on the testcase in question, but that's better than crashing. Here are the problems I see: 1) We need to decide whether we have a pseudo-parent or not before constructing frames. 2) If we create a pseudo-parent in ConstructHTMLFrame but we're not looking at an HTML frame, what happens? 3) <img style="display: table-row"> -- what should this do? #2 is what worries me as far as just hacking this goes...
Blocks: 278266
Attached patch Proposed patch (deleted) — Splinter Review
This fixes this bug, bug 278266, and bug 275625. Basic changes: 1) Hoist the pseudo-frame construction out of ConstructFrameByDisplayType and into ConstructFrameInternal so we do it for all frame types as needed (fixes this bug and bug 278266). 2) Make sure to construct pseudo-frames even for HTML frames that claim to be of a table display type if we happen to know that they won't get a table-internal frame (fixes bug 275625). The rest is just propagating the damage around. I added a |aHasPseudoParent| arg to a few functions where it's currently unused but _should_ be used. I'd be ok with removing those args for now, pending a better way of doing pseudo-frames altogether. I've run the regression tests on this, and verified that the fixes for bug 2479, bug 3037, etc. did not regress.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #171234 - Flags: superreview?(dbaron)
Attachment #171234 - Flags: review?(bernd_mozilla)
Priority: -- → P1
Summary: Crash after 2 or 3 reloads [@ nsCachedStyleData::GetStyleData] → [FIX]Crash after 2 or 3 reloads [@ nsCachedStyleData::GetStyleData]
Target Milestone: --- → mozilla1.8beta
Comment on attachment 171234 [details] [diff] [review] Proposed patch >the sate... state? if (IsTableRelated(aParentFrame->GetType(), PR_FALSE) && + (!IsTableRelated(aChildDisplay->mDisplay, PR_TRUE) || please explain this if statement as it is the heart of the patch and the PR_FALSE and then PR_TRUE argument is not obvious (while correct I believe) >// XXXbz somewhere here we should process pseudo frames if !aHasPseudoParent does that mean we will crash if hit this code?
Attachment #171234 - Flags: review?(bernd_mozilla) → review+
> state? Yes. > please explain this if statement Will do. I just lifted it from ConstructFrameByDisplayType, of course... ;) > does that mean we will crash if hit this code? I'm not actually sure. We may just end up with frames in the wrong places in the frame tree. But yes, we may lose frames altogether and leak and/or crash as a result.
Boris: could you check that we don't need something like https://bugzilla.mozilla.org/attachment.cgi?id=170350 from bug 277035 at the if statement that I asked you to better comment. In other words what happens if these html frames have display:none specified.
Bernd, there's a check for that right before the place where I put the AdjustParentFrame call. We never get to that if in the case when the display is "none".
Blocks: 275746
Attachment #171234 - Flags: superreview?(dbaron) → superreview+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attached patch Followup patch (deleted) — Splinter Review
I got the ordering of args to nsINodeInfo::Equals wrong, and this still compiled because one of the args was a macro for 0. I just checked this in, with r=sicking.
Verified FIXED; both testcase 1: https://bugzilla.mozilla.org/attachment.cgi?id=165961 and testcase 2: https://bugzilla.mozilla.org/attachment.cgi?id=169367 using build 2005-01-26-06 on Windows XP Seamonkey trunk builds.
Status: RESOLVED → VERIFIED
Flags: blocking-aviary1.1?
Would be nice to figure out a way to crashtest this.
Flags: in-testsuite?
layout/base/crashtests/269566-1.html http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsCachedStyleData::GetStyleData]
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: