M091, Trunk, & M92 crash [@ ChildIterator::ChildIterator]

VERIFIED FIXED in mozilla0.9.3

Status

()

Core
Layout
P1
critical
VERIFIED FIXED
17 years ago
4 years ago

People

(Reporter: greer, Assigned: Chris Waterson)

Tracking

({crash, topcrash})

Trunk
mozilla0.9.3
x86
Windows 98
crash, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PDT+, crash signature, URL)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
 
(Reporter)

Comment 1

17 years ago
Talkback reports are showing M091 crashing at ChildIterator::ChildIterator with
the following comments:

(31551999) Comments: I had just quit the mail part (Win 98)
     (31539149) Comments: I've clicked on a GrafigButton (Win 95)
     (31533659) URL: http://www.europe.hercules.com/main.php (win 98)
     (31533658) Comments: I clicked the link to Europe Hercules homepage on the
following page:http://www.hercules.com (Win 98)
     (31530934) Comments: I pressed the 'back' button several times rapidly (Win 98)
     (31505857) Comments: I'd clicked a hyperlink to go to a different page
     (31503566) Comments: Added Language German and Storing Preferences (Win NT)
     (31494487) URL: http://www.myfamily.com (Win 98)

Stack Trace: 

         ChildIterator::ChildIterator
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 830]
         nsCSSFrameConstructor::ProcessChildren
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 11434]
         nsCSSFrameConstructor::ConstructDocElementFrame
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 3540]
         nsCSSFrameConstructor::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 7447]
         StyleSetImpl::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp  line 1225]
         PresShell::ReconstructFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 4915]
         0x80000000
         nsPresContext::PreferenceChanged
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp  line 451]
         PrefChangedCallback
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp  line 80]
         pref_DoCallback       
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c 
line 1757]
         pref_HashPref 
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c  line
1362]
         PREF_SetIntPref       
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c 
line 570]
         nsPrefBranch::SetIntPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefBranch.cpp  line 244]
         nsPrefService::SetIntPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefService.h  line 40]
         nsPref::SetIntPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPref.cpp  line 246]
         XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 139]
         XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line
1837]
         XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1242]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 809]
         js_Interpret   [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2703]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 825]
         js_Interpret   [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2703]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 825]
         js_InternalInvoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c 
line 897]
         JS_CallFunctionValue   [d:\builds\seamonkey\mozilla\js\src\jsapi.c 
line 3322]
         nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp  line 937]
         nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp  line 140]
         nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1120]
         nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
2090]
         nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line
3631]
         PresShell::HandleDOMEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5562]
         nsButtonBoxFrame::MouseClicked
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsButtonBoxFrame.cpp  line 181]
         nsButtonBoxFrame::HandleEvent
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsButtonBoxFrame.cpp  line 128]
         PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5531]
         PresShell::HandleEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5486]
         nsEventStateManager::CheckForAndDispatchClick
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line
2464]
         nsEventStateManager::PostHandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line
1550]
         PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5535]
         PresShell::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5441]
         nsView::HandleEvent    [d:\builds\seamonkey\mozilla\view\src\nsView.cpp
 line
377]
         nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp  line 2051]
         HandleEvent    [d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 68]
         nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 716]
         nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 733]
         nsWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4197]
         ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4442]
         nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 3196]
         nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 980]
         KERNEL32.DLL + 0x363b (0xbff7363b)
         KERNEL32.DLL + 0x24407 (0xbff94407)
         0x00688ba6

Adding crash, topcrash for tracking and qawanted
Keywords: crash, qawanted, topcrash

Updated

17 years ago
Severity: normal → critical

Comment 2

17 years ago
Crash is hard to reproduce. Don't know the circumstances that lead to the 
occurance of such crashes. Works for me on the machines I use...
Also tried with foreign lang. packs...

The talkback reports contain different stacks, all of them have 
ChildIterator::ChildIterator at the top

Since the crash occurs allways in the constructor ChildIterator::ChildIterator, 
i suspect that the argument (nsIContent* aContent, ...) is corrupt (nsnull). The 
only thing that we can do is to check the pointer and keep the class safe for 
clients (very hard to keep safe since the class is declared as "struct").

If the pointer is not null and still corrupt... damn... 

The iterator is used to construct child frames of the given content, so if there 
is no content, there are no frames to construct and the iteration trough the 
child list is useles and has to be avoided.

Patch to follow.

Comment 3

17 years ago
Created attachment 39201 [details] [diff] [review]
the patch mentioned in the previous comment

Comment 4

17 years ago
I think that is the right idea, but I'd go one step further (actually two steps):

1) make a PRECONDITION on aContent in addition to your check for null to help us
find the problem caller.

2) check doc and mBindingManager for null too - in fact, the line 830 in the
talkback makes me think it might be the mBindingManger that is the problem here...

Please submit a new patch.

Comment 6

17 years ago
This is also a topcrasher on the current Trunk builds.  Here's the latest info
from Talkback on the Trunk crashes:

ChildIterator::ChildIterator   15
			 85422 	 NEW 	  	 karnaze@netscape.com --- 
     First BBID :31530429
     Last BBID  :31895994
     Min Runtime :181
     Max Runtime :42543
     First Appearance Date : 2001-06-10
     Last Appearance Date : 2001-06-18
     First BuildID : 2001060922
     Last BuildID : 2001061509

Stack Trace: 

	 ChildIterator::ChildIterator
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 830]
	 nsCSSFrameConstructor::ProcessChildren
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 11403]
	 nsCSSFrameConstructor::ConstructDocElementFrame
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 3483]
	 nsCSSFrameConstructor::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp 
line 7342]
	 StyleSetImpl::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp  line 1089]
	 PresShell::ReconstructFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 4922]
	 0x80000000
	 nsPresContext::PreferenceChanged
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp  line 451]
	 PrefChangedCallback
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp  line 80]
	 pref_DoCallback
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c  line 1773]
	 PREF_SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c  line 579]
	 nsPrefBranch::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefBranch.cpp  line 188]
	 nsPrefService::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefService.h  line 40]
	 nsPref::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPref.cpp  line 206]
	 PREF_SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c  line 579]
	 nsPrefBranch::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefBranch.cpp  line 188]
	 nsPrefService::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefService.h  line 40]
	 nsPref::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPref.cpp  line 206]
	 nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 3001]
	 nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 982]
	 KERNEL32.DLL + 0x363b (0xbff7363b)
	 KERNEL32.DLL + 0x242e7 (0xbff942e7)
	 0x00688ac4
 
 	Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp
line : 830
     (31850452)	URL:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=day&mindate=&maxdate=&cvsroot=%2Fcvsroot
(31850452)
Comments: -turbo mode
     (31813970)	Comments: Opening mail when didn't have focus.
     (31809860)	URL: www.nme.com
(31809860)
Comments: letting mozilla load the url in the background
     (31787285)	URL:
http://www.myfamily.com/isapi.dll?c=member&att=nPUjSA_wkNe8p9a0WpKj6L*CvPAl0u*CcEKCAE&htx=dailyinfo&siteid=akDDAE&memberid=000001
(31787285)
Comments: Build ID 2001061504after login to http://www.myfamily.comMozilla crashed
     (31670306)	URL: www.clevermedia.com/.....something....snowballfightgame....
     (31670306)	Comments: Trying to run snowballfight flash game
     (31662635)	Comments: not a damn thing with Mozilla.  The browser was in the put intothe
background on my Win2K box while I worked via exceedwindows.  Sat quietly for a
couple of hours then WHAM. He'sdead Jim.

Summary: M091 crash [@ ChildIterator::ChildIterator] → M091 & Trunk crash [@ ChildIterator::ChildIterator]

Comment 7

17 years ago
Patch 39237 looks good to me. [s]r=attinasi (please make sure you have run the
pre-checkin tests without the new assertions firing too).

CC'ing waterson for review...
(Assignee)

Comment 8

17 years ago
ContentIterator() is hyatt's code I think, I'd really like it if he could look
at the patch (especially the changes to the |NextChild()| method). It feels
wall-paperish: do we know why someone is trying to iterate over a null content node?

Comment 9

17 years ago
Could this be happening during that period when the old document has been torn
down but the frames are still there (before the new document has been built)?
There is some comment about using the back button repeatedly in the initial comment:

  (31530934) Comments: I pressed the 'back' button several times rapidly (Win 98)

Just a guess.

Comment 10

17 years ago
This looks weirder than that... note the preference changed callback on the stack.

Comment 11

17 years ago
Oh, that is weird! None of the comment mention changing a preference... But, it
looks like a Windows message MW_FONTCHANGE can cause a direct call into the
PrefService SetBoolPref method (windows/nsWindow.cpp line 2991).

Comment 12

17 years ago
BTW: the presContext is currently registering on all 'font.*' preference changes
- do we really need to do anything for this 'font.internaluseonly.changed' pref?
I cannot see why (but wtfdik).

Comment 13

17 years ago
I was unable to reproduce this on my machine using build release 2001-06-21-06.

Comment 14

17 years ago
me and Marc Attinasi tried to reproduce even using simulated modem-speed 
connection and didn't crashed. (we used the URLs and some other too)

there is the second patch(id=39237) that can make ChildIterator safer, but even 
if we apply that patch we're still not able to figure out what leads to this 
crash.

so what do we do with it? any ideea? has anyone reproduced the crash?
(Assignee)

Comment 15

17 years ago
Well, the changes look fine to me. But I'll let hyatt be the official sr=.

Comment 16

17 years ago
still showing up in the trunk topcrash reports.  
     First Appearance Date : 2001-06-13
     Last Appearance Date : 2001-06-21
     First BuildID : 2001061209
     Last BuildID : 2001062117

Here are the comments/urls from recent crashes:

     (31993524) Comments: I was trying to read a message with html and gif 
images included. The MS Office installer
fired and tried to install some unknown component. I canceled it (it may be 
related with the gif images in the
attachements). When i tried to delete this page it
     (31993524) Comments:  crashed with this error...
     (31949987) URL: 
http://www.mozillazine.org/talkback.html?article=1946&message=27#27
     (31942029) URL: http://ftp.au.xemacs.org/puib/xemacs/windows/
     (31942029) Comments: Trying to lanuch browser.
     (31930967) Comments: bug 86670
     (31928864) URL: bugzilla.mozilla.org
     (31928864) Comments: bug 86670
     (31926433) URL: bugzilla.mozilla.org
     (31926433) Comments: bug 86670
     (31925811) URL: bugzilla.mozilla.org
     (31925811) Comments: I've got to remember to write down the bug number 
before verifying crash bugs.  Now to
write down the talkback
     (31924017) URL: http://www.netscape.com
     (31924017) Comments: Clicked on throbber
     (31850452) URL: 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch
=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&
hours=2&date=day&mindate=&maxdate=&cvsroot=%2Fcvsroot
     (31850452) Comments: -turbo mode
     (31813970) Comments: Opening mail when didn't have focus.
     (31809860) URL: www.nme.com
     (31809860) Comments: letting mozilla load the url in the background
     (31787285) URL: 
http://www.myfamily.com/isapi.dll?c=member&att=nPUjSA_wkNe8p9a0WpKj6L*CvPAl0u*Cc
EKCAE&htx=dailyinfo&siteid=akDDAE&memberid=000001
     (31787285) Comments: Build ID 2001061504after login to 
http://www.myfamily.comMozilla crashed
     (31670306) URL: www.clevermedia.com/.....something....snowballfightgame....
     (31670306) Comments: Trying to run snowballfight flash game
     (31662635) Comments: not a damn thing with Mozilla.  The browser was in the 
put intothe background on my Win2K
box while I worked via exceedwindows.  Sat quietly for a couple of hours then 
WHAM. He'sdead Jim.

Comment 17

17 years ago
*** Bug 86670 has been marked as a duplicate of this bug. ***

Comment 18

17 years ago
Okay, so thanks to David Einstein & Ilya Konstantinov on bug 86670, we have 
a way to reproduce the crash with reasonable regularity. (It may not be 
the only way, but it is one way). 

Based on the other bug (with an extra bit so we it's easier to change the pref
with the modal preferences dialog on win32)

1. start browser and open a second navigator window
2. load www.mozilla.org in the second browser window
3. in the first browser window, open the preferences dialog and get the panel
   for Appearances->Color. Uncheck 'Use system colors' and check 'Use my chosen
   colors'
4. in the second browser window, click on the 'New Checkins' link on 
   mozilla.org
5. back to the prefs panel, and change the background color. Hit OK.
6. crash with this stack, in about half the times I tried this sequence

It's crashing because the GetDocument is returning a null |doc|
 
   nsCOMPtr<nsIDocument> doc;
   aContent->GetDocument(*getter_AddRefs(doc));
   doc->GetBindingManager(getter_AddRefs(mBindingManager)); // <-- crash.

Comment 19

17 years ago
I get the crash on Win2K using the recipe presented - thanks John!

Comment 20

17 years ago
From what I see, the problem is that the pref change is causing a full document 
reframe, and the root frame has no document. I think we can catch this much 
earlier than in the childiterator, like in 
nsCSSFrameConstructor::ReconstructDocElementHierarchy, but my question is, what 
should we do if there is no document for the root element, just skip the 
reframe?

Comment 21

17 years ago
uh, I meant to say the root content has no document, not the root frame - sorry. 
At any rate, I think that we can just check for the root content's document not 
being null where we check for the root content (line 7297 of 
cssframeconstructor.cpp - 
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSFrameConstruc
tor.cpp#7297)

Comment 22

17 years ago
I would patch the class ChildIterator with the proposed patch 39237 to prevent 
mozilla crashing, but would also continue to search why there's a nsIContent 
without a valid document (GetDocument delivers a null pointer). I tested the 
patch, it works (was able to reproduce the situation where there's no document), 
mozilla forces reframing and most important, it does not crash. So I would like 
to have this patch checked in ASAP be cause this is a topcrash, therefore I 
would need the SR= from Hyatt.

Hyat could you do this please?

Comment 23

17 years ago
I really don't want to slow down the iterator with a lot of additional checks
(since this code needs to be lightning fast).  Couldn't this be caught earlier?
 This seems like a band-aid that is avoiding the real issue (that the document
is null when it shouldn't be).

Comment 24

17 years ago
We can catch it at the root, in
nsCSSFrameConstructor::ReconstructDocElementHierarchy. If there is no Doc for
the root element, no sense trying to reframe it! I wonder where else we need to
check for this condition...

I think the ChildIterator ctor should at least have assertions, however, so we
can catch bad usage in development (not that this one would have triggered it, I
think, it is pretty obscure).

Comment 25

17 years ago
added myself (angelabu) to the cc: list

Comment 26

17 years ago
this is also a topcrash on M092

URLs and Comments from recent crashes:
 (32294156)	URL: www.broadcast.com
 (32294156)	Comments: i just went to the site
 (32294156)	Comments:  before it crashed.

(32308723)
Comments: pessing back on google.com
(32308144)
URL: http://bugzilla.mozilla.org/show_bug.cgi?id=69493
(32269416)
URL: www.indya.com
(32269416)
Comments: Only during some rare circumstances the browser terminates
(32261663)
URL: tvgiude.com
(32249908)
Comments: closing windows (disk) explorer while attempting to go to a bugzilla
bug from the bug list page.
(32247403)
URL: my.yahoo.com
(32182498)
Comments: selected history for the sidebar
(32091556)
URL: tvgiude.com
(32091556)
Comments: TV Giude online - local listings - refusing a cookie from a pop-up window
(32088906)
Comments: starting browser
(32088562)
URL: myfamily.com (passworded page)
(32088562)
Comments: crashed after starting to initially display the page.
(32036594)
URL: http://www.unison.ie/irish_independent/
(32036594)
Comments: Clicked on the 'Irish Independent' link in the left table
     (31993524)	Comments: I was trying to read a message with html and gif images included.
The MS Office installer fired and tried to install some unknown component. I
canceled it (it may be related with the gif images in the attachements). When i
tried to delete this page it
     (31993524)	Comments:  crashed with this error...
     (31949987)	URL: http://www.mozillazine.org/talkback.html?article=1946&message=27#27
(31930967)
Comments: bug 86670
     (31928864)	URL: bugzilla.mozilla.org
     (31928864)	Comments: bug 86670
     (31926433)	URL: bugzilla.mozilla.org
     (31926433)	Comments: bug 86670
     (31925811)	URL: bugzilla.mozilla.org
     (31925811)	Comments: I've got to remember to write down the bug number before verifying
crash bugs.  Now to write down the talkback
     (31924017)	URL: http://www.netscape.com
(31924017)
Comments: Clicked on throbber


stacktrace from build 2001062806:

Incident ID 32291593
ChildIterator::ChildIterator
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 830]
nsCSSFrameConstructor::ProcessChildren
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 11413]
nsCSSFrameConstructor::ConstructDocElementFrame
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 3483]
nsCSSFrameConstructor::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp,
line 7337]
StyleSetImpl::ReconstructDocElementHierarchy
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp, line 1091]
PresShell::ReconstructFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 4968]
PresShell::SetPreferenceStyleRules
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 1975]
nsPresContext::PreferenceChanged
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp, line 451]
PrefChangedCallback
[d:\builds\seamonkey\mozilla\layout\base\src\nsPresContext.cpp, line 80]
pref_DoCallback [d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c, line
1773]
pref_HashPref [d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c, line 1378]
PREF_SetBoolPref [d:\builds\seamonkey\mozilla\modules\libpref\src\prefapi.c,
line 579]
nsPrefBranch::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefBranch.cpp, line 189]
nsPrefService::SetBoolPref
[d:\builds\seamonkey\mozilla\modules\libpref\src\nsPrefService.h, line 42]
nsPref::SetBoolPref [d:\builds\seamonkey\mozilla\modules\libpref\src\nsPref.cpp,
line 208]
nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3012]
nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 989]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x24407 (0xbff94407)

Summary: M091 & Trunk crash [@ ChildIterator::ChildIterator] → M091, Trunk, & M92 crash [@ ChildIterator::ChildIterator]

Comment 27

17 years ago
always reproducible on www.broadcast.com . The site automatically opens a second
window and then crashes.

Comment 28

17 years ago
This bug now blocks me in bug 85334. When playing any game that requires
Shockwave 3dGroove like TankWars:

http://www.shockwave.com/content/tankwars/tankwarslanding.html
We are getting a WM_FONTCHANGE message, probably caused by the plugin. This 
eventually causes a reframe and a crash.

After speaking with Marc, I'm attaching a patch to at least unblock me. Can I
get some reviews and comments? Bug 85334 is PDT+.
Blocks: 85334

Comment 29

17 years ago
Created attachment 40924 [details] [diff] [review]
try #1 at a patch to prevent reframe for font.internaluseonly.change

Comment 30

17 years ago
The Plugin problem is caused by a WM_FONTCHANGE message being generated from
soemewhere, and that causes a SetBoolPref on the font.internaluseonly.changed
pref, which causes a reframe. Peter's change just insulates us from the pref
change on that value since it really only needs to cause the font cache to be
updated (according to yokoyama's comment when that code was added to nsWindow).

We need to document the filter and try to explain that we don't care about
changes on that pref value, and possibly indicate the Plugin bug that it fixes
too (bug 85334). Also, do we plan to continue trying to figure out why a) the
WM_FONTCHANGE message is coming in, and b) how we can handle it more gracefully
in the Object Frame?

I'm cool with this patch, since we do not need to handle that font-pref change 
in the PresContext anyway. [s]r=attinasi for patchID 40924

(Assignee)

Comment 31

17 years ago
Okay, I think that I've just caught this one in the debugger. I'll attach a 
stack trace. The weird thing is that we're processing a pref change where the 
pref is

  font.internaluseonly.changed

Apparently in response to a WM_FONTCHANGE message? LXR doesn't show that we 
_post_ any WM_FONTCHANGE messages, so presumably some other app on my system is 
spamming the event? (See <http://msdn.microsoft.com/library/default.asp?
url=/library/en-us/gdi/hh/gdi/fontext_1nol.asp>)
No longer blocks: 85334
(Assignee)

Comment 32

17 years ago
I don't think attachment 40924 [details] [diff] [review] is a good way to fix this problem. Why don't we 
really _want_ to respond to WM_FONTCHANGE?
(Assignee)

Comment 33

17 years ago
Okay, if I had to guess, I'd say that the problem has to do with 
nsDocument::SetScriptGlobalObject(). When we're tearing down the document, we 
call SetDocument(nsnull) on each of the children in the document. That means 
that they won't have a back-pointer to the doc, which busts the invariant that 
the frame constructor assumes in ProcessChildren() (namely, that the content 
node have a document back-pointer).

I think we should consider making SetScriptGlobalObject() to clear the 
document's |mRootContent| member so that you can no longer get to these 
orphaned content nodes via the |GetRootElement()| API. (I think this is 
probably the correct fix for the problem.)

For the more paranoid members of the crowd, I think adding a check to assure 
that the element has a document in ProcessChildren() might make sense. Or, we 
could just _use_ the frame constructor's |mDocument| member directly, instead 
of asking the content node for it.

Thoughts?
(Assignee)

Comment 34

17 years ago
(Sorry, the ChildIterator is where we query the element for its document, so 
we'd have to fix it, which is what an earlier patch suggested and hyatt didn't 
like.)

Comment 35

17 years ago
I think that in Peter's case the WM_FONTCHANGE is being sent by the plugin.

It seemed to me that the reason for setting the font.internaluseonly.changed
pref was to refresh the FontCache. I couldn't see why we would want to reframe
after that, but...

Looking now, I see that I was mistaken. The DeviceContext prefCallback is not
for the font.internaluseonly.changed value, but for something else. The
DeviceContext->FlushFontCache is done by the presContext's PrefChanged callback.
So, the patch is wrong (on at least one account).
(Assignee)

Comment 36

17 years ago
Mmm, fun. I've found a wonderful way to crash the app. It turns out that my 
Win32 X-server (Exceed) spams the world with WM_FONTCHANGE messages every time 
a new window opens or closes. So far, I've found the following unchecked use of 
nsIContent::GetDocument()...
(Assignee)

Comment 37

17 years ago
Created attachment 40956 [details] [diff] [review]
add checks for null to nsIContent::GetDocument() call sites
(Assignee)

Comment 38

17 years ago
With the above patch, I'm unable to reproduce the crash with my Xserver 
hackery...
(Assignee)

Comment 39

17 years ago
(Taking for now.)
Assignee: karnaze → waterson
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.3

Comment 40

17 years ago
Chris, your patch worked for me, I think! I'm going to pull and build a fresh
source just to be sure, but my testcase:
http://www.shockwave.com/content/tankwars/tankwarslanding.html

Now seems to work with your patch applied.

Comment 41

17 years ago
waterson, your patch looks good. [s]r=attinasi
Good-looking wallpaper (or necessary defensive code, at least in the case of
nsBindingManager.cpp) -- [s]r=brendan@mozilla.org for the whole patch, FWIW. 
Ob. nit.:

+
+  // XXX document may be null if we're in the midst of paint suppression
+  if (!document)
+    return NS_OK;
+

There's no waterson-style space after the ! operator; OTOH, what is hyatt-style
when in XBL-Rome?

/be
(Assignee)

Comment 43

17 years ago
I think hyatt eschews the space. Less wrist pain.

Comment 44

17 years ago
Marking PDT+
Whiteboard: PDT+

Comment 45

17 years ago
waterson, pdt+ is the a= of the branch world -- are you comfortable to land
given the sr=?
(Assignee)

Comment 46

17 years ago
I'll check it in to the branch and the trunk as soon as the tree opens.
Keywords: patch
(Assignee)

Comment 47

17 years ago
Patch checked in on trunk and MOZILLA_0_9_2_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 48

17 years ago
Additional Comments From Chris Waterson 2001-07-02 14:13
>I don't think attachment 40924 [details] [diff] [review] is a good way to fix this problem. Why don't we 
>really _want_ to respond to WM_FONTCHANGE?

For information about why 'font.internaluseonly.changed' is needed, see
bug 69117 "When new font install into the system, mozilla should update's it's 
cached font list and use it".

If WM_FONTCHANGE is happening so frequently and erratically, then maybe too much 
unanticipated side-effects are happening. Hence, a fine-tuning might be needed, 
e.g., clear the font cache without destroying the whole world by going through 
the rather agressive things that happen when the pres-context is involved. 

Comment 49

17 years ago
I agree with rbs. Specifically, I think we should move the handling of the
WM_FONTCHANGE to the DeviceContext instead of the PresContext. Then, we could
just do the font cache flush and avoid the reframe (it seems wrong that the
PresContext has to know that much about the internals of the DeviceContext
anyway...). The PresContext currently registers for and handles equally all
notifications on all font prefs, so we need to put in a more specific set of
registrations (or filter the actual pref values). Is this worth opening a new
bug on?

Comment 50

17 years ago
[Since plugins, and (Exceed), and etc] "spams the world with WM_FONTCHANGE 
messages every time a new window opens or closes", I would definitely think that 
opening another bug to clear this issue is worth it.

Comment 51

17 years ago
*** Bug 90714 has been marked as a duplicate of this bug. ***

Comment 52

17 years ago
Marking verified in the 20010716 branch build under Windows ME.
Status: RESOLVED → VERIFIED
Crash Signature: [@ ChildIterator::ChildIterator]
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.