208961 - make NS_XPCOM_CURRENT_PROCESS_DIR obsolete

Status: NEW

(Core :: XPCOM, enhancement, P3)

Description

[Benjamin Smedberg]

http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsDirectoryServiceDefs.h has a
define for NS_XPCOM_CURRENT_PROCESS_DIR. It is unfortunately-named, however,
becaus it is an application-specific directory, passed as the *binDirectory
parameter to NS_InitXPCOM2. The only thing guaranteed to be in the directory is
the components/ folder, which has its own define, NS_XPCOM_COMPONENTS_DIR.

To avoid confusion, I think we should make NS_XPCOM_CURRENT_PROCESS_DIR
obsolete. We will still provide it in the default implementation, but won't use
it and will hide it from further use.

Most current users of NS_XPCOM_CURRENT_PROCESS_DIR shouldn't actually be using
it... code inside the GRE should be using NS_GRE_DIR and application code
(outside the GRE such as xpfe/) should be using NS_OS_CURRENT_PROCESS_DIR.

I'm not sure about the following cases:
http://lxr.mozilla.org/seamonkey/source/extensions/python/xpcom/src/loader/pyloader.cpp#87
http://lxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#3842
http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsInstall.cpp#121

I'd be happy to put a patch together if this seems like an acceptable solution.

Comment 1

[Doug Turner (:dougt)]

among other things, i remember that the installer made use of this.  Maybe there
is a better way (by providing a directory service provider)?

Comment 2

[Benjamin Smedberg]

ccing mhammond for the pyxpcom reference... would pyxpcom be installed as a "GRE
extension" (see bug 209439 for discussion) or in each application? In any case
NS_XPCOM_COMPONENTS_DIR seems like an non-meaningful location in terms of embedding.

Comment 3

[Doug Turner (:dougt)]

bsmedberg:

NS_XPCOM_COMPONENTS_DIR means the applications components directory.  This is
going to remain forever.  We also have NS_GRE_COMPONENT_DIR which means the
components directory in the GRE directory. 

In a discussion with you?  I thougth that we were debating the need for
NS_XPCOM_CURRENT_PROCESS_DIR? 

Comment 4

[Benjamin Smedberg]

dougt: yes, I mean NS_XPCOM_CURRENT_PROCESS_DIR (which seems to have no real
meaning). NS_XPCOM_COMPONENTS_DIR is meaningful and useful.

Comment 5

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 6

[Conrad Carlen (not reading bugmail)]

Comment on attachment 126308 [details] [diff] [review]
patch v.1

Whoa! - by changing all uses of NS_XPCOM_CURRENT_PROCESS_DIR to
NS_OS_CURRENT_PROCESS_DIR,  it breaks anybody passing a different directory to
NS_InitXPCOM2() or NS_InitEmbedding().

You can deprecate the name, but you can't change the location these consumers
are getting in some apps (those that override).

I think the name NS_XPCOM_CURRENT_PROCESS_DIR should be deprecated, but we need
a new synonym which represents the exact same location.

Comment 7

[Doug Turner (:dougt)]

I do not believe any of the client that I changed require the XPCOM_ overriden
value.  

I am not removing this entry.

Comment 8

[Benjamin Smedberg]

ccarlen: what "meaning" does NS_XPCOM_CURRENT_PROCESS_DIR really have? XPCOM
doesn't *use* any files in that directory, it just appends "components" to it to
find the default components directory. Callers all over the tree are using it to
find (for instance) plugins dirs, chrome dirs, etc... all of these usages are
misleading at least, if not incorrect.

What specific use would embedders have for NS_XPCOM_CURRENT_PROCESS_DIR? I would
even go one step further than dougt, and move the #define from
nsDirectoryServiceDefs.h to nsDirectoryService.cpp (this keeps binary
compatibility and frozenness, but doesn't perpetuate this "useless" directory
definition.)

Comment 9

[Conrad Carlen (not reading bugmail)]

Say:

1. My application is located in /Applications/MyApp
2. It passes the directory /Library/Frameworks/Gecko as the bin directory to
NS_InitXPCOM2

This change:

Index: netwerk/protocol/res/src/nsResProtocolHandler.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/protocol/res/src/nsResProtocolHandler.cpp,v
retrieving revision 1.55
diff -u -8 -r1.55 nsResProtocolHandler.cpp
--- netwerk/protocol/res/src/nsResProtocolHandler.cpp	8 Jan 2003 22:25:57 -0000	1.55
+++ netwerk/protocol/res/src/nsResProtocolHandler.cpp	23 Jun 2003 17:41:15 -0000
@@ -124,29 +124,32 @@
     mIOService = do_GetIOService(&rv);
     if (NS_FAILED(rv)) return rv;
 
     // set up initial mappings
     rv = SetSpecialDir("programdir", NS_OS_CURRENT_PROCESS_DIR);
     if (NS_FAILED(rv)) return rv;
 
     // make "res:///" == "resource:/"
-    rv = SetSpecialDir("", NS_XPCOM_CURRENT_PROCESS_DIR);
+    rv = SetSpecialDir("", NS_OS_CURRENT_PROCESS_DIR);

is now going to give a different location for the "res" folder, breaking me.

Comment 10

[Conrad Carlen (not reading bugmail)]

> what "meaning" does NS_XPCOM_CURRENT_PROCESS_DIR really have?

It means the XPCOM binDir, as defined by NS_InitXPCOM2(). Rather than changing
all uses of NS_XPCOM_CURRENT_PROCESS_DIR to NS_OS_CURRENT_PROCESS_DIR, and
breaking the semantics of NS_InitXPCOM2 and any embedding app making use of
that, why not rename NS_XPCOM_CURRENT_PROCESS_DIR and move the old name to the
deprecated section?

Comment 11

[Doug Turner (:dougt)]

conrad, people do want to know what the "real" OS directory is.  

Comment 12

[Conrad Carlen (not reading bugmail)]

doug: I didn't say get rid of, or even deprecate, NS_OS_CURRENT_PROCESS_DIR.

Comment 13

[Doug Turner (:dougt)]

I misunderstood this statement, i guess: "why not rename
NS_XPCOM_CURRENT_PROCESS_DIR and move the old name to the
deprecated section" ???

Comment 14

[Conrad Carlen (not reading bugmail)]

what I meant was:

keep NS_OS_CURRENT_PROCESS_DIR as it is
rename NS_XPCOM_CURRENT_PROCESS_DIR => NS_XPCOM_BIN_DIR (or something)
move NS_XPCOM_CURRENT_PROCESS_DIR to the deprecated section

Comment 15

[Doug Turner (:dougt)]

that might be a plan.  However, i do not think that NS_XPCOM_CURRENT_PROCESS_DIR
currently works.  There are a few places where files are referenced relative to
NS_OS_CURRENT_PROCESS_DIR.  We should probably fix those up if we agree to
continue to support the xpcom "bin" directory.

I think what I would like to do is this:

1. obsolete NS_XPCOM_CURRENT_PROCESS_DIR.
2. change all callers to use NS_OS_CURRENT_PROCESS_DIR.
3. set NS_OS_CURRENT_PROCESS_DIR to the "bin" directory passed to InitXPCOM2.

This would mean that clients can't get to the *real* bin directory, but that
might be a good thing.

Comment 16

[Benjamin Smedberg]

dougt: I like your last plan. The whole point of this bug  was that the codebase is using the two indiscriminately. We should settle on one, and the one passed to the "binDir" parameter is fine with me.

ccarlen: I'm planning on shipping the "res" directory shipped with the GRE to avoid the fact that currently each application has to ship it separately... see bug 179834

I would still like to propose that whichever one we *don't* use can be removed from nsDirectoryServiceDefs.h ... keep providing the string in nsDirectoryService.cpp to provide backwards compatibility.

Comment 17

[Conrad Carlen (not reading bugmail)]

The two directory locations in question, NS_XPCOM_CURRENT_PROCESS_DIR, and
NS_OS_CURRENT_PROCESS_DIR, are different - and both need to remain. If we have
callers using the two indiscriminately, let's fix that - not eliminate the
difference between the two. People get confused by the two because:
1. they both contain CURRENT_PROCESS.
2. the two are always equivalent for mozilla.
By renaming NS_XPCOM_CURRENT_PROCESS_DIR, the code is clearer, we don't break
anyone, and we don't lose the distinction between the two - which we need.

As far as why we need the two:

NS_OS_CURRENT_PROCESS_DIR is the directory containing the current process. There
are not nescesarily any gecko resources installed here.
NS_XPCOM_CURRENT_PROCESS_DIR is the root directory where components can find
gecko-related resources. 

Some examples from the diff:
1. nsScriptSecurityManager.cpp - it is looking for a file, systemSignature.jar,
which is clearly a part of gecko. There's no reason for this file to be in the
same dir as the current process. It should use NS_XPCOM_CURRENT_PROCESS_DIR.
2. nsAboutBloat.cpp - it is dumping out data about the process which is using
gecko. Clearly, this is not relevant to all processes using gecko, so this
should go in NS_OS_CURRENT_PROCESS_DIR.


  

Comment 18

[Benjamin Smedberg]

Gecko-specific files should use NS_GRE_DIR not NS_XPCOM_CURRENT_PROCESS_DIR

There is no guarantee that NS_XPCOM_CURRENT_PROCESS_DIR contains any gecko 
resources at all, and in GRE installations, it will *not* contain /res/ or 
/chrome/ files (yet).  And in shared installations of Gecko, I would hope that 
bloat-logs would be stored per-application, not in a shared directory.

Comment 19

[Conrad Carlen (not reading bugmail)]

> There is no guarantee that NS_XPCOM_CURRENT_PROCESS_DIR contains any gecko 
resources at all

In a non-GRE install, it has to.

The GRE has its keys, and these in question (NS_XPCOM_COMPONENTS_DIR and
NS_OS_COMPONENTS_DIR) had well-established meaning before the GRE existed. We
can't break things which used to work.

But, if we can say that, in a non GRE world, NS_GRE_DIR ==
NS_XPCOM_CURRENT_PROCESS_DIR, then NS_GRE_DIR is my renamed
NS_XPCOM_CURRENT_PROCESS_DIR :-)

Comment 20

[Benjamin Smedberg]

> But, if we can say that, in a non-GRE world, NS_GRE_DIR ==
> NS_XPCOM_CURRENT_PROCESS_DIR, then NS_GRE_DIR is my renamed

which is how it currently works... so are we agreed that
NS_XPCOM_CURRENT_PROCESS_DIR is no longer useful, and clients should either use
NS_OS_CURRENT_PROCESS_DIR or NS_GRE_DIR?

Comment 21

[Doug Turner (:dougt)]

mass reassigning to nobody.