123572 - Build fails to launch: Profile manager problem - Trunk [@ nsWebShellWindow::LoadContentAreas]

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Tracy Walker [:tracy]]

seen on linux commercial build 2002-02-05-08-trunk

-install and launch application
-from profile manager select a profile or create a new one (it doesn't matter)
-start the app.

the profile manager goes away as expected but the application never launches.  
no errors, nothing.  just silent failure.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Has anybody got any ideas (confirmed or not) where/what fails?

Comment 2

[Doug Turner (:dougt)]

I have a build from this morning (~7am pst) and I can not reproduce this
problem.  has there been any changes to the packaging?

Comment 3

[Kai Engert (:KaiE:)]

My own depend build shows this problem, too. Therefore I don't think it's packaging.

When Mozilla hangs during startup, I looked at the stacks of the running
threads, and I didn't see any clue.

I used NSPR_LOG_MODULES=nsFileIO:5 to see what happens.

The last logged event from that with today's build is:
  nsFileIO: opening local file
/home/kaie/current-trunk/mozilla-debug/dist/bin/res/forms.css for input (0)

An older build continues with 
  logically closing /home/kaie/098-mozilla/mozilla-debug/dist/bin/res/forms.css:
status=0
but today'd build never arrives there.

Comment 4

[Akkana Peck]

I see this too.  The browser and editor windows come up fine if I have an
existing profile, but anything going through the profile manager fails. 
Attempting to convert an NS4 profile fails to bring up a browser window;
clicking Manage Profiles crashes, in

#0  0x40966856 in nsWebShellWindow::LoadContentAreas (this=0x81861d8)
    at nsWebShellWindow.cpp:1454
#1  0x40965910 in nsWebShellWindow::OnStateChange (this=0x81861d8, 
    aProgress=0x82384cc, aRequest=0x8284440, aStateFlags=786448, aStatus=0)
    at nsWebShellWindow.cpp:1281
#2  0x4121c65f in nsDocLoaderImpl::FireOnStateChange (this=0x82384b8, 
    aProgress=0x82384cc, aRequest=0x8284440, aStateFlags=786448, aStatus=0)
    at nsDocLoader.cpp:1109
#3  0x4121b770 in nsDocLoaderImpl::doStopDocumentLoad (this=0x82384b8, 
    request=0x8284440, aStatus=0) at nsDocLoader.cpp:749
#4  0x4121b47a in nsDocLoaderImpl::DocLoaderIsEmpty (this=0x82384b8)
    at nsDocLoader.cpp:645
#5  0x4121b1e9 in nsDocLoaderImpl::OnStopRequest (this=0x82384b8, 
    aRequest=0x8315698, aCtxt=0x0, aStatus=0) at nsDocLoader.cpp:575

Comment 5

[Chris McAfee]

adding profile manager to summary

Comment 6

[Chris McAfee]

-> profile manager, new owner

Comment 7

[[not reading bugmail]]

DougT,

packaging changed I believe in bug 123399..

Comment 8

[Conrad Carlen (not reading bugmail)]

I have a linux build in progress. In the meanwhile, a question:
If you have only 1 profile or start with -P, what happens?

Comment 9

[Akkana Peck]

Backing out Adam Lock's changes to webShellWindow and webBrowserPersist
(probably only webShellWindow needed backing out but I did everything just in
case) cures the core dump and makes the profile manager window come up.

The profile manager window still hangs rather than bringing up a browser window
once you're finished converting or creating the new profile.  But if I hit ^C at
that point and restart, I can run.  It runs fine (even before the backout, in
answer to Conrad's question) if there's exactly one profile so the profile
manager  window doesn't have to come up.

Comment 10

[(not reading, please use [e-mail] instead)]

on win2k, I'm doing "-installer" and I get the crash, too.

webNav, line 1450 of nsWebShellWindow.cpp is null.

NTDLL! 77f9f9df()
nsDebug::Assertion(const char * 0x01d81b60 `string', const char * 0x01d81ba4 
`string', const char * 0x01d81bb4 `string', int 650) line 291 + 13 bytes
nsDebug::PreCondition(const char * 0x01d81b60 `string', const char * 0x01d81ba4 
`string', const char * 0x01d81bb4 `string', int 650) line 434 + 21 bytes
nsCOMPtr<nsIWebNavigation>::operator->() line 650 + 34 bytes
nsWebShellWindow::LoadContentAreas() line 1454 + 32 bytes
nsWebShellWindow::OnStateChange(nsWebShellWindow * const 0x005291d4, 
nsIWebProgress * 0x0052b2b4, nsIRequest * 0x02a35c00, int 786448, unsigned int 
0) line 1283
nsDocLoaderImpl::FireOnStateChange(nsIWebProgress * 0x0052b2b4, nsIRequest * 
0x02a35c00, int 786448, unsigned int 0) line 1110
nsDocLoaderImpl::doStopDocumentLoad(nsIRequest * 0x02a35c00, unsigned int 0) 
line 750
nsDocLoaderImpl::DocLoaderIsEmpty() line 647
nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x0052b2a4, nsIRequest * 
0x02fb6620, nsISupports * 0x00000000, unsigned int 0) line 578
nsLoadGroup::RemoveRequest(nsLoadGroup * const 0x0052b230, nsIRequest * 
0x02fb6620, nsISupports * 0x00000000, unsigned int 0) line 526 + 44 bytes
nsJARChannel::OnStopRequest(nsJARChannel * const 0x02fb6624, nsIRequest * 
0x02fb6154, nsISupports * 0x00000000, unsigned int 0) line 617
nsOnStopRequestEvent::HandleEvent() line 213
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x02fb7c54) line 116
PL_HandleEvent(PLEvent * 0x02fb7c54) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00529460) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x002500c4, unsigned int 49392, unsigned int 0, 
long 5411936) line 1071 + 9 bytes
USER32! 77e13eb0()
USER32! 77e1401a()
USER32! 77e192da()
nsXULWindow::ShowModal(nsXULWindow * const 0x00529170) line 283
nsWebShellWindow::ShowModal(nsWebShellWindow * const 0x00529170) line 1091
nsContentTreeOwner::ShowAsModal(nsContentTreeOwner * const 0x02a3236c) line 434
nsWindowWatcher::OpenWindowJS(nsWindowWatcher * const 0x004b1874, nsIDOMWindow 
* 0x00000000, const char * 0x004befa0, const char * 0x01e9a9fc, const char * 
0x01e9a584, int 1, unsigned int 1, long * 0x004bee80, nsIDOMWindow * * 
0x0012fb74) line 699
nsWindowWatcher::OpenWindow(nsWindowWatcher * const 0x004b1870, nsIDOMWindow * 
0x00000000, const char * 0x004befa0, const char * 0x01e9a9fc, const char * 
0x01e9a584, nsISupports * 0x004beeb0, nsIDOMWindow * * 0x0012fb74) line 450 + 
48 bytes
nsProfile::LoadDefaultProfileDir(nsCString & {...}, int 1) line 574 + 94 bytes
nsProfile::StartupWithArgs(nsProfile * const 0x004b1220, nsICmdLineService * 
0x004b4890, int 1) line 418 + 16 bytes
nsAppShellService::DoProfileStartup(nsAppShellService * const 0x004b4510, 
nsICmdLineService * 0x004b4890, int 1) line 243 + 31 bytes
InitializeProfileService(nsICmdLineService * 0x004b4890) line 936 + 31 bytes
main1(int 2, char * * 0x00444b90, nsISupports * 0x00000000) line 1238 + 14 bytes
main(int 2, char * * 0x00444b90) line 1625 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

Comment 11

[Kai Engert (:KaiE:)]

Conrad: Starting with -P indeed does the trick, I can start when I use option
"-P default".

Comment 12

[rpotts (gone)]

it looks like the old nsWebShellWindow::GetContentShellById(...) returned a
NS_FAILURE if the child could not be found.

The version in nsXULWindow::GetContentShellById() *always* returns NS_OK -- even
if the child treeitem cannot be found.

So, this explains the crash :-)  but it doesn't explain why the content area
hasn't been created when the 'document done' notification is fired...

-- rick

Comment 13

[Adam Lock]

Has my checkin been backed out, if so I'll work on an updated patch for bug
122861, otherwise I can apply some more pointer checks around this code.

I tried stepping into this code but I was unable to trigger it from Win32. I
have no idea why Linux would and Win32 wouldn't.

Comment 14

[Dan M]

No need to back out your change. Just change change line 1444 from

if (NS_SUCCEEDED(rv))

to

if (content)

to put it back to its original behaviour. By the way, it seems like the review
process should have caught this.

It's failing to find the content shell here because searchSpec from which it's
trying to pull the contentAreaID and contentURL is "manage=true", which doesn't
look like a content area ID. No telling how long that's been broken.

Comment 15

[Adam Lock]

Attachment: Patch

If my patch hasn't been backed out, someone r/sr this please.

Comment 16

[Kai Engert (:KaiE:)]

I applied your patch, and made a depend build in xpfe/appshell.
It does not fix the startup problem on my system.

Comment 17

[Conrad Carlen (not reading bugmail)]

I made that change and, while it doesn't crash on startup, it does hang after
dismissing the profile dialog.

Comment 18

[Dan M]

Odd. I applied my "patch" in comment 14, and it fixed the problem for me. Adam's
is equivalent. Though as I keep pointing out, the question of whether a failure
to return a value from an accessor method should return NS_OK or some failure
indication is a matter of semantics.

It's really not clear and the source uses both conventions throughout. Worrying
about the integer return value is almost never worth your time. I'd just check
the value's returned value; a pattern done throughout the codebase.

By the way I checked all the other places (all zero of them) where this method
is used to make sure that suddenly returning an error indication where one was
not returned before would be OK. r=me.

Comment 19

[Adam Lock]

To be clear, this patch is to fix the immediate crash not any subsequent
problems that it it might be hiding. I have a 3-4 day Linux build, I have tried
the patch and it corrects the issue with the -ProfileManager arg and brings up
the main window.

Comment 20

[Dan M]

Oh. Sorry. It does still hang after the profile manager window goes away. Still,
fixing the null dereference is important and should be checked in. I guess now
we have another problem.

Comment 21

[Radha on family leave (not reading bugmail)]

a=radha for adam's crash fix.  

Comment 22

[Radha on family leave (not reading bugmail)]

FWIW, I just can not reproduce this bug in my fresh W2K tree pulled few hrs ago.
I can manage profiles, create new profiles or start the browser with an existing
profile with out any problem. My linux build will complete shortly. Shall check
in it then. 

Comment 23

[Conrad Carlen (not reading bugmail)]

I would imagine the hang is linux only. Dan, is your hang on linux? 

Comment 24

[Dan M]

Yes, I'm just talking Linux. There is a crash from a null dereference. Adam's
patch clearly returns it to its previous behaviour and fixes the crash. Then
after hitting Start Mozilla in the profile manager window, the app just hangs.
I'm showing a failure to write data in some NSPR event posted and several
unprocessed events. Not quite sure why yet. (Which should not be taken to mean
that I'm clueful, but I am looking at it.)

Comment 25

[Matthias Versen [:Matti]]

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

Comment 26

[Simon Fraser [no longer active]]

Comment on attachment 67996 [details] [diff] [review]
Patch

sr=sfraser

Comment 27

[Adam Lock]

First patch is checked in. ETA for my Linux to update & rebuild is 1 hour.

Comment 28

[Kai Engert (:KaiE:)]

I have some hope, I tried backing out the change to nsFileChannel.cpp, and it
seems to work. Doing more tests.

Comment 29

[Chris McAfee]

adding rpotts, nsFileChannel.cpp change owner from the hook

Comment 30

[Kai Engert (:KaiE:)]

I feel confident now that I found the cause for the hang on Linux.
Re-applying the patch made the hang re-appear.

The patch in bug 122150 seems to have caused this problem.

Comment 31

[Kai Engert (:KaiE:)]

I suggest to do:
  cvs update -j1.116 -j1.115 mozilla/netwerk/protocol/file/src/nsFileChannel.cpp

Dan or Conrad, are you able to test that?

Comment 32

[Dan M]

Yes, that also fixes it for me. Nice, Kai!

Comment 33

[Radha on family leave (not reading bugmail)]

changing owner to rpotts

Comment 34

[rpotts (gone)]

Comment on attachment 67996 [details] [diff] [review]
Patch

hey adam,

did you check the other places where nsXULWindow::GetElementByID() is called to
make sure they correctly deal with NS_ERROR_FAILURE being returned ?

If so, then r=[e-mail]

Comment 35

[Conrad Carlen (not reading bugmail)]

Thank you Kai!

Comment 36

[Adam Lock]

Rick, nsWebShellWindow is the one and only place GetContentShellById is called from.

Comment 37

[rpotts (gone)]

ok... so i'm going to back out my change to nsFileChannel to get rid of this bug.

i still don't understand why this is linux-only though :-(  it REALLY should
have been all platforms  !!

-- rick

Comment 38

[rpotts (gone)]

i've just backed out my change to nsFileChannel.cpp.

Comment 39

[Adam Lock]

Confirming with fresh build, backing out nsFileChannel.cpp works for me too.

Comment 40

[Adam Lock]

Marking this bug FIXED.

Comment 41

[Jay Patel [:jay]]

Adding crash, topcrash keywords and Trunk [@ nsWebShellWindow::LoadContentAreas]
for future reference.

Comment 42

[Tracy Walker [:tracy]]

launch successful with commercial build 2002-02-07-07-trunk

marking verified