557398 - "ASSERTION: This is unsafe! Fix the caller!" with <xul:wizard>, XBL, iframe

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: This is unsafe! Fix the caller!: 'Error', file /Users/jruderman/mozilla-central/content/events/src/nsEventDispatcher.cpp, line 490

I'm initially treating this as sg:critical because bug 531176 was sg:critical.

Comment 1

[Jesse Ruderman]

Attachment: stack trace for the assertion

Comment 2

[Johnny Stenback (:jst)]

Smaug, can you have a look and decide whether this should remain sg:critical etc?

Comment 3

[Olli Pettay [:smaug]]

Oh, this looks very much like sg:critical

Comment 4

[Olli Pettay [:smaug]]

The bad thing is
#15 0x00952251 in nsXBLProtoImplAnonymousMethod::Execute (this=0x1e0ee000, aBoundElement=0x1e0df2b0) at /Users/jruderman/mozilla-central/content/xbl/src/nsXBLProtoImplMethod.cpp:329
#16 0x009446cd in nsXBLPrototypeBinding::BindingAttached (this=0x1e0ec170, aBoundElement=0x1e0df2b0) at /Users/jruderman/mozilla-central/content/xbl/src/nsXBLPrototypeBinding.cpp:488
#17 0x0093eb23 in nsXBLBinding::ExecuteAttachedHandler (this=0x1e0ec820) at /Users/jruderman/mozilla-central/content/xbl/src/nsXBLBinding.cpp:976
#18 0x00963f85 in nsBindingManager::ProcessAttachedQueue (this=0x1e09a1e0, aSkipSize=0) at /Users/jruderman/mozilla-central/content/xbl/src/nsBindingManager.cpp:1014
#19 0x0041ee4f in PresShell::InitialReflow (this=0x1e0e28d0, aWidth=600, aHeight=600) at /Users/jruderman/mozilla-central/layout/base/nsPresShell.cpp:2518
#20 0x003e1bb6 in DocumentViewerImpl::InitPresentationStuff (this=0x1e0999b0, aDoInitialReflow=1) at /Users/jruderman/mozilla-central/layout/base/nsDocumentViewer.cpp:750
#21 0x003e2dea in DocumentViewerImpl::Show (this=0x1e0999b0) at /Users/jruderman/mozilla-central/layout/base/nsDocumentViewer.cpp:1979
#22 0x00dfee6a in nsDocShell::SetVisibility (this=0x1dfdc520, aVisibility=1) at /Users/jruderman/mozilla-central/docshell/base/nsDocShell.cpp:4605
#23 0x0070dbb5 in nsFrameLoader::Show (this=0x1dfdbe90, marginWidth=-1, marginHeight=-1, scrollbarPrefX=1, scrollbarPrefY=1, frame=0x4be9720) at /Users/jruderman/mozilla-central/content/base/src/nsFrameLoader.cpp:562
#24 0x0047a549 in nsSubDocumentFrame::ShowViewer (this=0x4be96f0) at /Users/jruderman/mozilla-central/layout/generic/nsFrameFrame.cpp:323
#25 0x0047a7e3 in nsSubDocumentFrame::Init (this=0x4be96f0, aContent=0x1dfdb470, aParent=0x4be9660, aPrevInFlow=0x0) at /Users/jruderman/mozilla-central/layout/generic/nsFrameFrame.cpp:288
#26 0x00398535 in nsCSSFrameConstructor::InitAndRestoreFrame (this=0x1e0cb680, aState=@0xbfffc864, aContent=0x1dfdb470, aParentFrame=0x4be9660, aPrevInFlow=0x0, aNewFrame=0x4be96f0, aAllowCounters=1) at /Users/jruderman/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:4513
#27 0x003a2685 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x1e0cb680, aItem=@0x1e0e1920, aState=@0xbfffc864, aParentFrame=0x4be9660, aFrameItems=@0xbfffc5dc) at /Users/jruderman/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:3740

We are doing the InitialReflow while constructing frames.

Should we postpone nsFrameLoader::Show to happen when it is safe to run scripts?
Or postpone something else? I'll investigate this during this weekend.
I'm hoping to have this fixed by the end of next week.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Postponing ShowViewer sounds pretty good to me....

Comment 6

[Olli Pettay [:smaug]]

I would assume that fixing this takes a day (I just may not all the time before
next week).
Writing the code should take just minutes, but testing hours.

Comment 7

[Olli Pettay [:smaug]]

My earlier comments were based on the stack trace.

Now that I'm trying to get the assertion, I'm
having problems to reproduce this - at least on linux.
I'll retry on OSX.

Comment 8

[Olli Pettay [:smaug]]

Ok, I can reproduce on OSX.

Comment 9

[Olli Pettay [:smaug]]

Attachment: a patch

I uploaded this to try server.

Comment 10

[Olli Pettay [:smaug]]

Comment on attachment 438369 [details] [diff] [review]
a patch

Based on tryserver and some (i)frame heavy sites this should work.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 438369 [details] [diff] [review]
a patch

Ugh, nsWeakFrame!

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 438369 [details] [diff] [review]
a patch

r=jst

Comment 13

[Robert Sayre]

Please check this in ASAP

Comment 14

[Olli Pettay [:smaug]]

http://hg.mozilla.org/mozilla-central/rev/ac0109fc6043

Waiting a bit before asking approval for branches.

Comment 15

[Daniel Veditz [:dveditz]]

Branch patches will need to fix the regression bug 561981

Comment 16

[Reed Loden [:reed]]

(In reply to comment #14)
> Waiting a bit before asking approval for branches.

Both 1.9.2 and 1.9.1?

Comment 17

[Timothy Nikkel (:tnikkel)]

The fix for the regression this bug caused (bug 561981) landed on mozilla-central.