"ASSERTION: Losing track of existing primary frame" on layout/xul/base/src/crashtests/432058-1.xul

RESOLVED FIXED in mozilla8

Status

()

Core
Layout
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: mats)

Tracking

({assertion, testcase})

Trunk
mozilla8
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 -, status2.0 wanted)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 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
Flags: in-testsuite+
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?
And this is bad; it can leak frames.
blocking2.0: --- → ?
I can take this, but not for a few weeks.
(Reporter)

Comment 4

7 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.
> 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....
(Reporter)

Comment 6

7 years ago
It doesn't for me.
Should this be a security bug?
blocking2.0: ? → -
status2.0: --- → wanted
(Assignee)

Updated

6 years ago
Assignee: nobody → matspal
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 8

6 years ago
Created attachment 545638 [details]
Content tree in nsListBoxBodyFrame::GetFirstItemBox

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

Comment 9

6 years ago
Created attachment 545639 [details] [diff] [review]
fix v1

Do the same checks in nsListBoxBodyFrame::GetFirstItemBox as in GetNextItemBox.
Attachment #545639 - Flags: review?(bzbarsky)
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?
Attachment #545639 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 11

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

http://hg.mozilla.org/integration/mozilla-inbound/rev/373edcdacf30
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/373edcdacf30
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.