138720 - [PATCH] Startup crash on M1RC2 Trunk due to native scrollbars being specified in a theme [@ nsNativeScrollbarFrame::Init]

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[greer]

The Talkback data shows a group of users crashing the M1.0 release candidate and
the Trunk at startup. All of the crashes are on the Windows platform. I'll
attach some stacks and Talkback data below.

Comment 1

[greer]

Attachment: Talkback data

The data in M10RC1 is not as full as that of a release so the numbers are
smaller, but it would be good to nail a startup crash before it gets that far.

Comment 2

[Laura Krietemeyer]

Changing QA Contact

Comment 3

[Marc Attinasi]

Attachment: patch to check for null scrollbar before using it.

Cannot dup the crash, so putting in safety checks and returning error if
scrollbar widget cannot be created.

Comment 4

[Marc Attinasi]

Accepting, null-check patch attached

Comment 5

[Jay Patel [:jay]]

Adding qawanted keyword to see if we can reproduce this in-house.

Comment 6

[greer]

I have been able to reproduce this one twice using M1RC1 with these steps on a 
WinNT4.0 box:

1) Open http://homepage.mac.com/kgerich/pinstripe.html in a tab
2) Open another page in a new tab (I chose cnn.com)
3) In the first tab click on "install Pinstripe"
If it doesn't crash at this point then
4) Click on the second tab while the skin is installing

You should crash.

Comment 7

[greer]

adt1.0.0 per Shiva.

Needs r=/sr= for this one liner.

Comment 8

[greer]

User-
(5842729) - [Windows NT  5.1 build 2600] (Build 2002041717): I installed a skin 
that somehow messed up mozilla.  I tried to uninstall mozilla  I deleted the 
directory  rebooted the computer and did a fresh install of mozilla and I still 
can not run mozilla.

Greer-
I've been able to crash this one with a branch build 2002042512. Following my 
initial crash (or two) I've been seeing the same problems mentioned above. That 
is:
1) I cannot restart that build in any way (icon, quicklaunch, command line) w/o 
a crash at startup
2) I cannot restart another build currently on my machine w/o a startup crash
3) I have just installed the current nightly (on another partition) and cannot 
open the browser without a startup crash.

In short - I can't *any* installation of Mozilla to work. It appears that we 
have some sort of profile corruption going on. If I restart from the command 
line with mozilla.exe -P and create a new profile that profile works.

I don't understand how the profile would get corrupted in connection with this 
particular crash, but it appears from the comment at the top that I am not the 
only one with this trouble. 

The machine I am crashing is my home machine. Lisa, can we get a QA machine to 
crash this (with my steps in #6) and see what is happening?

Comment 9

[Patty Mac]

Tom, yes, I could able to reproduce this problem follow your steps on additional 
comment #6, but not quite as same steps as yours:
(windows NT, branch build: 2002-05-01-08-1.0.0) Choose "CUSTOM" install.

1) Open http://homepage.mac.com/kgerich/pinstripe.html in a tab
2) Open another page in a new tab (I chose cnn.com)
3) In the first tab click on "install Pinstripe"
Then it crashes while the skin is installing.

Stack Trace:

nsNativeScrollbarFrame::Init [d:\builds\seamonkey\mozilla\layout\xul\base\src\
nsNativeScrollbarFrame.cpp, line 123] 
nsCSSFrameConstructor::InitAndRestoreFrame [d:\builds\seamonkey\mozilla\layout\
html\style\src\nsCSSFrameConstructor.cpp, line 6660] 
nsCSSFrameConstructor::ConstructXULFrame [d:\builds\seamonkey\mozilla\layout\
html\style\src\nsCSSFrameConstructor.cpp, line 5747] 
nsCSSFrameConstructor::ConstructFrameInternal [d:\builds\seamonkey\mozilla\
layout\html\style\src\nsCSSFrameConstructor.cpp, line 7283] 
nsCSSFrameConstructor::ConstructFrame [d:\builds\seamonkey\mozilla\layout\html\
style\src\nsCSSFrameConstructor.cpp, line 7167] 
nsCSSFrameConstructor::ProcessChildren [d:\builds\seamonkey\mozilla\layout\html\
style\src\nsCSSFrameConstructor.cpp, line 12113] 
nsCSSFrameConstructor::ConstructXULFrame [d:\builds\seamonkey\mozilla\layout\
html\style\src\nsCSSFrameConstructor.cpp, line 5769] 
nsCSSFrameConstructor::ConstructFrameInternal [d:\builds\seamonkey\mozilla\
layout\html\style\src\nsCSSFrameConstructor.cpp, line 7283] 
nsCSSFrameConstructor::ConstructFrame [d:\builds\seamonkey\mozilla\layout\html\
style\src\nsCSSFrameConstructor.cpp, line 7167] 
nsCSSFrameConstructor::ContentInserted [d:\builds\seamonkey\mozilla\layout\html\
style\src\nsCSSFrameConstructor.cpp, line 8872] 
nsCSSFrameConstructor::RecreateFramesForContent [d:\builds\seamonkey\mozilla\
layout\html\style\src\nsCSSFrameConstructor.cpp, line 11993] 
nsCSSFrameConstructor::ProcessRestyledFrames [d:\builds\seamonkey\mozilla\layout\
html\style\src\nsCSSFrameConstructor.cpp, line 10124] 
PresShell::ReconstructStyleData [d:\builds\seamonkey\mozilla\layout\html\base\
src\nsPresShell.cpp, line 5399] 
PresShell::StyleSheetRemoved [d:\builds\seamonkey\mozilla\layout\html\base\src\
nsPresShell.cpp, line 5428] 
nsDocument::UpdateStyleSheets [d:\builds\seamonkey\mozilla\content\base\src\
nsDocument.cpp, line 1454] 
nsChromeRegistry::RefreshWindow [d:\builds\seamonkey\mozilla\rdf\chrome\src\
nsChromeRegistry.cpp, line 1373] 
nsChromeRegistry::RefreshWindow [d:\builds\seamonkey\mozilla\rdf\chrome\src\
nsChromeRegistry.cpp, line 1247] 
nsChromeRegistry::RefreshSkins [d:\builds\seamonkey\mozilla\rdf\chrome\src\
nsChromeRegistry.cpp, line 1200] 
XPTC_InvokeByIndex [d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\
win32\xptcinvoke.cpp, line 106] 
EventHandler [d:\builds\seamonkey\mozilla\xpcom\proxy\src\nsProxyEvent.cpp, line 
567] 
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 597] 
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, 
line 530] 
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 
1078] 
nsAppShellService::Run [d:\builds\seamonkey\mozilla\xpfe\appshell\src\
nsAppShellService.cpp, line 309] 
main1 [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1434] 
main [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1769] 
WinMain [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1787] 
WinMainCRTStartup() 
KERNEL32.DLL + 0xd326 (0x77e8d326) 

Comment 10

[Kevin McCluskey (gone)]

Comment on attachment 80798 [details] [diff] [review]
patch to check for null scrollbar before using it.

r=[e-mail]

Comment 11

[Marc Attinasi]

The problem is the theme is specifying a native scrollbar, but there is no
native scrollbar factory except for on Mac and Cocoa.

We could (probably should) correctly handle the errors all the way up the
call-chain, but it involves a lot of code changes all over the place. The
null-check stops the crash on Windows, and presumably on the other platforms
that don't support native scrollbars.

The long-term 'right thing to do' is probably to support the native scrollbars
on all platforms, so we don't have OS-specific themes - ACK!

CC'in pinkerton, as I think he was involved with the changes for native control
on the Mac.

Comment 12

[Jaime Rodriguez, Jr.]

Thanks for the patch for this crasher, but it needs a sr= before we can add
adt1.0.0 keyword.

Comment 13

[Mike Pinkerton (not reading bugmail)]

oops my bad, forgot the null check there. thanks for the patch.

Comment 14

[Marc Attinasi]

SR has been requested (5/2)

Comment 15

[kinmoz]

Comment on attachment 80798 [details] [diff] [review]
patch to check for null scrollbar before using it.

sr=[e-mail]

You can avoid indenting everything if you just check:

if (!mScrollbar)
  return NS_ERROR_FAILURE;

after the assertion. Just mentioning it cause it seems to be the style used in
the file. Either way is ok with me.

Comment 16

[Jaime Rodriguez, Jr.]

 Let's get this one in after RC2. adt1.0.0- [adt2 RTM]

Comment 17

[Marc Attinasi]

Fix checked in on trunk. Waiting for branch approval.

ps. How was this ever a topcrash anyway? It seems pretty unlikely and rare.

Comment 18

[Kevin McCluskey (gone)]

Resolving as fixed. Please add fixed1.0.0 keyword when it is checked in on the
branch.

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 80798 [details] [diff] [review]
patch to check for null scrollbar before using it.

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 20

[scottputterman]

adding adt1.0.0+ for Mozilla 1.0 branch checkin.  Please get drivers approval
again since their approval is more than 3 days old.  After checking in, please
add the fixed1.0 keyword.

Comment 21

[Jay Patel [:jay]]

Updating summary with M1RC2 since this is still showing up in Mozilla 1.0 RC2
Talkback data.  Is this going to make it onto the branch for RC3?

Comment 22

[chris hofmann]

re=a=chofmann for the 1.0 branch and rc3

Comment 23

[Marc Attinasi]

Checked into 1.0.0 branch.

Comment 24

[greer]

Verified. This one is no longer on the branch or Trunk according to Talkback data.