Closed Bug 305815 Opened 19 years ago Closed 19 years ago

Build failure when building libXUL + Javaconnect

Categories

(Core Graveyard :: Java to XPCOM Bridge, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: jhpedemonte)

References

Details

(Keywords: fixed1.8.0.1, fixed1.8.1, Whiteboard: javaconnect-to-branch)

Attachments

(1 file, 5 obsolete files)

When building libXUL with Javaconnect enabled, I get a build failure in
extensions/java/xpcom/build.  GenerateJavaInterfaces is failing.  When
debugging, I noted that |NS_NewLocalFileOutputStream()| fails with
NS_ERROR_FACTORY_NOT_REGISTERED for NS_LOCALFILEOUTPUTSTREAM_CID.  Don't get
this build break when building the Suite + Javaconnect.

Looks to be caused by |NS_InitXPCOM3| change.
Attached patch patch (obsolete) — Splinter Review
I just needed to pass the appropriate static structures to |NS_InitXPCOM3| in
GenerateJavaInterfaces.  After it built correctly, I saw that I also needed to
update the |NS_InitEmbedding| call in nsJavaInterfaces.cpp.
Attachment #193741 - Flags: review?(benjamin)
Attached patch patch v1.1 (obsolete) — Splinter Review
The last patch broke the non-libXUL case, so I added the code to define the two
static parameters for non-libXUL.
Attachment #193741 - Attachment is obsolete: true
Attachment #193753 - Flags: review?(benjamin)
Attachment #193741 - Flags: review?(benjamin)
See also bug 302099. I'm not sure about this patch... I don't understand where
the static component list is coming from (it's not exported from libxul).
Depends on: 302099
The patch works since I'm an building on Linux, and |kPStaticModules| and
|kStaticModuleCount| are both exported on Linux.  But this probably wouldn't
work on Win32.  So it looks like I'll just wait until bug 302099 is fixed, and
use those APIs.
Attachment #193753 - Attachment is obsolete: true
Attachment #193753 - Flags: review?(benjamin)
Attached patch patch v2 (obsolete) — Splinter Review
Patch based on bsmedberg's changes in bug 302099.  Since libXUL is supposed to
be the new embedding base, I changed |GeckoEmbed| to |XUL| class, and changed
the C code to properly call |XRE_InitEmbedding| & |XRE_TermEmbedding|.	I also
had to change 'GenerateJavaInterfaces.cpp' to call |NS_InitXPCOM3|.
Attachment #197081 - Flags: review?(benjamin)
Comment on attachment 197081 [details] [diff] [review]
patch v2


>+  public static
>+  void initEmbedding(File aLibXULDirectory,
>+                     File aAppDirectory,
>+                     AppFileLocProvider aAppDirProvider)

Per email discussion, I don't think that Java clients ought to be responsible
for locating libxul. Javaconnect either ought to do it automatically (my
preferred solution), or should expose GRE_GetGREPathWithProperties so that the
proper gre-finding logic is exposed.

Also, where is javaconnect installed? In a perfect world it should be using the
standalone glue and therefore this method ought to call XPCOMGlueStartup.


>Index: tools/genifaces/GenerateJavaInterfaces.cpp

>-  rv = NS_InitXPCOM2(nsnull, nsnull, nsnull);
>+  nsStaticModuleInfo const *staticModules = nsnull;
>+  PRUint32 staticModuleCount = 0;
>+#ifdef MOZ_ENABLE_LIBXUL
>+  XRE_GetStaticComponents(&staticModules, &staticModuleCount);
>+#endif
>+
>+  rv = NS_InitXPCOM3(nsnull, nsnull, nsnull, staticModules, staticModuleCount);

This code snipped may be unnecessary, see bug 310105.
Attachment #197081 - Flags: review?(benjamin) → review-
(In reply to comment #6)
> Per email discussion, I don't think that Java clients ought to be responsible
> for locating libxul. Javaconnect either ought to do it automatically (my
> preferred solution), or should expose GRE_GetGREPathWithProperties so that the
> proper gre-finding logic is exposed.

I think it would be best to expose a |GRE.getGREPathWithProperties| method that
the Java embedder could use to find the installed libXUL, with whichever version
they require.

> Also, where is javaconnect installed?

Javaconnect (which consists of the javaxpcom.dll and mozjava.jar files) should
be installed in the bin directory of libXUL.  At least, that makes the most
sense to me.  Of course, there's nothing to prevent those 2 files from being
installed elsewhere, and having the code load the DLL from wherever it's located.

> In a perfect world it should be using the
> standalone glue and therefore this method ought to call XPCOMGlueStartup.

So the Java embedder would call |GRE.getGREPathWithProperties| to get the path
for the installed libXUL, then pass that path to |GRE.initEmbedding|, which
would call |XPCOMGlueStartup|, load the relevant DLLs, and call |XRE_InitEmbedding|.

Otherwise, if the Java embedder is providing their own libXUL (for whatever
reason), then they would just provide their own path to |GRE.initEmbedding|. 
How's that sound?

Of course, using |GRE_GetGREPathWithProperties| assumes that libXUL has been
properly installed.  At the moment, it seems that only the Mac builds come with
an installer.  The Win32 and Linux builds are just zips.  I assume these will
have proper installers by release time, correct?

So, to recap, I would have a class that looked like this:
   class GRE {
      public getGREPathWithProperties(...);
      public initEmbedding(...);
      public termEmbedding(...);
   }
> I think it would be best to expose a |GRE.getGREPathWithProperties| method that
> the Java embedder could use to find the installed libXUL, with whichever version
> they require.

Sure, but where would this code live? It can't live in libxul, because you need
to call the code to launch libxul. It would probably have to be installed with
the java application (much like it is a static library function for binaries).

> So the Java embedder would call |GRE.getGREPathWithProperties| to get the path
> for the installed libXUL, then pass that path to |GRE.initEmbedding|, which
> would call |XPCOMGlueStartup|, load the relevant DLLs, and call
|XRE_InitEmbedding|.

Yes. I'm not quite sure how Java dynamic loading works, so I'm not clear on how
the XRE_InitEmbedding signature would be called.

> an installer.  The Win32 and Linux builds are just zips.  I assume these will
> have proper installers by release time, correct?

1.8 release, probably not. You can register any GRE by running
  xulrunner.exe -register-global
or, if the user is non-privileged
  xulrunner -register-user
(In reply to comment #8)
> Sure, but where would this code live? It can't live in libxul, because you need
> to call the code to launch libxul.

Right.  There would be a 'stub' class that the Java application would link in. 
This class would have |getGREPathWithProperties|, which is really a native JNI
function; the JNI code would be linked with the XPCOM glue library.

It would also have an |initEmbedding| method, which would use the path returned
by |getGREPathWithProperties| to create a ClassLoader to load the "mozjava.jar"
file available with the libXUL install.  Then it can call the |initEmbedding|
method in "mozjava.jar"; this method takes care of loading the appropriate
libraries, and calling |XRE_InitEmbedding|.  I believe that this way,
|XPCOMGlueStartup| would not need to be called.

However, things are different for Eclipse.  Due to how plugins in Eclipse work,
there would need to be a plugin that encapsulates "mozjava.jar" and
"javaxpcom.dll" (the JNI lib), so that it could be used in an Eclipse app and
all the dependencies would resolve.  There could be one plugin per Mozilla
release, and the plugin would take care of versioning.
> all the dependencies would resolve.  There could be one plugin per Mozilla
> release, and the plugin would take care of versioning.

Why? What of these APIs are not frozen in such a way that we couldn't use one
plugin with only frozen APIs permanently?
> Right.  There would be a 'stub' class that the Java application would link in. 
> This class would have |getGREPathWithProperties|, which is really a native JNI
> function; the JNI code would be linked with the XPCOM glue library.

It is possible to code an equivalent getGREPathWithProperties method in pure
java, so that each Java app did not have to ship a binary. The GRE-finding code
is not a black-box mystery.
(In reply to comment #10)
> Why? What of these APIs are not frozen in such a way that we couldn't use one
> plugin with only frozen APIs permanently?

I've opened bugs for most of the non-frozen interfaces that are used by the
Eclipse code, although it looks like most of them will not be frozen for the 1.8
release.  I can look those up if you want that list.

Conversely, should C++ embedders be able to use non-frozen interfaces, but Java
be restricted to only frozen interfaces?  If we provide an Eclipse plugin per
release (with proper versioning), then we let the Eclipse embedder deal with
which interfaces and which releases to link to.

(In reply to comment #11)
> It is possible to code an equivalent getGREPathWithProperties method in pure
> java...

That's actually a good idea, since I don't need the rest of the XPCOMGlue.  I'll
do that.

> Conversely, should C++ embedders be able to use non-frozen interfaces, but Java
> be restricted to only frozen interfaces?  If we provide an Eclipse plugin per

Maybe I don't understand how the Java embedding stuff works. Why would the
eclipse glue need to know anything about any interfaces except for the ones it
actually uses? That is, why couldn't java code use non-frozen interfaces, but
not the actual eclipse plugin?
(In reply to comment #13)
> Why would the
> eclipse glue need to know anything about any interfaces except for the ones it
> actually uses?

Well, the eclipse glue itself doesn't need the other interfaces, but there might
be an eclipse app that uses this glue code but also uses some of the other
interfaces.  Also, this code (whether for a regular Java app or Eclipse app) is
meant to be as generic as possible.  It is not meant to be something that is
tailored specifically for the current browser embedding code in Eclipse.
(In reply to comment #11)
> It is possible to code an equivalent getGREPathWithProperties method in pure
> java...

Looking over |GRE_GetGREPathWithProperties|, it shouldn't be terribly hard to
code a Java version.  For Mac OS X, I can't do any of the |CFBundle| calls in
Java, but I don't think we need to worry about that since Java apps won't be
shipped in bundles.  For Unix, I will need to code a Java version of nsINIParser.

Windows is where it gets interesting.  There are no _public_ Java APIs for
getting at the registry. So we have two choices:
(1) From Java, run the "reg.exe" command line app and parse its output.  This
option is slow, but safe.
(2) Using Reflection APIs, get at the non-public
java.util.prefs.WindowsPreferences interface (forms the basis for the public
interface java.util.prefs.Preferences).  The WindowsPreferences interface has
several methods for dealing directly with the registry.  And while there seem to
be quite a few developers that are using this interface, this option is still
quite hacky.  But the performance will be better.

What do you think?  Personally, I'm leaning towards the first option.  This code
will only get run when trying to find the GRE/libXUL installation, so
performance isn't a major concern.

(In reply to comment #9)
> It would also have an |initEmbedding| method, which would use the path
> returned by |getGREPathWithProperties| to create a ClassLoader to load the
> "mozjava.jar" file available with the libXUL install.

No, turns out the ClassLoader idea was just dumb.  This is what we really want:
first, we'll have an MozillaInterfaces.jar that will contain all of the javafied
IDLs, as well as several embedding & helper classes.  This jar file need to ship
with whichever Java app wishes to embed Mozilla (for Eclipse, we'd provide a
plugin with this jar).  For clarity, we might want to actually have two jar
files here: one for frozen interfaces, and one for non-frozen.

Secondly, we'd have an "implementation" jar file that lived with each version of
libXUL/Mozilla.  This jar would have, for example, an implementation of
|initEmbedding| that loaded the necessary files; the stub |initEmbedding| in
MozillaInterfaces.jar would load the implementation jar and call its
|initEmbedding|.  And we'd also have the JNI library (javaxpcom.dll), which
would also be tied to a specific version of libXUL.
Attached file WinRegTest.java (obsolete) —
This test shows how to parse the Win32 Registry for the GRE values we care
about.	It uses "regedit.exe /e" to export the desired key to a file, which is
then parsed to find the installed GREs, their 'Version' & 'GreHome' values. 
Tested on WinXP and Win2k.

Can't use "reg.exe" as it is only available on WinXP.

bsmedberg, what do you think of this approach?	You probably know much more
about the Win32 Registry that I do...
Attached patch patch v3 (obsolete) — Splinter Review
Mostly complete patch, which splits the code into an Interfaces jar and an
implementation bundle.

The Interfaces jar consists mainly of the interfaces generated from IDLs, as
well as a few helper classes.  The main helper class is the 'Mozilla' singleton
class.	It provides the |getGREPathWithProperties| static method.  It also
implements the 'IGRE' and 'IXPCOM' interfaces, which define the methods to
init/shutdown embedding/XPCOM and to get the component and service managers. 
When calling |initEmbedding| or |initXPCOM|, the 'Mozilla' class loads
"javaconnect.jar" and the Mozilla shared libs from the specified GRE location.

The implementation bundle consists of "javaconnect.jar", which has impls of
'IGRE' and 'IXPCOM', and the JNI code library.

bsmedberg, when you've got a chance, can you tell me what you think?
Attachment #197081 - Attachment is obsolete: true
Attachment #199172 - Attachment is obsolete: true
Attachment #199883 - Flags: review?(benjamin)
Comment on attachment 199883 [details] [diff] [review]
patch v3

I did not do a detailed review, but the linking and embedding strategy seems a
lot more sane on the surface.
Attachment #199883 - Flags: review?(benjamin) → review+
Attached patch patch v3.1Splinter Review
* Changed |getGREPathWithProperties| to return actual GRE path, rather than
path to xpcom.dll, since Java developer has no use for xpcom.dll path.	Also,
only returns a GRE that supports Java embedding (looks for "javaconnect.jar").
* Replaced some custom classes with Sun classes.
* Only install MozillaInterfaces.jar in SDK.
* Added code to find XUL.framework if Java file is packaged as Mac OS X bundle.

* General code style cleanup.

I may just check this in to trunk so I don't have to keep messing around with
such a large patch.  I'm still looking for someone who knows Java well to code
review my Java classes.  I'll handle any fallout from that in other bugs.
Attachment #199883 - Attachment is obsolete: true
I reviewed the Java changes on this patch and it looks good. I communicated directly with Javier any necessary changes. I don't have EDIT privileges so I could not set r+.
Checked in to trunk, with Gino's suggestions. ->FIXED
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Whiteboard: javaconnect-to-branch
Depends on: 316090
Attachment #200458 - Flags: approval1.8.1?
Attachment #200458 - Flags: approval1.8.0.1?
Comment on attachment 200458 [details] [diff] [review]
patch v3.1

Please consider for 1.8.1 - 1.8.0.1 is for major security and crash issues only.
Attachment #200458 - Flags: approval1.8.0.1? → approval1.8.0.1-
Comment on attachment 200458 [details] [diff] [review]
patch v3.1

Re-requesting approval per drivers, this affects javaxpcom and xulrunner only.
Attachment #200458 - Flags: approval1.8.0.1- → approval1.8.0.1?
Comment on attachment 200458 [details] [diff] [review]
patch v3.1

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.
Attachment #200458 - Flags: approval1.8.1?
Attachment #200458 - Flags: approval1.8.1+
Attachment #200458 - Flags: approval1.8.0.1?
Attachment #200458 - Flags: approval1.8.0.1+
Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: