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)
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)
113.13 KB,
patch
|
dveditz
:
approval1.8.0.1+
dveditz
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•19 years ago
|
||
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)
Assignee | ||
Comment 2•19 years ago
|
||
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)
Assignee | ||
Updated•19 years ago
|
Attachment #193741 -
Flags: review?(benjamin)
Comment 3•19 years ago
|
||
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
Assignee | ||
Comment 4•19 years ago
|
||
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.
Assignee | ||
Updated•19 years ago
|
Attachment #193753 -
Attachment is obsolete: true
Attachment #193753 -
Flags: review?(benjamin)
Assignee | ||
Comment 5•19 years ago
|
||
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 6•19 years ago
|
||
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-
Assignee | ||
Comment 7•19 years ago
|
||
(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(...); }
Comment 8•19 years ago
|
||
> 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
Assignee | ||
Comment 9•19 years ago
|
||
(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.
Comment 10•19 years ago
|
||
> 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?
Comment 11•19 years ago
|
||
> 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.
Assignee | ||
Comment 12•19 years ago
|
||
(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.
Comment 13•19 years ago
|
||
> 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?
Assignee | ||
Comment 14•19 years ago
|
||
(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.
Assignee | ||
Comment 15•19 years ago
|
||
(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.
Assignee | ||
Comment 16•19 years ago
|
||
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...
Assignee | ||
Comment 17•19 years ago
|
||
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 18•19 years ago
|
||
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+
Assignee | ||
Comment 19•19 years ago
|
||
* 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
Comment 20•19 years ago
|
||
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+.
Assignee | ||
Comment 21•19 years ago
|
||
Checked in to trunk, with Gino's suggestions. ->FIXED
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Whiteboard: javaconnect-to-branch
Assignee | ||
Updated•19 years ago
|
Attachment #200458 -
Flags: approval1.8.1?
Attachment #200458 -
Flags: approval1.8.0.1?
Comment 22•19 years ago
|
||
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 23•19 years ago
|
||
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 24•19 years ago
|
||
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+
Comment 25•19 years ago
|
||
Fixed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
Keywords: fixed1.8.0.1,
fixed1.8.1
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•