271669 - crash [@ nsXULDocument::AttributeChanged]

Reporter

Description

•

20 years ago

20041112306/trunk/W2K

I met crash while opening SeaMonkey Preferences window after startup, maybe I
have clicked checkbox in Download manager.

Comment's from same signature Talkback:
Opened preferences. Clicked + next to "Appearance" when crash occurred.
---
starting firefox after last crash
---
i opened my "mozilla 1.8a4_de_AT webbrowser 5 seconds later a system warning
appeared
---
Trying to open my bookmarks.
---
trying to install and subsiquently run newsmonster

TB2166970:
nsXULDocument::AttributeChanged 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/document/src/nsXULDocument.cpp,
line 1142]
nsXULElement::SetAttrAndNotify 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2202]
nsXULElement::SetAttr 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2126]
nsTreeContentView::ToggleOpenState 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/xul/base/src/tree/src/nsTreeContentView.cpp,
line 584]
XPTC_InvokeByIndex 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp,
line 102]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 2036]
XPC_WN_CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1288]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1288]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3621]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1307]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3621]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1307]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1384]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
3769]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1346]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 181]
nsXBLPrototypeHandler::ExecuteHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp,
line 491]
nsXBLEventHandler::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xbl/src/nsXBLEventHandler.cpp,
line 85]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1513]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1589]
nsXULElement::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/xul/content/src/nsXULElement.cpp,
line 2820]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5959]
PresShell::HandleEventWithTarget 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5875]
nsEventStateManager::CheckForAndDispatchClick 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 2942]
nsEventStateManager::PostHandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventStateManager.cpp,
line 1936]
PresShell::HandleEventInternal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 6012]
PresShell::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/base/src/nsPresShell.cpp,
line 5844]
nsViewManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2402]
nsViewManager::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsViewManager.cpp,
line 2131]
HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/view/src/nsView.cpp,
line 166]
nsWindow::DispatchEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1078]
nsWindow::DispatchWindowEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1095]
nsWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5329]
ChildWindow::DispatchMouseEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 5581]
nsWindow::ProcessMessage 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 4091]
nsWindow::WindowProc 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsWindow.cpp,
line 1356]
USER32.dll + 0x1ef0 (0x77e11ef0)
USER32.dll + 0x204c (0x77e1204c)
USER32.dll + 0x21af (0x77e121af)
nsAppStartup::Run 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/components/startup/src/nsAppStartup.cpp,
line 216]
main1 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1331]
main 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1802]
WinMain 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1828]
WinMainCRTStartup()
KERNEL32.DLL + 0x2893d (0x796f893d)

Boris Zbarsky [:bzbarsky]

Comment 1

•

20 years ago

Adam, is this reproducible?  And what's the actual build ID?  The one you gave
has one too many digits...

Adam Hauner

Reporter

Comment 2

•

20 years ago

20041112306/trunk/W2K should be 2004112306/trunk/W2K correctly.

I can't reproduce, but I filled it to track it, because I'm not only one seeing
it (there are 35 incidents actually in TB db). Many reports are about opening
Preferences, but not all.

Some other comments for record:
I clicked on Prefs|Avanced to check settings. And, here I am. Previously I tried
to install the home button v.0.7, which is supposed to be compatible with
1.8a5. No response.	
---
trying to reset SSL	
---
browser open to default page, went to Edit, Preferences, and Appearances. Crash!	
---
after upgrade from 1.8a4 to 1.8a5 try to change language in user-profile	
---
http://www.geocities.com/alfredkayser/mozilla/suite.htm
trying to install micro mozilla theme
---
Home Page - Google
Selected Preferences, pressed + for Appearance	
---
opening the bookmarks menu
---
http://planet.mozilla.org/
1) downloading nightly in DM
2) loading planet.mozilla.org
3) opening preferences	
---
starting firefox after last crash
---
Opened preferences. Clicked + next to "Appearance" when crash occurred.
---
i opened my "mozilla 1.8a4_de_AT webbrowser 5 seconds later a system warning
appeared "mozilla - exe hat fehler verursacht und wird geschlossen..." at this
momen running tasks: 1276:865 - FreeMail von WEB.DE - Opera c´t 23/2004, S.166:
Soft-Link -

Boris Zbarsky [:bzbarsky]

Comment 3

•

20 years ago

Well, it looks like the line number from talkback is bogus as usual (line 1142
can't possibly be crashing here).

The crash _could_ happen in line 1143 if something truly bizarre is going on and
aElement gets destroyed in the middle of this notification...

neil@parkwaycc.co.uk

Comment 4

•

20 years ago

Does the crash only happen with certain extension(s)?
Does anyone have a debug build they can crash and debug?

Adam Hauner

Reporter

Comment 5

•

20 years ago

Neil: I don't know, I usually don't use any extensions. Debug build - sorry, no.

Adam Hauner

Reporter

Comment 6

•

20 years ago

Boris, 
TB2363692 points to
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/xul/document/src/nsXULDocument.cpp&mark=1140&rev=AVIARY_1_0_20040515_BRANCH#1140
and TB2358776 points to:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/xul/document/src/nsXULDocument.cpp&mark=1136&rev=#1136
In both cases it is line with alone "aModType":

// Now notify external observers
for (PRInt32 i = mObservers.Count() - 1; i >= 0; --i) {
    nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
    observer->AttributeChanged(this, aElement, aNameSpaceID, aAttribute,
                              aModType);
}

Is it better?

Boris Zbarsky [:bzbarsky]

Comment 7

•

20 years ago

Hmm.. that could crash if we have a null or deleted observer...

Ian Neal

Comment 8

•

20 years ago

I got this crash but no matter how I try doing the same steps again, it will not
crash again. I was trying to expland the "Appearance" pref tree. Again the stack
trace points to the aModType line.

timeless

Comment 9

•

19 years ago

do we want to apply wallpaper? is there anything preventing these lists from
changing as the code loops around them?

Assignee: nobody → dbaron

Summary: crash [@ nsXULDocument::AttributeChanged() ] → crash [@ nsXULDocument::AttributeChanged]

Adam Guthrie

Comment 10

•

19 years ago

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

Adam Guthrie

Comment 11

•

19 years ago

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

Adam Guthrie

Updated

•

19 years ago

Status: UNCONFIRMED → NEW

Ever confirmed: true

OS: Windows 2000 → All

QA Contact: xptoolkit.xul

Hardware: PC → All

Roman Yepishev

Comment 12

•

19 years ago

OK, I managed to build the debug version and I can reproduce the same bug.

Could not figure what combination of account names/settings cause this and are there any relations of that kind, because it crashed randomly, sometimes working normally, sometimes not. There exists no other clickable button with arrow (except "print", but it's inactive at this stage), but it is clear that no activity should be performed before you click that nice button to make the crash happen again. I mean if any account has a 'Check on startup' it should be disabled as well as any possible start-up pop-ups.

#0  0x4094de06 in __libc_nanosleep () from /lib/libc.so.6
#1  0xfffffff4 in ?? ()
#2  0x4094dc87 in sleep () from /lib/libc.so.6
#3  0x08061232 in ah_crap_handler (signum=1084018420) at nsSigHandlers.cpp:132
#4  0x0806218a in nsProfileLock::FatalSignalHandler (signo=11)
    at nsProfileLock.cpp:210
#5  0x4020e5c3 in __pthread_sighandler () from /lib/libpthread.so.0
#6  <signal handler called>
#7  nsXULDocument::AttributeChanged (this=0x832fcf0, aElement=0x8577f58, 
    aNameSpaceID=0, aAttribute=0x81df650, aModType=2)
    at ../../../../../src/mozilla/content/xul/document/src/nsXULDocument.cpp:1138
#8  0x41aa0e38 in nsXULElement::SetAttrAndNotify (this=0x8577f58, 
    aNamespaceID=0, aAttribute=0x81df650, aPrefix=0x0, aOldValue=@0xbf845cd0, 
    aParsedValue=@0xbf845d68, aModification=0, aFireMutation=0, aNotify=1)
    at ../../../../../src/mozilla/content/xul/content/src/nsXULElement.cpp:1515
#9  0x41aa1223 in nsXULElement::SetAttr (this=0x8577f58, aNamespaceID=0, 
    aName=0x81df650, aPrefix=0x0, aValue=@0xbf845dcc, aNotify=1)
    at ../../../../../src/mozilla/content/xul/content/src/nsXULElement.cpp:1440
#10 0x4178e95b in nsIContent::SetAttr (this=0x0, aNameSpaceID=0, aName=0x0, 
    aValue=@0x0, aNotify=0) at nsIContent.h:284
#11 0x4192fa1b in nsMenuFrame::OpenMenu (this=0x87d8ee8, aActivateFlag=1)
    at ../../../../../src/mozilla/layout/xul/base/src/nsMenuFrame.cpp:732
#12 0x4192cbad in nsMenuFrame::ToggleMenuState (this=0x87d8ee8)
    at ../../../../../src/mozilla/layout/xul/base/src/nsMenuFrame.cpp:551
#13 0x41931319 in nsMenuFrame::HandleEvent (this=0x87d8ee8, 
    aPresContext=0x83fff30, aEvent=0xbf846258, aEventStatus=0xbf846094)
    at ../../../../../src/mozilla/layout/xul/base/src/nsMenuFrame.cpp:407
#14 0x4178af30 in PresShell::HandleEventInternal (this=0x821d0b8, 
    aEvent=0xbf846258, aView=0x8400768, aFlags=1, aStatus=0xbf846094)
    at ../../../src/mozilla/layout/base/nsPresShell.cpp:6408
#15 0x4178bf2a in PresShell::HandleEvent (this=0x821d0b8, aView=0x8400768, 
    aEvent=0xbf846258, aEventStatus=0xbf846094, aForceHandle=1, 
    aHandled=@0xbf84608c)
    at ../../../src/mozilla/layout/base/nsPresShell.cpp:6203
#16 0x41ad2b14 in nsViewManager::HandleEvent (this=0x84006e8, aView=0x8400768, 
    aEvent=0xbf846258, aCaptured=0)
    at ../../../src/mozilla/view/src/nsViewManager.cpp:2557
#17 0x41ad368e in nsViewManager::DispatchEvent (this=0x84006e8, 
    aEvent=0xbf846258, aStatus=0xbf8461c0)
    at ../../../src/mozilla/view/src/nsViewManager.cpp:2246
#18 0x41ac5e73 in HandleEvent (aEvent=0xbf846258)
    at ../../../src/mozilla/view/src/nsView.cpp:171
#19 0x4146b55e in nsCommonWidget::DispatchEvent (this=0x84007d8, 
    aEvent=0xbf846258, aStatus=@0xbf8462a8)
    at ../../../../src/mozilla/widget/src/gtk2/nsCommonWidget.cpp:219
#20 0x4146485e in nsWindow::OnButtonPressEvent (this=0x84007d8, 
    aWidget=0x8331ac8, aEvent=0x82fe828)
    at ../../../../src/mozilla/widget/src/gtk2/nsWindow.cpp:1556
#21 0x41464933 in button_press_event_cb (widget=0x0, event=0x82fe828)
    at ../../../../src/mozilla/widget/src/gtk2/nsWindow.cpp:3706
#22 0x4036f6cc in _gtk_marshal_BOOLEAN__BOXED ()
   from /usr/lib/libgtk-x11-2.0.so.0
#23 0x4061a5fc in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0
#24 0x4062a2f3 in g_signal_stop_emission () from /usr/lib/libgobject-2.0.so.0
#25 0x4062b2d1 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0
#26 0x4062b8c9 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0
#27 0x40456db6 in gtk_widget_event_internal ()
   from /usr/lib/libgtk-x11-2.0.so.0
#28 0x4036dbd5 in gtk_propagate_event () from /usr/lib/libgtk-x11-2.0.so.0
#29 0x4036e09f in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0
#30 0x4055d1ea in gdk_event_dispatch () from /usr/lib/libgdk-x11-2.0.so.0
#31 0x40673b7c in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#32 0x40676f6b in g_main_context_check () from /usr/lib/libglib-2.0.so.0
#33 0x40677287 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#34 0x4036d1b3 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#35 0x41469342 in nsAppShell::Run (this=0x8190948)
    at ../../../../src/mozilla/widget/src/gtk2/nsAppShell.cpp:139
#36 0x415405b1 in nsAppStartup::Run (this=0x8190900)
    at ../../../../../src/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:150
#37 0x08051169 in XRE_main (argc=1, argv=0xbf846bc4, aAppData=0x8067500)
    at ../../../src/mozilla/toolkit/xre/nsAppRunner.cpp:2313
#38 0x0804b398 in main (argc=0, argv=0x0)
    at ../../../src/mozilla/mail/app/nsMailApp.cpp:62

Boris Zbarsky [:bzbarsky]

Comment 13

•

19 years ago

So when you crash, what do local variables look like?  What's the exact line of code you crash on?

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 14

•

19 years ago

In general we probably should be copying the observers array before notifying observers; it's a safer pattern to use.  But I don't see why the bug was assigned to me.

Assignee: dbaron → nobody

Roman Yepishev

Comment 15

•

19 years ago

The built in debug trace emits this right before segfault:

###!!! ASSERTION: index out of range: '0 <= aIndex && aIndex < Count()', file ../../dist/include/xpcom/nsVoidArray.h, line 80
Break: at file ../../dist/include/xpcom/nsVoidArray.h, line 80

Boris Zbarsky [:bzbarsky]

Comment 16

•

19 years ago

What's the stack trace to that assert?  What's the index look like there?

Roman Yepishev

Comment 17

•

19 years ago

I couldn't trace the behavior while running tb in debugger. It works without any errors or crashes when started this way :-/

Steve England [:stevee]

Comment 18

•

19 years ago

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

Adam Guthrie

Comment 19

•

19 years ago

According to talkback, this is the #11 topcrasher for Firefox15.

Keywords: topcrash

Vlad Dascalu

Comment 20

•

19 years ago

I'm able to reproduce this bug with Thunderbird 1.5.

Upon startup, click the drop-down next to the "Get Messages" button.

I have 100k+ mails, and it seems it takes a couple of seconds on startup to initialize. If I click the "Get Messages" button within those seconds, I get a reproductible crash.

Talkback crashIDs: TB14397314X TB14461878H TB14461884E TB14512083Y TB14512175H TB14512178W TB14512183W TB14512189K


The crash happens all the time in the notification of observers thing, in nsXULDocument::AttributeChanged

1135 waterson            // Now notify external observers
1136                     for (PRInt32 i = mObservers.Count() - 1; i >= 0; --i) {
1137                         nsIDocumentObserver*  observer = (nsIDocumentObserver*)mObservers[i];
1138 jst                     observer->AttributeChanged(this, aElement, aNameSpaceID, aAttribute,
1139                                        aModType);

Crashing at 1138.

Vlad Dascalu

Comment 21

•

19 years ago

Regarding the previous post, no extensions installed except Talkback.

Using Mozilla Thunderbird version 1.5 (20051201)

Vlad Dascalu

Comment 22

•

19 years ago

Also, "Get Messages" should read "Get Mail" in comment #20.

Vlad Dascalu

Comment 23

•

19 years ago

Hmm, code-wise: do we have here some kind of lock in order to ensure that nobody else modifies the observers array while we do the 'for' loop?

If an array is removed while we're in the for loop, a crash would easily happen.

Any code gurus can answer if there is a lock or not? (I could take a peak at the code but I'm way behind :) )

Vlad Dascalu

Comment 24

•

19 years ago

array -> observer

Adam Guthrie

Updated

•

19 years ago

Flags: blocking1.8.1?

Flags: blocking1.8.0.2?

Bernd

Comment 25

•

19 years ago

#10 0x4178e95b in nsIContent::SetAttr (this=0x0, aNameSpaceID=0, aName=0x0, 
    aValue=@0x0, aNotify=0) at nsIContent.h:284

Doesnt that mean that at
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsMenuFrame.cpp#737
mContent is not valid?

Vlad Dascalu

Updated

•

19 years ago

Whiteboard: reproductible

Boris Zbarsky [:bzbarsky]

Comment 26

•

19 years ago

> Hmm, code-wise: do we have here some kind of lock in order to ensure that
> nobody else modifies the observers array while we do the 'for' loop?

No, we do not.  nsDocument has some such protection; this class does not.

If you can reproduce reliably, I can post a patch to do the nsDocument thing for you to try...

bernd, that stack frame looks confused (note that the attr name is 0 too and that everything is fine in the next frame).

Daniel Veditz [:dveditz]

Comment 27

•

19 years ago

No fix, misses 1.5.0.2

Flags: blocking1.8.0.3?

Flags: blocking1.8.0.2?

Flags: blocking1.8.0.2-

Vlad Dascalu

Comment 28

•

19 years ago

(In reply to comment #26)
> If you can reproduce reliably, I can post a patch to do the nsDocument thing
> for you to try...

That would mean setting up an environment where I can compile/run a debug version of Thunderbird, which I don't have the time for, currently.

Boris Zbarsky [:bzbarsky]

Comment 29

•

19 years ago

So actually, AttributeChanged handles an observer removing itself; just not others.  Just like nsDocument...

Marc Bejarano

Comment 30

•

18 years ago

TB16449102Z, too

Darin Fisher

Comment 31

•

18 years ago

boris: are you interested in trying to fix this for the 1.8 branches?  if a fix is simple enough (such as what dbaron proposed in comment #14) then perhaps we should try to get this into the stable branches.  thoughts?

Boris Zbarsky [:bzbarsky]

Comment 32

•

18 years ago

Copying the observers array might work... it'd also be a pretty big perf hit.  And it's not immediately obvious to me that it would help.  I suppose we could write up a patch to do that and see whether it does help somehow...

Daniel Veditz [:dveditz]

Comment 33

•

18 years ago

A fix in 1.8.0.3 isn't looking like it will happen. Please renominate if this gets a patch on the trunk.

Flags: blocking1.9a1?

Flags: blocking1.8.0.3?

Flags: blocking1.8.0.3-

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Updated

•

18 years ago

Whiteboard: reproductible → reproducible

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 34

•

18 years ago

Attached patch possible patch (obsolete) — Details — Splinter Review

This does what I suggested in comment 14.

Assignee: nobody → dbaron

Status: NEW → ASSIGNED

Attachment #217776 - Flags: superreview?(jst)

Attachment #217776 - Flags: review?(bzbarsky)

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Updated

•

18 years ago

Whiteboard: reproducible → [patch] reproducible

Boris Zbarsky [:bzbarsky]

Comment 35

•

18 years ago

Comment on attachment 217776 [details] [diff] [review]
possible patch

I'll try to review tonight, but this looks like a case where a macro (with PR_BEGIN/END_MACRO to handle scoping for the nsCOMArray) with the method&args for the call on each observer as the arg would be nice.

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 36

•

18 years ago

Comment on attachment 217776 [details] [diff] [review]
possible patch

I'll post such a patch after dinner.

Attachment #217776 - Attachment is obsolete: true

Attachment #217776 - Flags: superreview?(jst)

Attachment #217776 - Flags: review?(bzbarsky)

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 37

•

18 years ago

Attached patch possible patch — Details — Splinter Review

Attachment #217808 - Flags: superreview?(jst)

Attachment #217808 - Flags: review?(bzbarsky)

Boris Zbarsky [:bzbarsky]

Comment 38

•

18 years ago

Comment on attachment 217808 [details] [diff] [review]
possible patch

Looks good.  Here's hoping the perf hit will not be noticeable.  ;)

Attachment #217808 - Flags: review?(bzbarsky) → review+

Johnny Stenback (:jst)

Comment 39

•

18 years ago

Comment on attachment 217808 [details] [diff] [review]
possible patch

sr=jst

Attachment #217808 - Flags: superreview?(jst) → superreview+

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 40

•

18 years ago

Checked in to trunk.

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 41

•

18 years ago

So it looks like this did hurt Tdhtml and Txul a little.

Boris Zbarsky [:bzbarsky]

Comment 42

•

18 years ago

Makes sense; those have tons of document observer notifications....

It might help to have fewer observers (eg have the presshell be notified by the binding manager).

Not sure what else would help, offhand.  :(

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Updated

•

18 years ago

Attachment #217808 - Flags: approval-branch-1.8.1?(bzbarsky)

Boris Zbarsky [:bzbarsky]

Updated

•

18 years ago

Attachment #217808 - Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1+

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Updated

•

18 years ago

Attachment #217808 - Flags: approval1.8.0.3?

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Updated

•

18 years ago

Depends on: 327256

Daniel Veditz [:dveditz]

Comment 43

•

18 years ago

Fixed per comment 40

Status: ASSIGNED → RESOLVED

Closed: 18 years ago

Resolution: --- → FIXED

Darin Fisher

Comment 44

•

18 years ago

Can you guys keep a nsCOMArray around as a member variable that will generally be large enough to avoid a call to malloc?

Boris Zbarsky [:bzbarsky]

Comment 45

•

18 years ago

Darin, at the moment notifications can actually nest in some cases.... Probably a bug in most of those, but it precludes using a member for any sort of temporary here.

Darin Fisher

Comment 46

•

18 years ago

OK.  You could, I suppose, use a temp nsCOMArray when the member variable is not empty or if some boolean flag is set.

Daniel Veditz [:dveditz]

Comment 47

•

18 years ago

Comment on attachment 217808 [details] [diff] [review]
possible patch

approved for 1.8.0 branch, a=dveditz for drivers

Attachment #217808 - Flags: approval1.8.0.3? → approval1.8.0.3+

David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)

Assignee

Comment 48

•

18 years ago

Checked in to MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.

Marking appropriate fixed keywords, although we're really not sure if this fixes it, I don't think.

Keywords: fixed1.8.0.3, fixed1.8.1

Jan Varga [:janv]

Comment 49

•

18 years ago

Attached patch a back port for Firefox 1.0 — Details — Splinter Review

Boris Zbarsky [:bzbarsky]

Updated

•

18 years ago

Depends on: 340733

Adam Guthrie

Comment 50

•

18 years ago

This crash isn't showing up in the topcrash lists for 2.0 or 1.5.0.8. Marking as VERIFIED and clearing blocking flags.

Status: RESOLVED → VERIFIED

Flags: blocking1.9a1?

Flags: blocking1.8.1?

Keywords: fixed1.8.0.4, fixed1.8.1 → verified1.8.0.4, verified1.8.1

timeless

Updated

•

16 years ago

Component: XP Toolkit/Widgets: XUL → XUL

QA Contact: xptoolkit.xul → xptoolkit.widgets

Nobody; OK to take it and work on it

Updated

•

13 years ago

Crash Signature: [@ nsXULDocument::AttributeChanged]

possible patch 18 years ago David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) 25.39 KB, patch		Details \| Diff \| Splinter Review
possible patch 18 years ago David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) 23.89 KB, patch	bzbarsky : review+ jst : superreview+ bzbarsky : approval-branch-1.8.1+ dveditz : approval1.8.0.4+	Details \| Diff \| Splinter Review
a back port for Firefox 1.0 18 years ago Jan Varga [:janv] 24.21 KB, patch		Details \| Diff \| Splinter Review