82194 - Crash in Trunk & M091 [@ nsSliderFrame::CurrentPositionChanged]

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Simon Fraser [no longer active]]

I'm crashing in nsSliderFrame::CurrentPositionChanged() at the
thumbFrame->GetRect(thumbRect);
line because thumbFrame is null.

To repro:
1. Go to the URL above
2. Scroll down until you see the 'I Accept' button (I scrolled by dragging
   the thumb)
3. Mouse over the button (don't click it!)
4. Hit the reload button

You'll crash. Stack coming

Comment 1

[Simon Fraser [no longer active]]

Attachment: Stack of the crash

Comment 2

[Eric Vaughan]

Not sure why anyone is creating a slider with no thumb but here is the patch so 
it won't crash when you do:

Index: nsSliderFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xul/base/src/nsSliderFrame.cpp,v
retrieving revision 1.68
diff -u -r1.68 nsSliderFrame.cpp
--- nsSliderFrame.cpp	2001/03/02 03:07:50	1.68
+++ nsSliderFrame.cpp	2001/05/22 20:11:22
@@ -688,6 +688,9 @@
 
     // get the thumb's rect
     nsIFrame* thumbFrame = mFrames.FirstChild();
+    if (!thumbFrame)
+      return NS_OK;
+
     nsRect thumbRect;
     thumbFrame->GetRect(thumbRect);
 

Comment 3

[Simon Fraser [no longer active]]

I could not repro on Windows. Maybe this has something to do with the Mac smart 
scrolling settings?

Comment 4

[Aaron Leventhal]

Easy
r=aaronl

Comment 5

[Jay Patel [:jay]]

Is this particular crash a Mac only bug?  Talkback data is also showing a 
similar crash with win32 (mostly Windows 2K).  The Windows crash is reported in 
bug 82363...can someone look into that and see if it's a dup?

Comment 6

[Jaime Rodriguez, Jr.]

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

Comment 7

[Jaime Rodriguez, Jr.]

This is also happening on Win NT. Pls see bug 82363

Comment 8

[Chris Abbey]

bug 81754 looks like it could be related (not the same though).

Comment 9

[Håkan Waara]

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

Comment 10

[Håkan Waara]

Eek! This has been crashing for me for two days now, in nightlies when I press back.

Every time I press back, actually (I'm upgrading to blocker, because I can't use
my browser with it.)

Shouldn't we track down the original problem also after wallpapering over this
crash?

Can someone please sr= and check it in to the trunk?

Comment 11

[Chris Abbey]

has anyone else tried the patch? For me it just changed the crash to something
gdb can't cope with... still crashes in the same situation I described in my bug
81754. So as Håkan suggests, the root problem still needs to be found and dealt
with.

Comment 12

[Axel Hecht]

Confirming this for solaris, just managed to get this when pressing forward
button. Cool. I recall this warning before each crash I had so far:
WARNING: not supported for views, file
/tmp/mozilla/view/src/nsScrollPortView.cpp, line 98
(that is, I don't recall crashing without that warning)

Axel

Comment 13

[gabriel]

Weird...I just tried this with Linux build 2001052308. When I click on the URL
above, I get immediately redirected to Netscape search for the word 'dogspit' !!

Comment 14

[Shiva Thirumazhusai]

Adding topcrash keyword.  

Comment 15

[Chris Waterson]

This patch feels wallpaperish to me. How can we create a slider without a thumb?
I'm also worried about Chris Abbey's comments on 2001-05-27 12:47, where he says
this just moves the crash somewhere else. sfraser: have you verified that the
patch fixes the problem?

Comment 16

[Chris Abbey]

Chris, I believe the crash that I was moving into was fixed on 5/28 or 29, as 
the secondary crash I was experiencing went away somewhere in that timeframe, 
and as of this evening I went a full three hours with lots of stuff that 
should have triggered this and didn't.

(that fix could have been integrated later, I had about 10 patches applied to 
a tree from 5/27 in the last few days trying to get ready to use moz to demo a 
chunk of new web content this morning.) As of right now I only have this patch 
in my tree, the others have all been merged by now and I just finished 
pulling. Net is that while I agree that this is a wallpaper, it appears to be 
an effective wallpaper now.

Having said that though, I seem to be able to generate this "not so very good" 
condition all too easily with just the use of a scroll mouse and the history 
back/forward buttons. Perhaps tommorrow I'll change the return 
to an assert followed by a return so I can see how often it happens. Anyone 
got an idea how we can track the lifecycle of a single slider after the fact? 
maybe see where it came from??
(actually I want to see if the correlation that Axel Hecht and I both noticed 
is 100%....)

Comment 17

[Axel Hecht]

I attach two stack traces of the warning in nsScrollPortView::Release, the
object is 0x2, ugh.
I realized that this bug seems to be pretty dependent on whether I can see the
scrollbar or not.
I didn't manage to trigger it while the scrollbar was offscreen. I can't recall
to trigger it while having the window on my current desktop, but when I 
change to another virtual desktop while it's trying to load, it booms.

This happens in 
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsScrollBoxFrame.cpp#815
which has a nsCOMPtr of a view. We don't do refcounting on views, right?
This was added on 2001/05/12 by Dan, CCing dr.

Axel

Comment 18

[Axel Hecht]

Attachment: first stack trace to releasing a view

Comment 19

[Axel Hecht]

Attachment: ... and a second one

Comment 20

[Dan Rosen]

This is my mistake. That patch isn't right, or is only half-right. The view
shouldn't be an nsCOMPtr, and the QI should be |CallQueryInterface|. I'm not
seeing the crash, but I'll put together a patch for y'all to test, ASAP.

Comment 21

[Dan Rosen]

Attachment: [patch] nsCOMPtr::do_QueryInterface => CallQueryInterface, and bulletproof scrollbar per evaughan

Comment 22

[Dan Rosen]

Ok, Patch attached compiles but is untested (my tree is clobbered, but I wanted
to get the patch attached with The Quickness). Please test, review, approve.

Comment 23

[Chris Waterson]

Hello? Beuller? Anyone? Why is the thumb frame null?

Comment 24

[Dan Rosen]

waterson: No idea. The mis-nsCOMPtr-ing of the view is probably the root cause
(although, like I said, I haven't tested this patch -- my tree is building at
the moment), but it shouldn't hurt. OTOH, if the nsCOMPtr'ed view turns out to
be entirely orthogonal to the problem, then yeah... (and I'll reassign this back
to evaughan if that's the case).

Comment 25

[Simon Fraser [no longer active]]

I don't think the mis nsCOMPtr-ing of the view has anything to do with it. I bet 
nsView has stubbed out AddRef/Release methods that do nothing. Someone (Eric?) 
needs to dig around and find out why we can get into a state where the thumb 
frame is null. Are we in the middle of a reflow? (yes, look at the stack).

Comment 26

[Axel Hecht]

Hi Dan,
sorry to say that, but this still crashes.
Program received signal SIGSEGV, Segmentation fault.
0xfb7562fc in nsSliderFrame::CurrentPositionChanged (this=0x76ce04, 
    aPresContext=0x8d5710)
    at /tmp/mozilla/layout/xul/base/src/nsSliderFrame.cpp:693
693         thumbFrame->GetRect(thumbRect);

Get over to #mozilla, if I can help you with my gdb, I'm at the segfault right
now.

Axel (alias Pike)

Comment 27

[Dan Rosen]

Yes, nsView.cpp does have AddRef and Release methods which do nothing but print
warnings (it has to, since it implements nsISupports). If this isn't me, then
back to evaughan.

Comment 28

[Axel Hecht]

attaching current stacktrace, I'll be on #mozilla for like 15 mins for questions
and then I'm off to food/bed.
btw, dr didn't use evaughan's original wallpaper, so I didn't test that.

Axel

Comment 29

[Axel Hecht]

Attachment: stacktrace to nsSliderFrame::CurrentPositionChanged

Comment 30

[Chris Abbey]

ok, the stub'd out AddRef and Release in nsView are interesting and all, but
it's the parent/base class here... so I'm not that surprised that some methods
aren't real. But nsScrollPointView extends nsView, and then overrides these
methods with ... the exact same code, copied and pasted without even updating
the message.

I've applied this diagnostic patch to yesterday's tree:

Index: nsSliderFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xul/base/src/nsSliderFrame.cpp,v
retrieving revision 1.68
diff -u -u -b -r1.68 nsSliderFrame.cpp
--- nsSliderFrame.cpp	2001/03/02 03:07:50	1.68
+++ nsSliderFrame.cpp	2001/06/01 03:07:02
@@ -688,6 +688,11 @@
 
     // get the thumb's rect
     nsIFrame* thumbFrame = mFrames.FirstChild();
+    if (!thumbFrame) {
+      NS_WARNING("thumbFrame is null, imagine a segv here....");
+      return NS_OK;
+    }
+
     nsRect thumbRect;
     thumbFrame->GetRect(thumbRect);
 
basically its evaughan's wallpaper with a printout... here's the interesting part:

WARNING: not supported for views, file nsScrollPortView.cpp, line 98
WARNING: thumbFrame is null, imagine a segv here...., file nsSliderFrame.cpp,
line 692
WARNING: not supported for views, file nsScrollPortView.cpp, line 98
WARNING: not supported for views, file nsScrollPortView.cpp, line 98

I've done this a dozen or more times already by following the directions in my
original bug 81754. These warnigns *always* show up together, one not supported,
one is null, two or three not supported... all in a single page replacement.

I believe this is where the null thumbFrame is coming from. I've started up a
gdb session with breakpoints set on these two warnings... hopefully it will load
before I fall asleep.

Comment 31

[Chris Abbey]

argh. gdb is being a pain, see the post in n.p.m.unix entitled "gdb5.0 and
mozilla -- can't find thread" if anyone can lend some gdb know how.....

Comment 32

[Phil Peterson]

Talked to Eric yesterday and he said it was legal for a thumb frame not to
exist. Let's get *some* crash prevention checked in soon.

Comment 33

[Dan Rosen]

Chris: You need to pull a new snapshot of gdb/insight. See:

  http://mozilla.org/unix/debugging-faq.html#insight

Insight, BTW, is a GUI front end for gdb. It doesn't suck.

Comment 34

[Chris Waterson]

(Bullied from all sides!) Well, I certainly am not against checking in the
bail-out code if someone will confirm that it actually fixes the bug! :-)

sr=waterson

Comment 35

[Dan Rosen]

I'll give r= as well, so long as the patch applies the do_QueryInterface => 
CallQueryInterface change I made in my patch. Thanks.

dr . o O ( wouldn't it be nice if evaughan looked at this bug? )

Comment 36

[Håkan Waara]

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

Comment 37

[Dan Rosen]

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

Comment 38

[Dan Rosen]

adding keywords from 83669, reassigning to hyatt, to give to the right person.

Comment 39

[David Hyatt]

Fixed.

Comment 40

[K Carter]

What about the 0.9.1 Branch?

Comment 41

[Chris Petersen]

Marking verified in both trunk and branch builds (6/04).

Comment 42

[Tracy Walker [:tracy]]

reopening

seen on mac commercial build 2001-006-05-04-trunk

This is reproducable still in classic skin.  Modern theme works fine.

Comment 43

[Alec Flett]

its happening on the branch as well - I've sent in a number of talkback reports
on it...

Comment 44

[selmer (gone)]

Clearing old whiteboard and updating.

Comment 45

[Simon Fraser [no longer active]]

Fixing this was just moving scrollbar binding XML around, right? It should be 
easy to fix.

Comment 46

[David Hyatt]

Well, this must be trashed memory and not a null pointer dereference then.  

Comment 47

[Tracy Walker [:tracy]]

still seeing this crash on Mac commercial trunk build 2001-06-06-04-trunk in 
Tasks | Tools | History

Comment 48

[Simon Fraser [no longer active]]

Spam some more themes/XUL folks. Can someone eyeball the Mac classic scrollbar 
stuff, and check that it's OK?

Comment 49

[David Hyatt]

All three scrollbars (in all three skins) have the same split file-wise.

Comment 50

[David Hyatt]

Can someone repro the crash and tell me if memory is trashed?  I've bullet-
proofed the function on the trunk to bail if the thumb is null, so I don't 
think this is an XBL issue.

Comment 51

[David Hyatt]

Ok, this bug has been fixed.  The crash doing Tasks-Tools-History on the Mac is 
in a completely different place.

Marking FIXED.  If you want to file a bug on the History crash, go ahead, but 
it's not crashing here.

Comment 52

[Simon Fraser [no longer active]]

The classic History crash is bug 83669.

Comment 53

[Tracy Walker [:tracy]]

reopened bug 83669 which was a bug that got duped of this.

Comment 54

[Phil Peterson]

I crashed here using the official mozilla 0.9.1 build on Sunday. Was this fixed
only on the trunk?

Comment 55

[Alec Flett]

I've crashed in what I think is this bug MANY times in the branch... I have
filed at least 10 talkback reports since 6/6, all having to do with going "back"
to a bug in bugzilla. it doesn't happen EVERY time, but it's probably 50% of the
time. I'd direct someone to a talkback URL, but the talkback search seems to be
down..

Comment 56

[Chris McAfee]

this bug was fixed via checkin 1.70 for
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/layout/xul/base/src/nsSliderFrame.cpp

this checkin did not make it to the MOZILLA_0_9_1_BRANCH, as far as I can tell.

Comment 57

[Alec Flett]

and now it's back up. Sure enough, out of the 25 or so crashes that I have
reported, about 20 of them are in nsSliderFrame::CurrentPositionChanged

http://climate/reports/searchemail.cfm?email=alecf%40netscape.com

Comment 58

[Alec Flett]

in fact, 13 out of my 15 branch crashes were this bug. This seems like a
netscape beta stopper to me

Comment 59

[Chris McAfee]

reopening for branch comments

Comment 60

[Chris McAfee]

hyatt says this is a trivial fix and fixes the bug.

Comment 61

[Jan Carpenter]

topcrash for M091

Here are the comments from recent crashes:

    (31567395) Comments: Typing URL
     (31565606) Comments: Crashed whilst changing preferences.
     (31555561) Comments: pressed back button in main toolbar
     (31548065) Comments: I was trying to edit the URL and I got http://ww and 
it
frozethis was just after changing to the new modern theme
     (31544414) URL: http://lionsource.com/
     (31544414) Comments: Accessing the history via toolbar to get back to the 
site
forum list.
     (31543998) URL: http://www.macromedia.com/
     (31543998) Comments: Downloading the Macromedia Flash plugin.
     (31540042) Comments: i was setting up  the preferences like languajes 
(spanish)
     (31536886) Comments: Back on eBay
     (31536264) Comments: Back in Bugzilla
     (31535646) Comments: ?
     (31532161) Comments: Mozilla often crashes when I press the back-button. 
Both on
the toolbar and right-mousebutton-click.
     (31531630) Comments: Typing an URL
     (31527806) Comments: Only starting Mozilla
     (31522398) Comments: viewing an html file stored locallycrash occurred just 
after
closing Preferences
     (31518401) Comments: working with the settings menu .. browser reconfigured 
itself
and then crashed
     (31513567) Comments: I was typing an URL in the location field
     (31509500) Comments: i just looked through the whole preferences dialog
     (31504583) Comments: typing in the URL location box
     (31503897) Comments: I was about to write "www" in the address bar. The 
program
crashed just before the 3rd "w". Wow. 
     (31502695) Comments: I had minimized mozilla (i'm using blackbox
     (31494236) Comments: reload clicked
     (31493543) Comments: back by Alt + Left Allow
     (31491891) Comments: the arrow keys weren't moving the cursor in the adress 
bar
     (31484031) Comments: hitting back viewing an attachment! this is getting 
rediculous
     (31483935) Comments: hitting back again.. 
     (31482671) Comments: typing an address  in the URL input box
     (31481987) Comments: Typing in text to the location field.It was a blank 
page
     (31480247) Comments: typing location in location bar
     (31479004) Comments: Closing the Preferences dialog
     (31477501) Comments: I just unpacked mozilla 0.9.1 and configured it.I 
typed latex
in the URL textfield.It crashed.
     (31477247) Comments: it was loading a webpage
     (31476445) Comments: Reloading the local Doppler weather radar image in the
browser.Accessed the weather link from the browser sidebar.
     (31474914) Comments: After setting all of my preferences and hitting ok
     (31473289) URL: http://www.malayalamanorama.com
     (31473289) Comments: typing in the url  
     (31470595) Comments: clicked back button
     (31469867) Comments: typing URL after saving preferences
     (31469185) Comments: saved preferents
     (31469131) URL: www.mozilla.org
     (31469131) Comments: Changing preferences (Edit->Preferences)
     (31468918) URL: mozilla.org
     (31468918) Comments: scrolling the mouse
     (31467461) Comments: enabled the paint/flash/debug option out of curiosity.
     (31464856) Comments: hit back button/ focused mail


Stacktrace from 2001060713
OS: Windows NT

Incident ID 31567817 
nsSliderFrame::CurrentPositionChanged 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSliderFrame.cpp, line 695] 
nsSliderFrame::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSliderFrame.cpp, line 224] 
nsCSSFrameConstructor::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 10083] 
StyleSetImpl::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp, line 1290] 
PresShell::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 4816] 
nsDocument::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\base\src\nsDocument.cpp, line 1676] 
nsHTMLDocument::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\html\document\src\nsHTMLDocument.cpp, line 
1284] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3057] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3077] 
nsXBLPrototypeBinding::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLPrototypeBinding.cpp, line 
670] 
nsXBLBinding::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLBinding.cpp, line 1348] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3021] 
nsXULElement::SetAttribute 
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp, line 
3077] 
nsGfxScrollFrameInner::SetAttribute 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
1423] 
nsGfxScrollFrameInner::ScrollPositionDidChange 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
851] 
nsScrollPortView::ScrollTo 
[d:\builds\seamonkey\mozilla\view\src\nsScrollPortView.cpp, line 333] 
nsScrollBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsScrollBoxFrame.cpp, line 527] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
985] 
nsContainerBox::LayoutChildAt 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 595] 
nsGfxScrollFrameInner::LayoutBox 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
1039] 
nsGfxScrollFrameInner::Layout 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
1149] 
nsGfxScrollFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
1047] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
985] 
nsBoxFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 781] 
nsGfxScrollFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 
738] 
nsContainerFrame::ReflowChild 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 
745] 
ViewportFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp, line 538] 
nsHTMLReflowCommand::Dispatch 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLReflowCommand.cpp, line 
145] 
PresShell::ProcessReflowCommand 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5712] 
PresShell::ProcessReflowCommands 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5767] 
ReflowEvent::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5625] 
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591] 
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, 
line 524] 
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 
1072] 
nsAppShellService::Run 
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp, line 418] 
netscp6.exe + 0x16b5 (0x004016b5) 
netscp6.exe + 0x11b8 (0x004011b8) 
netscp6.exe + 0x2f7b (0x00402f7b) 
KERNEL32.DLL + 0x192a6 (0x77e992a6) 

Comment 62

[David Hyatt]

This has been fixed on the trunk, and I assume it's too late to get it into 
0.9.1, so I'm closing it again.

Comment 63

[Chris Petersen]

Looks fine on the 6/27 branch build. Marking verified