Closed Bug 339249 Opened 18 years ago Closed 18 years ago

[FIX]Crash with iExploder test 281619 [@ nsSubDocumentFrame::Reflow]

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9alpha1

People

(Reporter: j.moz, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(5 files)

Reduced test case (iExploder test 281619)
45 bytes, text/html
Details
Another example
23 bytes, text/html
Details
Frame dump (from Reflow), with call stacks of frame creation
19.41 KB, text/html
Details
Non-crashing testcase that demonstrates content doubling
34 bytes, text/html
Details
Proposed patch
6.18 KB, patch
mrbkap
: review+
sicking
: superreview+
Details | Diff | Splinter Review 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060524 Minefield/3.0a1

If you visit the url in the URL field (iExploder test 281619), the browser
crashes.

Found using http://toadstool.se/software/iexploder/

TB19114840H
The source is:
<table>
<a>
<link>
<iframe src=javascript:
TB19114869G, TB19115744Q for attachment 223354 [details]
Keywords: testcase
Confirmed with Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060521 Minefield/3.0a1.

The Mac OS X crash reporter says the top line of the stack is nsSubDocumentFrame::Reflow.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Component: General → Layout
Ever confirmed: true
OS: Windows XP → All
Product: Firefox → Core
QA Contact: general → layout
Hardware: PC → All
Summary: Crash with iExploder test 281619 → Crash with iExploder test 281619 [@ nsSubDocumentFrame::Reflow]
Attached file Another example — 
<iframe src=javascript:

This doesn't crash, but opens the JavaScript Console with this error message:

Security Error: Content at moz-nullprincipal:{ae009a68-a130-4952-baa7-39486984bf14} may not load or link to http://fxfeeds.mozilla.com/rss20.xml.
The fact that <iframe src="javascript:"> opens the JavaScript console is a bug, but the error message involving the feed is bug 338657.
That might be bug 243170, "Page can open JavaScript console even when are popup disabled".
This also crashes if I replace "javascript:" with "javascript:void 0" or "javascript:5".  So the crash here does not depend on "javascript:" opening the JavaScript console.

The crash does go away if I use "data:text/plain,5" or "http://www.google.com/" for the iframe src.
Sorry, I should have added the top of the stack in the summary. This is probably related to bug 336718.
Depends on: 336718
FWIW, the proposed patch for bug 336718 (dated 2006-05-05) did not help here...
The problem is that we insert two frames for the same <iframe> content,
the second call have a 'presShell' on line 706 so we return without
doing CreateViewAndWidget on line 746, which is the method that sets up
'mInnerView' so it's null when we Reflow this frame...
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsFrameFrame.cpp&rev=3.304&root=/cvsroot&mark=706-709,746#696

The source of the two calls to nsSubDocumentFrame::Init is in
SinkContext::CloseContainer, the first through mSink->NotifyAppend()
and the second through DidAddContent():
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/document/src/nsHTMLContentSink.cpp&rev=3.755&root=/cvsroot&mark=1369,1380#1321

See the call stacks in the attached frame dump.

It isn't too hard to avoid crashing in nsFrameFrame.cpp but it
leads to two frames being displayed which is probably not right.

It seems this is a parser/sink problem?

This is a regression from bug 332644.  Looking into it now.
Blocks: 332644
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed patch
        — 
          — Splinter Review
      

    


  
So what happens in those testcase is that when we handle the <link> we open a head context for it, which makes us flush tags on the <body> context.  As a result, we notify on the <a>.  Then when we close the <a> we notify on its kids (which makes sense) and call DidAddContent on the <a>.  But <body> has an insertion point, and DidAddContent doesn't check what's already been flushed and just goes ahead and notifies on the <a> a second time, which notifies on all its kids a second time.

So the part of this patch that fixes this bug is adding the 

>+      mStack[mStackPos - 1].mNumFlushed <
>+      mStack[mStackPos - 1].mContent->GetChildCount()) {

part in the patch.  The change to remove the unused second arg to DidAddContent is just window dressing.  The change to SinkContext::CloseContainer just makes sure we maintain our mNumFlushed for the container we're closing, though it probably doesn't matter since we'll never notify under it again...

        Attachment #224153 -
        Flags: superreview?(bugmail)

        Attachment #224153 -
        Flags: review?(mrbkap)

    
  
Priority: -- → P1
Summary: Crash with iExploder test 281619 [@ nsSubDocumentFrame::Reflow] → [FIX]Crash with iExploder test 281619 [@ nsSubDocumentFrame::Reflow]
Target Milestone: --- → mozilla1.9alpha

    
    

    
  
Comment on attachment 224153 [details] [diff] [review]
Proposed patch

>Index: content/html/document/src/nsHTMLContentSink.cpp
>+  if (0 < mStackPos &&
>+      mStack[mStackPos - 1].mInsertionPoint != -1 &&
>+      mStack[mStackPos - 1].mNumFlushed <
>+      mStack[mStackPos - 1].mContent->GetChildCount()) {

Want to indent the last line to be explictly clear that it's the continuation of the previous line?

        Attachment #224153 -
        Flags: review?(mrbkap) → review+

    
  
Keywords: regression

        Attachment #224153 -
        Flags: superreview?(bugmail) → superreview+

    
    

    
  
Fixed.

    
  
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

    
    

    
  
Verified FIXED; all testcases no longer crash for me, and the testcase https://bugzilla.mozilla.org/attachment.cgi?id=224152 which uses to demonstrate the content doubling is now working fine for me.

Build 2006-06-19-10 of SeaMonkey trunk under Windows XP.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsSubDocumentFrame::Reflow]

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: