9.17 KB, text/html
6.37 KB, patch
|Details | Diff | Splinter Review|
8.20 KB, patch
|Details | Diff | Splinter Review|
891 bytes, patch
|Details | Diff | Splinter Review|
This bug has been automatically processed, reduced, and uploaded by Paul's Automated Pen-Tester alpha. It still may require more work. Firefox version: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:18.104.22.168) Gecko/20070812 BonEcho/22.214.171.124 Details: eax=dddddddd ebx=7ffde000 ecx=dddddddd edx=dddddddd esi=00a078d8 edi=00011970 eip=0197f53a esp=0012df74 ebp=0012df78 iopl=0 nv up ei ng nz na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000286 *** WARNING: Unable to verify checksum for C:\mozilla\mozilla\firefox-debug\dist\bin\components\gklayout.dll gklayout!nsIFrame::GetStateBits+0xa: 0197f53a 8b4024 mov eax,dword ptr [eax+24h] ds:0023:ddddde01=???????? Stack trace: gklayout!nsIFrame::GetStateBits(void) gklayout!IsFrameSpecial( class nsIFrame * aFrame = 0xdddddddd) gklayout!GetIBContainingBlockFor( class nsIFrame * aFrame = 0x03c9fa60) gklayout!nsCSSFrameConstructor::ReframeContainingBlock( class nsIFrame * aFrame = 0x03c9fa60) gklayout!nsCSSFrameConstructor::ContentRemoved( class nsIContent * aContainer = 0x03c56a20, class nsIContent * aChild = 0x03c57148, int aIndexInContainer = 0, int aInReinsertContent = 0) gklayout!nsCSSFrameConstructor::RecreateFramesForContent( class nsIContent * aContent = 0x03c57148) gklayout!nsCSSFrameConstructor::ProcessRestyledFrames( class nsStyleChangeList * aChangeList = 0x0012e284) gklayout!nsIPresShell::ReconstructStyleDataInternal(void) gklayout!nsIPresShell::ReconstructStyleData(void) gklayout!PresShell::EndUpdate( class nsIDocument * aDocument = 0x033978c8, unsigned int aUpdateType = 2) gklayout!nsDocument::EndUpdate( unsigned int aUpdateType = 2) gklayout!nsStyleLinkElement::UpdateStyleSheet( class nsIDocument * aOldDocument = 0x033978c8, class nsICSSLoaderObserver * aObserver = 0x00000000) gklayout!nsHTMLStyleElement::UnbindFromTree( int aDeep = 1, int aNullParent = 1) gklayout!doRemoveChildAt( unsigned int aIndex = 4, int aNotify = 1, class nsIContent * aKid = 0x03cb7848, class nsIContent * aParent = 0x035cadc8, class nsIDocument * aDocument = 0x033978c8, class nsAttrAndChildArray * aChildArray = 0x035cade0) gklayout!nsGenericElement::RemoveChildAt( unsigned int aIndex = 4, int aNotify = 1) gklayout!nsGenericElement::RemoveChild( class nsIDOMNode * aOldChild = 0x03cb7864, class nsIDOMNode ** aReturn = 0x0012ea68) gklayout!nsHTMLHeadElement::RemoveChild( class nsIDOMNode * aOldChild = 0x03cb7864, class nsIDOMNode ** aReturn = 0x0012ea68) xpcom_core!XPTC_InvokeByIndex( class nsISupports * that = 0x035cade4, unsigned int methodIndex = 0x11, unsigned int paramCount = 2, struct nsXPTCVariant * params = 0x0012ea58) xpc3250!XPCWrappedNative::CallMethod( class XPCCallContext * ccx = 0x0012ebd4, XPCWrappedNative::CallMode mode = CALL_METHOD (0)) xpc3250!XPC_WN_CallMethod( struct JSContext * cx = 0x032f2e60, struct JSObject * obj = 0x02d246b8, unsigned int argc = 1, long * argv = 0x03ab64b4, long * vp = 0x0012ed34) A hash of the backtrace has been used to distinguish this from other bugs already reported by the tester. However, the automated pen-tester is still in development and this may not be the case. Also, I haven't added stack hashes to bugs that were not uploaded by the tester, so this bug may very well exist on bugzilla already.
Well, on the plus side, the pen-tester worked (almost) the whole way through. Still gotta work on the attachment uploader. On the down side, it reported the same bug as last time :P
Actually, looking back at this thing, I changed my mind about the duplicate.
NotifyListBoxBody() calls listBoxObject->GetListBoxBody() in an attempt to get the frame, but that flushes frames so end up with a recursive call to RecreateFramesForContent()... see attached stack. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1110.6.81&root=/cvsroot&mark=9220#9196 http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/nsBoxObject.cpp&rev=126.96.36.199&root=/cvsroot&mark=170#164
Created attachment 276758 [details] [diff] [review] Wallpaper This is a somewhat low-risk wallpaper we could take on branches unless someone has a better idea.
We need a version of GetListBoxBody that doesn't flush frames to use here. The weakframe approach is sorta-maybe ok, but I'm not sure I would trust it to insure against all possible ways to try to damage the frame tree....
Created attachment 276921 [details] [diff] [review] Branch patch
It looks to me like this could also crash on trunk since GetListBoxBody() leads to a Flush_Frames there too. But the testcase doesn't trigger the RecreateFramesForContent() call on trunk so we don't reach NotifyListBoxBody(). I'm guessing a slightly different testcase would crash on trunk though.
Created attachment 276930 [details] [diff] [review] Branch patch
Comment on attachment 276930 [details] [diff] [review] Branch patch We're not changing interfaces on branch. Please add a new interface with the new method on it instead, and QI as needed. See the various MOZILLA_1_8_BRANCH stuff on the branch. On trunk this approach looks good, though.
> We're not changing interfaces on branch. Ok, I somehow thought we could change the "private" ones, i.e. those with "nsPI" in the name...
Created attachment 276967 [details] [diff] [review] Branch patch, rev. 3
Created attachment 276969 [details] [diff] [review] Branch patch, rev. 4 ... shouldn't change IID for nsPIListBoxObject of course...
Comment on attachment 276969 [details] [diff] [review] Branch patch, rev. 4 Looks good. I agree that we should do this on trunk too.
Comment on attachment 276969 [details] [diff] [review] Branch patch, rev. 4 Is this attachment ready for a branch approval request? If this is an appropriate fix for the trunk could we land it there first for some sanity checking?
Mats: is this fix going to make the 188.8.131.52 code freeze (Oct 3)?
Comment on attachment 276969 [details] [diff] [review] Branch patch, rev. 4 It's ready for both branches.
Created attachment 283297 [details] [diff] [review] Trunk patch, rev. 1
Comment on attachment 283297 [details] [diff] [review] Trunk patch, rev. 1 r+sr=bzbarsky
Comment on attachment 283297 [details] [diff] [review] Trunk patch, rev. 1 Low-risk crash fix, if I can get this landed *now* it will give some "baking value" for the branch fix as well.
Comment on attachment 276969 [details] [diff] [review] Branch patch, rev. 4 approved for 184.108.40.206 and 220.127.116.11, a=dveditz
MOZILLA_1_8_BRANCH mozilla/layout/base/nsCSSFrameConstructor.cpp 1.1110.6.90 mozilla/layout/xul/base/src/nsListBoxObject.cpp 18.104.22.168 mozilla/layout/xul/base/src/nsPIListBoxObject.h 22.214.171.124 MOZILLA_1_8_0_BRANCH mozilla/layout/base/nsCSSFrameConstructor.cpp 1.1126.96.36.199.65 mozilla/layout/xul/base/src/nsListBoxObject.cpp 188.8.131.52 mozilla/layout/xul/base/src/nsPIListBoxObject.h 184.108.40.206
trunk: mozilla/layout/base/nsCSSFrameConstructor.cpp 1.1407 mozilla/layout/xul/base/src/nsListBoxObject.cpp 1.25 mozilla/layout/xul/base/src/nsPIListBoxObject.h 1.3 -> FIXED
verified fixed on trunk with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a9pre) Gecko/2007101204 Minefield/3.0a9pre ID:2007101204 also verified fixed 220.127.116.11 using Mozilla/5.0 (Windows; U; Windows NT 5.2; de; rv:18.104.22.168) Gecko/2007100816 Firefox/22.214.171.124 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ja-JP-mac; rv:126.96.36.199) Gecko/2007100816 Firefox/188.8.131.52 No crash on testcase - adding verified keyword
Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:184.108.40.206pre) Gecko/20071210 Firefox/220.127.116.11pre. It still crashes in 18.104.22.168 Firefox but not in the current branch build above. Marked as "Fixed" since this is a branch only bug.
Created attachment 8510623 [details] [diff] [review] REINTRODUCE-BUG-392285 I tried reintroducing the bug to see if the attached test would crash but it didn't.