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

RESOLVED WONTFIX

Status

()

--
critical
RESOLVED WONTFIX
12 years ago
7 years ago

People

(Reporter: martijn.martijn, Unassigned)

Tracking

({crash, regression})

Trunk
crash, regression
Points:
---
Bug Flags:
wanted-next +
blocking1.9 -
wanted1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

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

Comment 1

12 years ago
Created attachment 245796 [details]
backtrace from debug build
This worksforme.

Whoever can reproduce this, what's the document in question?  What's the node that has a null child?

Comment 3

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

Comment 4

12 years ago
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
Created attachment 245880 [details]
Stack showing the basic problem

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.

Comment 11

12 years ago
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]

Comment 13

12 years ago
I also crash at startup and I dont have "load time analyzer" installed!

Comment 14

12 years ago
(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.)

Comment 15

12 years ago
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).

Comment 17

12 years ago
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?

Comment 18

12 years ago
Oh, you filed bug 373756.
(Reporter)

Comment 19

11 years ago
This is now worksforme, using current trunk build and all necessary extensions installed (google toolbar and load time analyzer).
Status: NEW → RESOLVED
Last Resolved: 11 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+
(Assignee)

Updated

7 years ago
Crash Signature: [@ nsCSSFrameConstructor::ConstructFrame]

Comment 21

7 years ago
I am resolving this as won't fix. It seems to be an issue with Google Toolbar which is EOL.
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.