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

RESOLVED FIXED in mozilla1.8alpha4

Status

()

Core
Layout
P1
critical
RESOLVED FIXED
14 years ago
7 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: bz)

Tracking

({crash})

Trunk
mozilla1.8alpha4
crash
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
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
(Reporter)

Comment 1

14 years ago
Created attachment 157744 [details]
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.
(Reporter)

Comment 2

14 years ago
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

14 years ago
Aviary dies too, but not a crash, an OOM kill.
Summary: Crash with trunk builds on http://lojjic.net/blog/index.rdf.html → Crash with http://lojjic.net/blog/index.rdf.html
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.
Depends on: 256242
(Reporter)

Comment 5

14 years ago
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

14 years ago
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)

Updated

14 years ago
Keywords: crash
Summary: Crash with http://lojjic.net/blog/index.rdf.html → Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ]
Created attachment 158401 [details]
More minimal testcase
Attachment #157744 - Attachment is obsolete: true
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?
Component: Browser-General → Layout
Depends on: 228557
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.
Created attachment 158601 [details] [diff] [review]
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).
Assignee: general → bzbarsky
Status: NEW → ASSIGNED
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Summary: Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ] → [FIX]Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ]
Target Milestone: --- → mozilla1.8alpha4
Comment on attachment 158601 [details] [diff] [review]
Proposed patch

David, what do you think?
Attachment #158601 - Flags: superreview?(dbaron)
Attachment #158601 - Flags: review?(dbaron)
Attachment #158601 - Flags: superreview?(dbaron)
Attachment #158601 - Flags: superreview+
Attachment #158601 - Flags: review?(dbaron)
Attachment #158601 - Flags: review+
Summary: [FIX]Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ] → [FIXr]Crash with http://lojjic.net/blog/index.rdf.html [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ]
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
*** Bug 258112 has been marked as a duplicate of this bug. ***
*** Bug 257825 has been marked as a duplicate of this bug. ***

Comment 15

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

Comment 16

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

Comment 17

9 years ago
I checked in bz's version of the testcase as a crashtest.
Flags: in-testsuite+
Crash Signature: [@ 0x00000000 - nsCSSFrameConstructor::AppendFrames ]
You need to log in before you can comment on or make changes to this bug.