Closed Bug 397856 Opened 17 years ago Closed 17 years ago

[FIX]"ASSERTION: How did we start layout without notifying on root" with contentEditable, image link, iframe

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9beta1

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(Keywords: assertion, memory-leak, testcase)

Attachments

(4 files)

testcase
1.85 KB, text/html
Details
stack to PresShell::InitialReflow
12.13 KB, text/plain
Details
stacks to the first five assertions
33.78 KB, text/plain
Details
Possible fix
4.89 KB, patch
peterv
: review+
sicking
: superreview+
Details | Diff | Splinter Review
Attached file testcase — 
Steps to reproduce:
1. Load the testcase.
2. Click the home button.  (My home page is http://www.squarefree.com/start/.)

Result:

###!!! ASSERTION: How did we start layout without notifying on root?: '!mLayoutStarted', file /Users/jruderman/trunk/mozilla/content/html/document/src/nsHTMLContentSink.cpp, line 2350

###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 8841

###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file /Users/jruderman/trunk/mozilla/layout/base/nsFrameManager.cpp, line 571

###!!! ASSERTION: Unexpected child of document element containing block: 'mDocElementContainingBlock->GetFirstChild(nsnull) == nsnull', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 8869

###!!! ASSERTION: already have a child frame: 'mFrames.IsEmpty()', file /Users/jruderman/trunk/mozilla/layout/generic/nsHTMLFrame.cpp, line 263
--DOMWINDOW == 9


3. Close the browser window.

Result:

WARNING: Weak frames alive after destroying FrameManager: '!mWeakFrames', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 1661

###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file /Users/jruderman/trunk/mozilla/layout/base/nsPresShell.cpp, line 673


4. Quit.

Result: 

A bunch of stuff has leaked.
Flags: blocking1.9?
This worksforme, Mac and Linux....
I can still reproduce on Mac.  I can even reproduce in a pretty clean profile, where the home page is http://www.mozilla.org/projects/minefield/.
OK.  Can you do the following?

1)  Start under gdb
2)  Load the testcase.
3)  Breakpoint in PresShell::InitialReflow
4)  Hit the home button

I'd like to see the stacks to all the InitialReflow calls that happen at this point.

    
    

    
  
This bug can lead to the home page being shown blank or mostly-blank.

    
    

    
  


  

    
    

    
  
OK.  So at a guess, we're flushing from that paint notification, which flushes the sink before it's heard anything from the parser.  I think FlushTags ends up being a no-op in this case, and on particular, we do not notify on mRoot.  Then we StartLayout.  When we go through and get the call for the HTML tag open from the parser, we notify on the root, etc.  That's bad.

We should fix FlushTags to notify on the <html> as needed...

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Possible fix
        — 
          — Splinter Review
      

    


  
Jesse, can you test this?

    
    

    
  
This patch fixes all the problems for me.

    
    

    
  
Comment on attachment 283140 [details] [diff] [review]
Possible fix

OK, I think we should just do this, then.

        Attachment #283140 -
        Flags: superreview?(jonas)

        Attachment #283140 -
        Flags: review?(peterv)

    
    

    
  
Not sure how to test offhand...
Assignee: nobody → bzbarsky
Flags: in-testsuite?
Priority: -- → P2
Summary: "ASSERTION: How did we start layout without notifying on root" with contentEditable, image link, iframe → [FIX]"ASSERTION: How did we start layout without notifying on root" with contentEditable, image link, iframe
Target Milestone: --- → mozilla1.9 M9

    
    

    
  
We really need to run tests while looking for assertions. See bug 397725.

    
    

    
  
Jonas, I think the problem with testing this bug is more specific than that.  My steps-to-reproduce are more complicated than just "load the testcase", and bz can't reproduce at all.

        Attachment #283140 -
        Flags: review?(peterv) → review+
Flags: blocking1.9? → blocking1.9+

        Attachment #283140 -
        Flags: superreview?(jonas) → superreview+

    
    

    
  
Fix checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED

    
    

    
  
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050621 Firefox/3.0pre and the testcase. No Assertion and no leak :)

--> Verified fixed
Status: RESOLVED → VERIFIED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: