392335 - Crash [@ nsFrame::Destroy] when zooming text [@ DocumentViewerImpl::SetTextZoom]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

Crash [@ nsFrame::Destroy] when zooming text.

STEPS TO REPRODUCE
1. load URL (or any of the other URLs listed in the crash reports below)
2. press CTRL and use the mouse wheel to zoom the text up/down
3. every now and then type CTRL+R to cause a Reload (10 sec interval perhaps)
   continue zooming while the page loads

ACTUAL RESULT
I was able to reproduce it in Firefox trunk on Linux twice within
10 minutes of trying.

Crash reports with "SetTextZoom" on the stack in the past 3 months:
http://crash-stats.mozilla.com/?do_query=1&query_search=stack&query_type=contains&query=SetTextZoom&date=&range_value=3&range_unit=months

A typical stack looks like:

nsFrame::Destroy()
nsContainerFrame::Destroy()
nsFrameList::DestroyFrames()
nsContainerFrame::Destroy()
nsBlockFrame::DoRemoveFrame(nsIFrame*, int, int)
nsBlockFrame::RemoveFrame(nsIAtom*, nsIFrame*)
nsFrameManager::RemoveFrame(nsIFrame*, nsIAtom*, nsIFrame*)
nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, int, int)
nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*)
nsCSSFrameConstructor::ProcessRestyledFrames(nsStyleChangeList&)
nsPresContext::ClearStyleDataAndReflow()
DocumentViewerImpl::SetTextZoom(float)

Comment 1

[Nickolay_Ponomarev]

I can reproduce by changing the font size (via CTRL+'+') on http://www.sqlite.org/changes.html

works in firefox-2007-07-25-05-trunk
fails in firefox-2007-07-25-17-trunk

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-07-25+03%3A00%3A00&maxdate=2007-07-25+19%3A00%3A00&cvsroot=%2Fcvsroot

My guess is bug 386640 (it touched the code responsible for changing font size).

Comment 2

[Eli Friedman]

This is likely to be the same issue as Bug 389744... I'll look into it once that patch gets checked in.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: gdb session of crash I just hit

Here's a gdb session of a crash I just hit.

I think what's going on here is the following:
 1) we have unflushed style reresolves that need to happen (this can happen, right?)
 2) we do ClearStyleDataAndReflow
 3) the processing of the style change enqueues some frame reconstructs (due to the posted restyles that haven't been done yet) -- and when it does this, it doesn't change the style contexts on the frames due to be reconstructed or their descendants
 4) ClearStyleDataAndReflow destroys the old rule tree
 5) ClearStyleDataAndReflow processes the change list, which tries to access the pres context through a rule node that's been destroyed.

I think the simple fix is to swap the order of EndReconstruct and ProcessRestyledFrames in ClearStyleDataAndReflow.

Does that sequence of events sound realistic?  And does the fix seem reasonable?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Hrm, I suppose I should have looked at my review queue first!

Comment 5

[Martijn Wargers (dead)]

Is this still an issue?

Comment 6

[Nickolay_Ponomarev]

(In reply to comment #1)
> I can reproduce by changing the font size (via CTRL+'+') on
> http://www.sqlite.org/changes.html
> 
FWIW, I can't reproduce this with the latest nightly (2007100205). It's a different machine than the one I was testing on earlier though, so another confirmation would be great.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed by bug 389744.