Jesse Ruderman Reporter
	Description • 14 years ago

layout/xul/base/src/crashtests/432058-1.xul triggers this assertion.  It's not turning the tree orange because it's already marked as asserting in the manifest.

###!!! ASSERTION: Losing track of existing primary frame: '!aFrame || !mPrimaryFrame || aFrame == mPrimaryFrame', file ../../dist/include/nsIContent.h, line 875

nsIContent::SetPrimaryFrame(nsIFrame*) [nsIContent.h:876]
nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsIFrame*, nsFrameItems&) [layout/base/nsCSSFrameConstructor.cpp:3893]
nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) [layout/base/nsCSSFrameConstructor.cpp:5487]
nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsIFrame*, nsFrameItems&) [layout/base/nsCSSFrameConstructor.cpp:9552]
nsCSSFrameConstructor::CreateListBoxContent(nsPresContext*, nsIFrame*, nsIFrame*, nsIContent*, nsIFrame**, int, int, nsILayoutHistoryState*) [layout/base/nsCSSFrameConstructor.cpp:10623]
nsListBoxBodyFrame::GetFirstItemBox(int, int*) [layout/xul/base/src/nsListBoxBodyFrame.cpp:1154]
nsListBoxBodyFrame::CreateRows() [layout/xul/base/src/nsListBoxBodyFrame.cpp:1021]
nsListBoxBodyFrame::OnContentInserted(nsPresContext*, nsIContent*) [layout/xul/base/src/nsListBoxBodyFrame.cpp:1349]
NotifyListBoxBody [layout/base/nsCSSFrameConstructor.cpp:6846]
nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, int, int, nsILayoutHistoryState*, int) [layout/base/nsCSSFrameConstructor.cpp:6933]
nsCSSFrameConstructor::ContentInserted(nsIContent*, nsIContent*, int, nsILayoutHistoryState*, int) [layout/base/nsCSSFrameConstructor.cpp:6867]
nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, int) [layout/base/nsCSSFrameConstructor.cpp:9170]
nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList&) [layout/base/nsCSSFrameConstructor.cpp:8036]
nsCSSFrameConstructor::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::css::RestyleTracker&, int) [layout/base/nsCSSFrameConstructor.cpp:8125]
mozilla::css::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint) [layout/base/RestyleTracker.cpp:156]
mozilla::css::RestyleTracker::ProcessRestyles() [layout/base/RestyleTracker.cpp:216]
nsCSSFrameConstructor::ProcessPendingRestyles() [layout/base/nsCSSFrameConstructor.cpp:11693]
PresShell::FlushPendingNotifications(mozFlushType) [layout/base/nsPresShell.cpp:4785]
nsDocument::FlushPendingNotifications(mozFlushType) [content/base/src/nsDocument.cpp:6123]
nsBoxObject::GetPresShell(int) [layout/xul/base/src/nsBoxObject.cpp:185]
nsBoxObject::GetFrame(int) [layout/xul/base/src/nsBoxObject.cpp:149]
nsBoxObject::GetOffsetRect(nsIntRect&) [layout/xul/base/src/nsBoxObject.cpp:197]
nsBoxObject::GetHeight(int*) [layout/xul/base/src/nsBoxObject.cpp:299]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179]
CallMethodHelper::Invoke() [js/src/xpconnect/src/xpcwrappednative.cpp:3028]
CallMethodHelper::Call() [js/src/xpconnect/src/xpcwrappednative.cpp:2312]
XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [js/src/xpconnect/src/xpcwrappednative.cpp:2276]
XPCWrappedNative::GetAttribute(XPCCallContext&) [js/src/xpconnect/src/xpcprivate.h:2565]
XPC_WN_GetterSetter(JSContext*, JSObject*, unsigned int, long*, long*) [js/src/xpconnect/src/xpcwrappednativejsops.cpp:1833]
js_Invoke [js/src/jsinterp.cpp:654]
js_InternalInvoke [js/src/jsinterp.cpp:694]
js_InternalGetOrSet [js/src/jsinterp.cpp:730]
JSScopeProperty::get(JSContext*, JSObject*, JSObject*, long*) [js/src/jsscope.h:992]
js_NativeGet [js/src/jsobj.cpp:4758]
js_GetPropertyHelper [js/src/jsobj.cpp:4928]
js_Interpret [js/src/jsops.cpp:1485]
js_Invoke [js/src/jsinterp.cpp:664]
js_InternalInvoke [js/src/jsinterp.cpp:694]
JS_CallFunctionValue [js/src/jsapi.cpp:4634]
nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) [dom/base/nsJSEnvironment.cpp:2204]
nsJSEventListener::HandleEvent(nsIDOMEvent*) [dom/src/events/nsJSEventListener.cpp:228]
nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsPIDOMEventTarget*, unsigned int, nsCxPusher*) [content/events/src/nsEventListenerManager.cpp:1094]
nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsPIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) [content/events/src/nsEventListenerManager.cpp:1191]
nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsPIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) [content/events/src/nsEventListenerManager.h:146]
nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, unsigned int, int, nsCxPusher*) [content/events/src/nsEventDispatcher.cpp:213]
nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*, int, nsCxPusher*) [content/events/src/nsEventDispatcher.cpp:343]
nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<nsPIDOMEventTarget>*) [content/events/src/nsEventDispatcher.cpp:628]
DocumentViewerImpl::LoadComplete(unsigned int) [layout/base/nsDocumentViewer.cpp:1038]
nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, unsigned int) [docshell/base/nsDocShell.cpp:5757]
nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int) [docshell/base/nsDocShell.cpp:5637]
nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, int, unsigned int) [uriloader/base/nsDocLoader.cpp:1305]
nsDocLoader::doStopDocumentLoad(nsIRequest*, unsigned int) [uriloader/base/nsDocLoader.cpp:940]
nsDocLoader::DocLoaderIsEmpty(int) [uriloader/base/nsDocLoader.cpp:807]
nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) [uriloader/base/nsDocLoader.cpp:703]
nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, unsigned int) [netwerk/base/src/nsLoadGroup.cpp:680]
nsDocument::DoUnblockOnload() [content/base/src/nsDocument.cpp:6948]
nsDocument::UnblockOnload(int) [content/base/src/nsDocument.cpp:6886]
nsBindingManager::DoProcessAttachedQueue() [content/xbl/src/nsBindingManager.cpp:994]
nsRunnableMethodImpl<void (nsBindingManager::*)(), true>::Run() [nsThreadUtils.h:348]
nsThread::ProcessNextEvent(int, int*) [xpcom/threads/nsThread.cpp:547]
NS_ProcessPendingEvents_P(nsIThread*, unsigned int) [nsThreadUtils.cpp:200]
nsBaseAppShell::NativeEventCallback() [widget/src/xpwidgets/nsBaseAppShell.cpp:127]
nsAppShell::ProcessGeckoEvents(void*) [widget/src/cocoa/nsAppShell.mm:395]
CoreFoundation + 0x3f0fb
CoreFoundation + 0x3cbbf
CoreFoundation + 0x3c094
CoreFoundation + 0x3bec1
HIToolbox + 0x34f9c
HIToolbox + 0x34d51
HIToolbox + 0x34bd6
AppKit + 0x48a89
-AppKit + 0x482ca
-AppKit + 0xa55b
nsAppShell::Run() [widget/src/cocoa/nsAppShell.mm:747]
nsAppStartup::Run() [toolkit/components/startup/src/nsAppStartup.cpp:192]
XRE_main [toolkit/xre/nsAppRunner.cpp:3624]
main [browser/app/nsBrowserApp.cpp:158]
firefox-bin + 0x148e

	Boris Zbarsky [:bzbarsky]
	Comment 1 • 14 years ago

This is a real bug caught by the assert!

We're reframing the first listitem due to the display change.  So we call nsListBoxBodyFrame::OnContentInserted for the first listitem, which calls GetFirstItemBox via nsListBoxBodyFrame::CreateRows.

Now the <hbox>'s frame is a child of the box frame for the listbox, and NOT a child of the nsListBoxBodyFrame.  So we find as our first kid the second listitem look up the content at its index - 1, find the <hbox> and decide to create frames for it.

We have primary frame checks in this situation in GetNextItemBox, as well as checks for the right parent frame, and checks for xul listitems, etc.  Weseem to have none of that in GetFirstItemBox.

Timothy, you want to take a look?

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 14 years ago

And this is bad; it can leak frames.

	Timothy Nikkel (:tnikkel)
	Comment 3 • 14 years ago

I can take this, but not for a few weeks.

	Jesse Ruderman Reporter
	Comment 4 • 14 years ago

(In reply to comment #2)
> And this is bad; it can leak frames.

In the sense that a more complex testcase might trigger the "Some pres arena objects were not freed" assertion?  FWIW, I haven't seen that assertion in quite a while, so my fuzzer might be missing something.

	Boris Zbarsky [:bzbarsky]
	Comment 5 • 14 years ago

> In the sense that a more complex testcase might trigger the "Some pres arena
> objects were not freed" assertion?

I'd think this testcase would trigger that, in fact....

	Jesse Ruderman Reporter
	Comment 6 • 14 years ago

It doesn't for me.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 7 • 14 years ago

Should this be a security bug?

	Mats Palmgren (inactive) Assignee
	Comment 8 • 13 years ago

"startContent" (red) is the content node that loses its existing frame

	Mats Palmgren (inactive) Assignee
	Comment 9 • 13 years ago

Do the same checks in nsListBoxBodyFrame::GetFirstItemBox as in GetNextItemBox.

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 13 years ago

Comment on attachment 545639 [details] [diff] [review]
fix v1

r=me

Can you file a followup on adding a signature of IsXUL() that takes an nsIAtom like we have for HTML?

	Mats Palmgren (inactive) Assignee
	Comment 11 • 13 years ago

Filed bug 671847 for adding a signature of IsXUL().

http://hg.mozilla.org/integration/mozilla-inbound/rev/373edcdacf30

	Joe Drew (not getting mail)
	Comment 12 • 13 years ago

http://hg.mozilla.org/mozilla-central/rev/373edcdacf30