23157 - YET ANOTHER SYSTEM DIRECTORY PROBLEM

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P3)

Description

[edburns]

Someone recently introduced a change that ignored the nsSpecialSystemDirectory
magic that dp and I put in back in september.  This change prevents mozilla
from being embedded into another app.

Comment 1

[edburns]

The original manifestation of this bug was in

mozilla/netwerk/protocol/res/src/nsResProtocolHandler.cpp

It used the current process directory to find resources, which is not correct
when the current process is not mozilla.exe.

Comment 2

[edburns]

I know it's unorthodox to do a top level checkin like this, but I've got
so many files in so many different directories, that I think it's the
best way.  I've pulled and clobber_all'd my tree and got

r=dp

on this checkin.

Here are the touched files:

M mozilla/embedding/browser/activex/src/control/MozillaBrowser.cpp
M mozilla/embedding/browser/activex/src/control/MozillaBrowser.h
M mozilla/js/src/xpconnect/shell/xpcshell.cpp
M mozilla/netwerk/protocol/res/src/nsResProtocolHandler.cpp
M mozilla/xpcom/build/nsXPComInit.cpp
M mozilla/xpcom/components/nsComponentManager.cpp
M mozilla/xpcom/components/nsIServiceManager.h
M mozilla/xpcom/components/nsServiceManager.cpp
M mozilla/xpcom/io/nsSpecialSystemDirectory.cpp
M mozilla/xpcom/io/nsSpecialSystemDirectory.h
M mozilla/xpcom/tests/TestBuffers.cpp
M mozilla/xpcom/tests/TestPipes.cpp
M mozilla/xpcom/tests/TestShutdown.cpp
M mozilla/xpcom/tests/windows/TestHelloXPLoop.cpp
M mozilla/xpcom/tools/registry/regExport.cpp
M mozilla/xpcom/tools/registry/regxpcom.cpp
M mozilla/xpinstall/stub/xpistub.cpp
M mozilla/webshell/embed/ActiveX/MozillaBrowser.cpp
M mozilla/webshell/embed/ActiveX/MozillaBrowser.h
M mozilla/webshell/tests/viewer/nsMacMain.cpp
M mozilla/webshell/tests/viewer/nsPhMain.cpp
M mozilla/webshell/tests/viewer/nsWinMain.cpp
M mozilla/webshell/tests/viewer/unix/gtk/nsGtkMain.cpp
M mozilla/xpfe/appshell/src/nsFileLocations.cpp
M mozilla/xpfe/bootstrap/nsAppRunner.cpp

The heart of this checkin is a change in the signature and symantics
of NS_InitXPCOM.

The new signature is

extern NS_COM nsresult
NS_InitXPCOM(nsIServiceManager* *result, nsFileSpec* binDirectory);

I filed a bug for this problem:

b=23157

The original manifestation of this bug was in
mozilla/netwerk/protocol/res/src/nsResProtocolHandler.cpp It used the
current process directory to find resources, which is not correct when
the current process is not mozilla.exe.

I have added a new type to nsSpecialSystemDirectory, Moz_BinDirectory,
and made nsResProtocolHandler use that value.

Comment 3

[edburns]

This was fixed when I put in the change to the arguments for nsInitXPCOM.

Comment 4

[edburns]

This was fixed when I put in the change to the arguments for nsInitXPCOM.

Comment 5

[Paul MacQuiddy]

marking verified

Comment 6

[edburns]

This is broken again in M16.  Why can't this stay fixed?

Comment 7

[edburns]

The problem this time is due to at least two things.  Here I talk about one of 
these things.  nsFileLocations.cpp.  

You can't use nsSpecialSystemDirectory::OS_CurrentProcessDirectory in an 
embedding context, because the CurrentProcessDirectory isn't anything to do 
with the mozilla bin directory.  Please use 
nsSpecialSystemDirectory::Moz_BinDirectory instead.

I'm about to attach a patch to fix this AGAIN.

Comment 8

[edburns]

Attachment: Fix for the second occurece of this bug, first iteration.

Comment 9

[edburns]

Adding CC.

Comment 10

[edburns]

Attachment: Fix for the second occurrence of this bug, first iteration.

Comment 11

[av (gone)]

OK, seems right thing to do. r=av.

Comment 12

[Chris Waterson]

For the 2nd patch, why not use nsCAutoString? Or at least PL_strncpy()? Or at 
least sizeof(path) instead of "2000", hard coded! ;-)

Comment 13

[Conrad Carlen]

Changing the relative location in this case to be from Moz_BinDirectory instead 
of OS_CurrentProcessDirectory is right. Problem is, there are other cases in 
nsIFileLocator that suffer from the same thing. I am working on replacing 
nsIFileLocator with nsIDirectoryServices which is flexible and a client can 
redefine locations. What you're patching is going away.

Comment 14

[edburns]

waterson: that's in the original code.  In the interest of minimal impact, I 
changed as little as I could.  If I was going to change those things, I'd do it 
in a separate checkin, since changing those things is not related to the bug.

Comment 15

[edburns]

conrad: I understand that what I'm patching is going away, but is it going away 
by tomorrow at noon?  I need to have my little patch in before nspr2 deadline.  
Can I please put it in?

Comment 16

[edburns]

Fix checked in.  M16 users must apply patches to fix this problem.

Comment 17

[Paul Wyskoczka]

Verified