Closed Bug 64501 Opened 24 years ago Closed 24 years ago

Apps using NS_InitEmbedding cannot specify location of app registry

Categories

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

Product:
Core Graveyard
Component:
Embedding: APIs
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.8

People

(Reporter: ccarlen, Assigned: ccarlen)

Details

Attachments

(5 files)

makes reg location be provided by app filelocprovider
40.49 KB, patch
Details | Diff | Splinter Review
something I missed in previous patch
1.98 KB, patch
Details | Diff | Splinter Review
patch to add XP_STANDALONE checks
1.27 KB, patch
Details | Diff | Splinter Review
Adds chrome dirs & deps to xpcom module list
1.48 KB, patch
Details | Diff | Splinter Review
2nd patch for XPCOM_STANDALONE
1.46 KB, patch
Details | Diff | Splinter Review 
Currently the application registry location is provided by the directory service
provider in XPCOM. In order for an app not to use the same location as mozilla,
it can pass a different product name to NS_InitXPCOM2. NS_InitEmbedding, though,
 calls NS_InitXPCOM which does not use the product name param.
The patch here solves this by having the location of the app registry be
provided by the application file location provider. It also makes
nsAppFileLocationProvider.cpp get built by XPCOM. Before, it was built as a
separate static lib and linked into everything. An embedding app can provide
their own application file location provider by passing it to NS_InitEmbedding.
Having the default implementation built by XPCOM itself makes things easier
because things don't need to link with the appfilelocprovider static lib and
then register the provider with directory service.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.8
Updating QA Contact
QA Contact: jrgm → mdunn
+        NS_WITH_SERVICE(nsIProperties, directoryService,
NS_DIRECTORY_SERVICE_CONTRACTID, &rv);

+        if (NS_SUCCEEDED(rv)) {

+            rv = directoryService->Get(NS_XPCOM_CURRENT_PROCESS_DIR,
NS_GET_IID(nsIFile), getter_AddRefs(mMozBinDirectory));
+
+            if (NS_FAILED(rv)) {
+                rv =
directoryService->Get(NS_OS_CURRENT_PROCESS_DIR,NS_GET_IID(nsIFile),
getter_AddRefs(mMozBinDirectory));
+            }
+        }
+        if (NS_FAILED(rv))
+            return rv;

Obligatory nit-picking: if the NS_SUCCEEDED condition is false, there's no need
to test NS_FAILED, so move that last if inside the then block immediately above it.

I defer to cls and sfraser for the build stuff, which looks good enough for my
rubber stamp.  What worries me about this change is that it breaks XPCOM API and
ABI compatibility.  Is anyone (a Java VM plugin, perchance?) using NS_InitXPCOM2
in the wild?  Cc'ing buddies.

/be
> Is anyone (a Java VM plugin, perchance?) using NS_InitXPCOM2 in the wild?

NS_InitXPCOM2 was introduced fairly recently so as not to change NS_InitXPCOM. I
took that to mean NS_InitXPCOM was frozen but hopefully NS_InitXPCOM2 isn't.
Doug - any thoughts? I think you went through this.

ed please look at this bug.  I think that it effects you the most.

I am of the opinion that nothing has been frozen yet. I do not believe that I
have seen anything from mozilla.org or the xpcom owners stating that any
interface has been frozen.  Heck, we are only now doing api reviews.  The
history is that DP requested that I not change the init method of xpcom, but I
did this renaming reluctantly.

scc, as the owner of xpcom, what is the state of this function.  We have a
requirement to change the call signature for embedding purposes.  Should we just
deprecate NS_InitXPCOM* and create NS_XPCOM_Init()?  I would be happy with that.




Hi Doug, 

Thanks for remembering me!  

Yes, this is huge for us, since in our embedding scenario, the current working 
directory is that of Java.exe.  We currently do this:

NativeEventThread.cpp:        NS_InitEmbedding(pathFile, nsnull);

I'd like to just keep it simple, like that.  As long as I can pass an nsIFile 
representing the mozilla bin dir to NS_InitEmbedding, I'm happy.

Ed
[dougt: don't scare me with NS_XPCOM_Frobnosticate naming style (shades of Ada)
instead of NS_FrobnosticateXPCOM!]

ed: has Sun already shipped any plugin that depends on NS_InitXPCOM2?

/be
Ed, what I'm proposing doesn't change the signature of NS_InitEmbedding() at
all. You can continue to use it as is.
Brendan,

I just ran a find command on the java plugin source and the only occurrences of
XPCOM were in comments...like this:

// Changed by Mark : XPCOM headers have changed

...

// UGH, XPCOM has changed again

...

// DAMN IT!  If XPCOM changes again, I'm going to quit this project.

...

Actually I'm kidding.  But seriously, it's only in comments.  It looks like we
don't call any functions with XPCOM in the name.
Ok, then if the build stuff works and passes r= muster from cls and sfraser,
sr=brendan@mozilla.org.  Can you get an r= from someone on all the touched files?

/be
Doug, you already reviewed this a while back. Can you record that here?
ccarlen.  Got the cash.  here is the r=dougt.


cls and sfraser - Can you look at the build changes? As far as the build goes,
it just moves the files nsAppFileLocationProvider.h/.cpp and
nsAppDirectoryServiceDefs.h into xpcom/io. Only nsAppDirectoryServiceDefs.h
 gets exported. Once this is done, modules/appfilelocprovider does not need to
be built, nothing needs to link with that static lib, and
modules/appfilelocprovider can be removed.
Just for future reference, please don't add new subdirectories to
mozilla/modules -- we're trying to get rid of that.

/be
I've done full clobber builds of both mozilla and commercial with this on Win
and Linux and dep build on Mac (which always clobbers all of dist anyway). Found
another 2 makefiles i commercial tree which needed tweaking. Those are on
bugscape bug 3666.
Blocks: 64833
the build changes look fine. sr=sfraser on those.

cls, can you look at the build changes? This is ready to go except for your
review of that.
Sorry about the delay.  r=cls on the makefile changes.

Thanks. Now, about moving the 3 files: I need to move
nsAppFileLocationProvider.h/.cpp and nsAppDirectoryServiceDefs.h from
mozilla/modules/appfilelocprovider to mozilla/xpcom/io while preserving their
history. I'd like to move them to the new location, leaving them in the old
location and, make the check in. Then after a good build cycle, go remove
mozilla/modules/appfilelocprovider. That seems safest. What's the CVS magic to
do this? Can I do it from here?

cc: scc on move of files into xpcom/io.

/be
i can help you with this. provided you are *certain* about this move and scc 
gives consent, i'll go ahead and do the move.
actually, dougt is the xpcom/io module owner, according to owners.html...
The bug has r=dougt. It was discussion with him which led to this solution.
You need to remove the appreg library from the gtkmozembed library too since I
link it into there statically.
the specified three files have been moved to xpcom/io
Christopher (Blizzard) - which is the makefile for the gtkmozembed library?
Wouldn't this have been built when doing a clobber and then build_all on Linux?

Obviously, I didn't check out this patch as well I as should have.  Is it really
a good idea for xpcom to have a dependency upon chrome?

No, and I just discovered that because that. There was a routine not in the
diffs which needs to find the locale. There should be another way to do that. In
the meanwhile, it should be surrounded by XP_COM_STANDALONE and the default
locale returned in that case.
The dependency introduced by this file move is filed as bug 65907. I've got a
way to get rid of it - will put patch on new bug.
In current CVS 1/19 this broke the xpcom standalone build. It references 
nsIChromeRegistry.h which is not part of the standalone download.
The above patch (22978) surrounds the refs to chrome with XP_COM_STANDALONE
conditionals. A better patch will be put on bug 65907 which removes dependency
on chrome in all cases.

Can somebody review the patch to add XPCOM_STANDALONE #ifdefs? If this will fix
the standalone build, I'd like to get it in soon while working on getting rid of
the use of chrome here at all. 
The unix builds no longer use XPCOM_STANDALONE.  Users have to pull the extra
dependencies (usually just headers) so that we have the same functionality
between the standalone & non-standalone versions.  This patch adds the chrome &
necko dirs to the xpcom module.  The ifdef patch looks right but I don't have a
windows box to test on at the moment.

The windows standalone patch in not correct. The chrome CID also needs to be 
ifdef'd out since it comes from the chrome h file. With the additional ifdef it 
compiles. I don't know if it works.
Conrad, anyone: how about an updated patch that does work for XPCOM standalone?

/be
Hopefully this patch compiles in a real STANDALONE world. I have another patch
which removes all refs to chrome from here - whether STANDALONE or not. Once
that is a bit more tested, I'll post that.
Better patch which completely gets rid of dependency is on bug 65907.
Conrad, I suggest you checkin your patch to fix XPCOM_STANDALONE right away - 
with or without additional review (sr=jband). AFAIC the tree is busted. I just 
wrote a similar patch on my own before finding this bug so that I could get on 
with my work. I don't think others should have to do the same. Thanks. John.
Done. Thanks for sr=.
Marking as fixed. Since this bug had no visible symptoms, can be verified by
looking at:
http://lxr.mozilla.org/seamonkey/source/embedding/browser/powerplant/source/CBrowserApp.cp#191
This embedding app creates and supplies it own file location provider which
gives it its own file locations - including that of the registry.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Correction: Changing QA contact for the Embed API bugs to David Epstein.
QA Contact: mdunn → depstein
per Conrad's 1/24/01 comment, verifying. Checked code change in CBrowserApp.cp,
where the file location provider object is passed in NS_InitEmbedding():

CAppFileLocationProvider *fileLocProvider = new
CAppFileLocationProvider(kProgramName);
ThrowIfNil_(fileLocProvider);
rv = NS_InitEmbedding(appDir, fileLocProvider);

mfcEmbed uses winEmbedFileLocProvider object in NS_InitEmbedding().
Status: RESOLVED → VERIFIED
No longer blocks: 64833
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: