Closed Bug 360992 Opened 18 years ago Closed 13 years ago

Crash [@ nsCSSFrameConstructor::ConstructFrame] on start-up with Load Time Analyzer extension installed

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

RESOLVED WONTFIX

People

(Reporter: martijn.martijn, Unassigned)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

When I have the Load Time Analyzer extension installed on trunk, I crash.

Talkback ID: TB26105236Z
nsCSSFrameConstructor::ConstructFrame  [mozilla\layout\base\nscssframeconstructor.cpp, line 7774]
nsCSSFrameConstructor::ProcessChildren  [mozilla\layout\base\nscssframeconstructor.cpp, line 11611]
nsCSSFrameConstructor::ConstructXULFrame  [mozilla\layout\base\nscssframeconstructor.cpp, line 6483]
nsCSSFrameConstructor::ConstructFrameInternal  [mozilla\layout\base\nscssframeconstructor.cpp, line 7925]
nsCSSFrameConstructor::ConstructFrame  [mozilla\layout\base\nscssframeconstructor.cpp, line 7793]
nsCSSFrameConstructor::ProcessChildren  [mozilla\layout\base\nscssframeconstructor.cpp, line 11611]
nsCSSFrameConstructor::ConstructDocElementFrame  [mozilla\layout\base\nscssframeconstructor.cpp, line 4611]
nsCSSFrameConstructor::ContentInserted  [mozilla\layout\base\nscssframeconstructor.cpp, line 9174]


This regressed between 2006-11-11 and 2006-11-12:
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=2006-11-11+05&maxdate=2006-11-12+06&cvsroot=%2Fcvsroot
I guess this is a regression from bug 319654, somehow.

To reproduce:
- Install nightly tester tools (to be able to install Load Time Analyzer): ->
http://users.blueprintit.co.uk/~dave/web/firefox/nightly
- Install Load Time Analyzer: -> https://addons.mozilla.org/firefox/3371/
- Restart Firefox

Also note bug 360616, btw, that is also about a certain crash with the Load Time Analyzer extension.
This worksforme.

Whoever can reproduce this, what's the document in question?  What's the node that has a null child?
I don't seem to be able to reproduce either. Any specific steps I should follow? Any non-default settings?

The only special things about my setup that I can see is that didn't use the NTT (edited install.rdf directly instead), have extensiondev and leak monitor installed.
Sorry, I just found out that you also need to have the Google toolbar installed.
I have Google Toolbar version 2.1.20060807W
http://www.google.com/search?q=google+toolbar&ie=utf-8&oe=utf-8&rls=org.mozilla:en-US:unofficial&client=firefox-a
http://tools.google.com/firefox/toolbar/install.html won't even let me download the toolbar XPI, because it thinks I have firefox older than 1.5.
Ah, ispiked has pointed me to http://dl.google.com/firefox/google-toolbar-linux.xpi which does reproduce the crash for me.  Yay.  The basic problem is that we end up with

(gdb) p last
$17 = {mContent = {mRawPtr = 0x8889c40}, mIndex = 7, mNodes = {mRawPtr = 0x0}}
(gdb) p iter
$18 = {mContent = {mRawPtr = 0x8889c40}, mIndex = 6, mNodes = {mRawPtr = 0x0}}
(gdb) p aContent->GetChildCount()
$19 = 6
(gdb) p aContent
$20 = (class nsIContent *) 0x8889c40

in nsCSSFrameConstructor::ProcessChildren.  All the kids of aContent are XUL elements in this case.

The document URI is (surprise!) chrome://browser/content/browser.xul.

The aContent node in question is a <toolbox> which is a child of a <window>.  The <toolbox> has id="navigator-toolbox".

So this is interesting.... the incoming node has a GetChildCount of 7 when we first start the loop...
So we run extension script under frame construction, and the extension blows part of us away.  Details coming up.
OS: Windows XP → All
Hardware: PC → All
So the key is that in frame 0 we're calling RemoveChildAt on the same exact node on which we called nsCSSFrameConstructor::ProcessChildren in frame 72.  Then we unwind back to that code, and since ChildIterator does not handle DOM mutations (nor should it need to!) we crash.

In more detail, we are constructing an nsImageBoxFrame for a <xul:image> which is the kid of a node with id="gtbChevron" (presumably for the google toolbar).  We resolve style for that node, which triggers a load of the "list-style-image:chrome://global/skin/toolbar/chevron.gif" (see frame 41).  This load is a foreground load, so when we add it to the loadgroup we synchronously send notifications to the docloader (frame 35) and hence to progress listeners.

Now the Load Time Analyzer extension has a progress listener installed, so we run the code in file:///home/bzbarsky/moztest/.mozilla/firefox/eige2occ.test/extensions/%7B289b1c0c-379b-4165-81bb-72463915cb20%7D/lib/ffloadgraph.js  line 152.  This is kinda obfuscated, but at the very least it seems to call getElementById("ffloadgraph-toolbaritem").  Then when we're returning the node to JS we start wrapping its ancestors, and reach one which has an XBL binding, which we go ahead and attach (frames 20 through 14).

The node we're attaching the XBL binding to is a <toolbar id="ffloadgraph-toolbar">.  The binding is chrome://global/content/bindings/toolbar.xml#toolbar and the constructor for that binding is:

 97       <constructor>
....
106           if (!toolbox.palette) {
...
119             toolbox.palette = node;       
120             toolbox.removeChild(node);

which is the removeChild call in frame 0; the js stack at that point is:

(gdb) jsstack 
0 [native frame]
1 () ["chrome://global/content/bindings/toolbar.xml":120]
    toolbox = [object XULElement @ 0xb27caf00 (native @ 0xb5206ad8)]
    node = [object XULElement @ 0xb27e9a18 (native @ 0xb29a68a0)]
    currentSet = undefined
    this = [object XULElement @ 0xb27fe220 (native @ 0xb526c2b0)]
2 [native frame]

Then we unwind and crash.

Ideally, we would not be calling out into progress listeners synchronously here, either by starting the image loads async or by notifying progress listeners async or something....
Oh, and I would guess this is in fact somehow a regression from bug 319654, but I have _no_ idea how.
Flags: blocking1.9?
Another option is to just make nsContentUtils::LoadImage queue up the image if we're in an unsafe state (as flagged on the document, say) and for the frame constructor to process such pending loads once it's done with frame construction (and if we're inside an update batch at the end of the update batch).  That's probably the way to go here, if we can make LoadImage() callers deal with effectively a delayed return value.
Just to say that this also affect 1.8.1 (Firefox 2). Also, it's possible to trigger this bug without the google toolbar, but I haven't pinpointed which of my extensions is causing the issue.


Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070120 BonEcho/2.0.0.2pre ID:2007012003
Yeah, in the end, the real problem is that the load time analyzer is doing things that are unsafe (though it really had no way to know that).  Has someone filed a bug on them yet?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
I also crash at startup and I dont have "load time analyzer" installed!
(In reply to comment #13)
> I also crash at startup and I dont have "load time analyzer" installed!
> 
Then it's likely a different bug. In any case it should be investigated separately. Required information: talkback id/stack trace of the crash, any customizations on the profile (preferably something that can be reproduced starting with a new profile.)
TB30191020M, TB30190957Y, TB30190939W, TB30186402G

Seems to be related to the Web Developer Toolbar. If I disable that Firefox starts fine
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070312 Minefield/3.0a3pre ID:2007031204 [cairo]
Henrik, which part of "different bug" did you miss?  File a bug, with detailed steps to reproduce starting from a vanilla profile (including URIs to the exact XPIs you installed or detailed click sequences to allow someone else with no experience installing extensions to install identical ones).  Feel free to cc me on this bug (though I won't have a chance to look until a few weeks from now).
BTW this signature is now featured on http://talkback-public.mozilla.org/reports/firefox/

Is all of that you, Henrik, or do we have another regression with the same signature?
Oh, you filed bug 373756.
This is now worksforme, using current trunk build and all necessary extensions installed (google toolbar and load time analyzer).
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WORKSFORME
Yeah, but the underlying problem is still there.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Crash Signature: [@ nsCSSFrameConstructor::ConstructFrame]
I am resolving this as won't fix. It seems to be an issue with Google Toolbar which is EOL.
Status: REOPENED → RESOLVED
Closed: 17 years ago13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: