185260 - crash on startup when using splash screen

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[cls]

If I startup any profile with the splashscreen enabled, then the browser crashes in the splash screen observer.  This is on BeOS 5.0.3.  I believe this is what's keeping the tinderbox orange as well.

loading symbols
segment violation occurred
BBitmap::Bounds(void) const:
Bounds__C7BBitmap:
+000c  ec18cd58:   *        1840d9    fld     0x00000018(%eax)
mozilla-bin:sc
   frame         retaddr
fd001658   80011d53  nsSplashScreenBeOS::Observe(nsISupports *, char const *, unsigned short const *) + 0000004b
fd0016ec   ec82247f  nsObserverService::NotifyObservers(nsISupports *, char const *, unsigned short const *) + 00000133
fd001768   80010ba7  main1(int, char **, nsISupports *) + 000007e7
fd001898   8001126d  main + 00000111
fd0018c4   8000bd15  _start + 00000061
mozilla-bin:

Comment 1

[cls]

Ok, so the quick fix is easy enough.  In nsSplashScreenBeOS::Observe, we assume
that the bitmap used by the splash screen is still loaded but the bitmap gets
unloaded when nsSplashScreenBeOS::Hide is called.   

So, there's 2 questions:
1) Why are we reloading/deleting the bitmap whenever Show()/Hide() are called?
2) Why are we hiding the splash screen before we're done calling all of the
notifiers of 'startup_user_notifcations'?
 

Comment 2

[cls]

Ok, ::Hide is being called from
http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsAppShellService.cpp#1122
through a long list of calls that leads back to InitializeProfileService(). 
Does this mean that the old feature of showing component registration & such in
the splash screen no longer works?

After some testing, I see the that splash screen disappears early if the profile
manager needs to come up.  If I only have one profile, then I can see the
"Creating first window..." notification in the splash screen before it disappears.

Comment 3

[Paul]

Attachment: Fix for Show/Hide of splash screen

Well, I just did a complete update/build, and did not get the crash.
However, this patch is in answer to your first question in Comment#1
I don't know the answer to question #2.

Initial registration of components do not show up in the splash screen anymore,
correct.  But, if a component is updated, its registration is seen in the
splash screen.

And Hide does not seem to be called, until just before the first window is
created.  So, the fact that Hide was destroying the window/bitmap, does not
really seem to matter, but, it won't do it anymore, with the above patch.

Comment 4

[Paul]

Attachment: Updated patch

Oops, the first patch didn't really address some of the problems brought up
here.  This one should, however.  

Either way, I still don't get a crash.

Comment 5

[cls]

Bryner told me on irc that the component registration messages only worked on
the Mac though I thought they were working on BeOS at one point.  That patch is
slightly confusing without any surrounding context but just moving the
LoadBitmap() call into the class constructor and the |delete bitmap;|  into the
destructor also fixed the problem.  And introduces a small new problem as it
appears that the destructor never gets called.  I got sidetracked trying to
figure out that new problem.

Comment 6

[Paul]

It was originally only working on the Mac.  Then, at one point, I made it work on BeOS too :)Actually, I don't think the patch introduces a new problem ... I don't think the destrcutorwas ever called, and it was never noticed before.  Is it because the Observer is neverreleased?

Comment 7

[cls]

I meant that moving the bitmap load/delete into the constructor/destructor
causes the new problem.  The observer is released at
http://lxr.mozilla.org/seamonkey/source/xpfe/bootstrap/nsAppRunner.cpp#1526 but
as the comment a few lines later states, the nsNativeApp object is owned by
appshell.  I'm guessing that the appshell never releases the
nativeApp/SplashScreen object.  Not sure if that's a feature or a bug.

Comment 8

[Paul]

My guess would be that it is a bug, since, after startup, when is it ever needed again?

Comment 9

[cls]

Comment on attachment 109356 [details] [diff] [review]
Updated patch

Ok, after applying the patch and looking at the unified diff, I see that you
did basically the same thing I did.  Which means that you'll hit the same bug
about the destructor not being called.
r=cls

Comment 10

[Paul]

if you notice, the mac port destroy's the dialog in the HideSplashScreen as well.  I wonder if it has the same problem, but was never caught before, since, they do nothingto check.  My guess is, that they do, and that it is probably a bug.

Comment 11

[cls]

Comment on attachment 109356 [details] [diff] [review]
Updated patch

The mac definitely doesn't have the original problem (crashing due to the
dialog disappearing).  I think that's because the mac exits ::Observe early if
it can't get a control handle for the dialog.  Since I think it's doubtful that
the second bug will actually be fixed, we should probably mimic the mac again.

Comment 12

[cls]

Attachment: follow the mac

Comment 13

[Paul]

Comment on attachment 109618 [details] [diff] [review]
follow the mac

looks good to me

Comment 14

[cls]

Patch has been checked in.