Last Comment Bug 460803 - (CVE-2008-5500) [FIX]PresShell::InitialReflow "ASSERTION: Why are we being called?" with XUL iframe
(CVE-2008-5500)
: [FIX]PresShell::InitialReflow "ASSERTION: Why are we being called?" with XUL ...
Status: RESOLVED FIXED
[sg:critical?]
: assertion, testcase, verified1.8.1.19, verified1.9.0.5
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
Depends on:
Blocks: 344486 framedest
  Show dependency treegraph
 
Reported: 2008-10-20 11:30 PDT by Jesse Ruderman
Modified: 2008-12-16 16:51 PST (History)
9 users (show)
samuel.sidler+old: blocking1.9.0.5+
samuel.sidler+old: blocking1.8.1.19+
asac: blocking1.8.0.next+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (has external dependencies) (528 bytes, application/xhtml+xml)
2008-10-20 11:30 PDT, Jesse Ruderman
no flags Details
Do it like the sinks (1.14 KB, patch)
2008-10-20 15:14 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
jonas: review+
jonas: superreview+
dveditz: approval1.9.0.5+
Details | Diff | Review
1.8 branch patch (525 bytes, patch)
2008-10-21 13:21 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details | Diff | Review
1.8 branch non-ugly patch (1.19 KB, patch)
2008-10-21 13:22 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
dveditz: approval1.8.1.19+
asac: approval1.8.0.next+
Details | Diff | Review

Description Jesse Ruderman 2008-10-20 11:30:45 PDT
Created attachment 343940 [details]
testcase (has external dependencies)

About 50% of the time, loading the testcase triggers these six assertions:

1. ###!!! ASSERTION: Why are we being called?: '!mDidInitialReflow', file layout/base/nsPresShell.cpp, line 2373

2. ###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file layout/base/nsCSSFrameConstructor.cpp, line 8760

3. ###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file layout/base/nsFrameManager.cpp, line 574

4. ###!!! ASSERTION: unexpected mInitialContainingBlock: 'processChildren ? !mInitialContainingBlock : mInitialContainingBlock == contentFrame', file layout/base/nsCSSFrameConstructor.cpp, line 4308

5. ###!!! ASSERTION: unexpected second call to SetInitialChildList: 'Not Reached', file layout/generic/nsContainerFrame.cpp, line 111

6. ###!!! ASSERTION: Why is the root in mDirtyRoots already?: 'mDirtyRoots.IndexOf(rootFrame) == -1', file layout/base/nsPresShell.cpp, line 2462

and then closing it triggers:

7. ###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file layout/base/nsPresShell.cpp, line 676
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-20 14:58:25 PDT
Good stuff.  We get into nsMediaDocument::StartLayout when OnStartRequest fires.  We grab the presshell, start layout on it.  Then we do a synchronous paint, which flushes pending notifications.  That triggers a frame reconstruct in the parent document (from the pending restyle), which blows away the presshell we just started layout, creates a new one, and starts layout on it.  Then we return from the paint call, move on to the next presshell for the document (the newly-created one!) and proceed to try starting layout on it.  Then we lose.
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-20 15:14:08 PDT
Created attachment 343977 [details] [diff] [review]
Do it like the sinks
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-20 15:15:17 PDT
The key here is that the image needs to come in after the attr set happened but before the restyle is processed.  Fun.  :(  Not sure how to guarantee that in a test.
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-21 13:18:18 PDT
Pushed changeset f05dd60fa3ed.
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-21 13:19:39 PDT
Comment on attachment 343977 [details] [diff] [review]
Do it like the sinks

I do think that we should take this on branch, but I'd request apprval11.9.0.5 (not .4) if I could...
Comment 6 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-21 13:21:35 PDT
Created attachment 344162 [details] [diff] [review]
1.8 branch patch

Again, I'd probably try for the next branch release not this very next one.
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-10-21 13:22:38 PDT
Created attachment 344163 [details] [diff] [review]
1.8 branch non-ugly patch
Comment 8 Daniel Veditz [:dveditz] 2008-11-03 11:20:19 PST
Comment on attachment 343977 [details] [diff] [review]
Do it like the sinks

approved for 1.9.0.5 and 1.8.1.19, a=dveditz for release-drivers

(please watch tinderbox for the tree to open before landing)
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-11-06 11:45:59 PST
Fixed on both branches.
Comment 10 Carsten Book [:Tomcat] 2008-12-03 08:43:06 PST
verified fixed for 1.8.1.19 - using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.20pre) Gecko/2008120304 Firefox/2.0.0.20pre (1.8.1 Debug Build) and the Testcase from Jesse.

I do not see the Assertions from Comment #0, only ###!!! ASSERTION: Adding child where we already have a child?  This will likely misbehave: 'Error', file /work/mozilla/builds/1.8.1/mozilla/xpfe/components/shistory/src/nsSHEntry.cpp, line 580, which is a different Bug.

--> Verified fixed 1.8.1.19
Comment 11 Carsten Book [:Tomcat] 2008-12-03 11:04:43 PST
also verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2008120309 Firefox/3.0.6pre (Debug Build) and the testcase from Jesse. I do not see the Assertions from Comment #0 when using the Testcase

--> Verified 1.9.0.5
Comment 12 Alexander Sack 2008-12-16 01:04:31 PST
Comment on attachment 344163 [details] [diff] [review]
1.8 branch non-ugly patch

a=asac for 1.8.0

Note You need to log in before you can comment on or make changes to this bug.