257818 - [FIXr]Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ]

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040902 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040902 Firefox/0.9.1+

When visiting that site, the browser crashes/get into an infinite loop.
It is a regression. This does not happen in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040811
Firefox/0.9.1+
It happens in:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a3) Gecko/20040812
Firefox/0.9.1+
A Talkback ID I got is: TB717956X

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Actual Results:  
Crash/infinite loop/freeze

Expected Results:  
Just load the page

Comment 1

[Martijn Wargers (dead)]

Attachment: Simpler testcase

I've tried to make a simple crash testcase, but the script that causes the
crash is complicated. I guess this could be reduced further, but it is kinda
hard.

Comment 2

[Martijn Wargers (dead)]

Boris, could you triage this?
This the Talkback link:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB717956X
Although I have no idea what is causing this, I think it is another regression
from bug 255153.
Maybe this should be on the dependency list for bug 257694?

Comment 3

[Jeremy M. Dolan]

Aviary dies too, but not a crash, an OOM kill.

Comment 4

[Boris Zbarsky [:bzbarsky]]

The stack is not so helpful; I'll add it to the dep list so I can retest once
that's fixed.  Chances are, it's something else, though (and a minimal testcase
would be a good start in triaging this).

Note that bug 256242, bug 255153, etc are not issues on aviary.  So perhaps you
should look for checkins in your range that landed on aviary too.

Comment 5

[Martijn Wargers (dead)]

Jeremy, what build were you using? Could you test it again?
I just tried it with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040902
Firefox/1.0 PR (NOT
But it is not crashing/freezing for me, neither on the website nor the testcase.

Another talkback link:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB718422Z
It looks basically the same as the last one, so probably not useful.

Comment 6

[Jeremy M. Dolan]

Crap. I accidentally downloaded trunk instead of aviary today. My previous
comment should read "today's trunk", rather than aviary.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040831 Firefox/0.9.1+
(not sure why the gecko stamp is 3 days old, but this build /was/ uploaded today)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: More minimal testcase

Comment 8

[Boris Zbarsky [:bzbarsky]]

Here's my analysis of what's happening here...

The page is creating a <div>, putting it in the document, setting its style to
display:none, then sticking some stuff in it (including a text control).

Now the text control inits its editor and all that, which somehow ends up
flushing pending restyles (per the code I added in bug 255153, I assume), thus
wiping out the <div>'s frame.  The text control frame is not in the <div>'s
child list yet, so doesn't die.  Thus we don't crash immediately.  However, we
end up walking the parent chain from our call to SelectAll() in
nsTextControlFrame::SetValue(), which ends up reaching the destroyed blockframe
and crashes.

I suspect the patch to bug 228557 will fix this particular crash (because the
restyle event is posted before the editor event, so will happen earlier).  But
there may be other ways to trigger this situation; I'll need to think about it...

The real root of the problem here is that we can end up processing restyle
events that cause frame destruction while we are in frame code or accessing
frames (what bug 255153 was trying to fix).   I can probably fix bug 255153
"better" by making editor not flush our restyles and removing the flush I added,
but there may be other situations where we grab a frame and then some other code
flushes restyles.  One possible solution is to clearly document which operations
with frames may and may not cause flushes and to:

1) change content code to reget the primary frame any time it may have caused a
   flush.
2) change frame code to never call methods that may cause a flush.

Another possible solution is to back out the lazy style reresolution patch....

Thoughts?

Comment 9

[Boris Zbarsky [:bzbarsky]]

The more I think about it, the more I think the right thing to do is to make
editor not flush restyles and to remove the code I added for bug 255153.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

I think this is the right way to go.  The nsHTMLGenericElement change fixes
this crash and a few others; the presshell change refixes bug 255153 (and
probably other bugs where editor init was ending up flushing out restyles).

Comment 11

[Boris Zbarsky [:bzbarsky]]

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

David, what do you think?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fix checked in.

Comment 13

[Boris Zbarsky [:bzbarsky]]

*** Bug 258112 has been marked as a duplicate of this bug. ***

Comment 14

[Boris Zbarsky [:bzbarsky]]

*** Bug 257825 has been marked as a duplicate of this bug. ***

Comment 15

[timeless]

*** Bug 285023 has been marked as a duplicate of this bug. ***

Comment 16

[timeless]

*** Bug 285963 has been marked as a duplicate of this bug. ***

Comment 17

[Jesse Ruderman]

I checked in bz's version of the testcase as a crashtest.