405732 - Crash [@ nsHTMLContainerFrame::CreateViewForFrame(nsIFrame*, nsIFrame*, int) ]

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Brian Polidoro]

Attachment: breakpad crash stack

I see plenty of Breakpad reports with this URL.  And visiting it crashes for me.  

I see bug 354908 and bug 346014 but those look like they don't occur on trunk and are blocking1.9-. 

bp-5667a3ae-9c8b-11dc-acbf-001a4bd43e5c

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This looks like a layout bug. We're crashing while doing the initial reflow if the document.

Comment 2

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

We're crashing because there is no root view. That suggests some kind of error in presentation setup. Still looking.

Comment 3

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

I get loads of "not thread-safe" errors but they're probably related to Java and perhaps not this bug.

Comment 4

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

The problem is timing-related because if I set a breakpoint in nsViewManager::SetRootView that just dumps some stack and continues, we don't crash.

Comment 5

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

Attachment: fix

The problem is that we're calling DocumentViewer::SetDOMDocument after we've already constructed frames. I don't know if that represents a deeper bug, but anyway SetDOMDocument is trying to recover by destroying the current mPresShell. The recovery doesn't work because destroying the presshell tears down the frame tree, and destroying the root frame destroys the root view, so we end up with a prescontext, a viewmanager, but no root view. So when we build a new preshell and construct frames again, the new viewport frame (which would normally be assigned the root view) doesn't get a view, and badness ensues.

The solution in this patch is just to run MakeWindow during recovery. That creates a new viewmanager, root view and root widget, replacing the existing ones. As far as I can tell that is quite safe at this point --- the presshell has been destroyed so there shouldn't be a reference to the old viewmanager hanging around.

I refactored MakeWindow a little bit to make it easier to call from SetDOMDocument.

Comment 6

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

Note that there is no need to do mWindow->Destroy() here ... it has already been destroyed when the root view was destroyed when the root frame was destroyed.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

whow!! It's a know problem that calling SetDOMDocument after initial reflow causes crashes. I didn't realize that's what was going on here.

Are you really fixing that? That would rock bigtime! (It would allow for all sorts of coolness, such as having DOM APIs to set the document for a given window)

We've been trying real hard to avoid that from ever happening by trying to prevent initial reflow from ever happening for XSLT documents. I would be very interested to know why it's happening in this case.

Brian, any chance you could try to reduce bseindia.com into a smaller testcase that also crashes for you?

Comment 8

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

I don't claim to be fixing the general problem, but I'm doing enough to fix the crash on this page. The page seems to work fine after loading.

Comment 9

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

(but I actually don't know which part is XSLT-generated)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 290853 [details] [diff] [review]
fix

>+  // Reset the bounds offset so the root view is set to 0,0. The
>+  // offset is specified in nsIViewManager::Init above.

That second sentence doesn't make that much sense...  r+sr=bzbarsky with it beaten into shape.  Thanks for making this code more robust!

I do think it's worth understanding why we're starting layout before the transform runs here, in a followup bug.

Comment 11

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

Checked in. Filed bug 406301 on the XSLT issue.

Comment 12

[Marcia Knous [:marcia]]

verified fixed using  Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050706 Minefield/3.0pre. No crash on Mac nightly either using the URL.