Provide helper method for embedding in AWT/Swing

REOPENED
Unassigned

Status

Core Graveyard
Java to XPCOM Bridge
REOPENED
11 years ago
3 years ago

People

(Reporter: jhp (no longer active), Unassigned)

Tracking

({fixed1.8.1.2})

Trunk
fixed1.8.1.2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

11 years ago
Currently, in order to embed XULRunner in a Java app, you need to call |nsIBaseWindow::InitWindow()|.  This function takes a native window handle, which tells Mozilla where to draw.  This isn't a problem for SWT, since it provides access to the window handle.

AWT (and Swing, which extends AWT) doesn't provide this access.  A developer would need to create a JNI function to pull the window handle from an AWT Canvas (as explained here: http://java.sun.com/j2se/1.3/docs/guide/awt/AWT_Native_Interface.html).  However, this is not ideal, since now the developer has to worry about creating a JNI library for each platform he wants to support.

A better alternative would be to roll this into JavaXPCOM, and expose it as a Java helper method.
Is that AWT technique frozen/reliable?
(Reporter)

Comment 2

11 years ago
Well, the APIs haven't changed since they were introduced in Java 1.3 and are still present in 1.5.0.  Plus, they were specifically introduced in order to replace another (non-public) API (see http://java.sun.com/j2se/1.5.0/docs/guide/awt/AWTChanges.html#drawingSurface).

So I would consider the APIs more or less frozen (none are marked as deprecated).  And judging from the forums posts, they seem to be pretty reliable.
(Reporter)

Comment 3

11 years ago
From http://forum.java.sun.com/thread.jspa?forumID=52&threadID=593759:

"Before getting HWND from a component you should call isLightweight() method. If it is FALSE the you can get HWND from this component otherwise go to Parent component and check isLightweight() == false, and so on. In my code before getting HWND I synchronize access to this component instance by getTreeLock()."
(Reporter)

Comment 4

11 years ago
Created attachment 245215 [details] [diff] [review]
Mac patch

This is the JavaXPCOM patch I used for https://bugs.eclipse.org/bugs/show_bug.cgi?id=154597, in order to allow the proper embedding of XULRunner on Mac OS X.  Note that the method in MacJawt.mm returns the 'cocoaViewRef', which is an NSView.  That means that this patch is only useful for a XULRunner built with Cocoa widgets (which is currently 1.9/trunk, or a custom built 1.8.x).

Comment 5

11 years ago
(In reply to comment #4):
Excellent!
Would there be a way to get some test build for XULRunner with Cocoa widgets?
I am not sure I would be able to do that, yet I could test the heck out of it :-)

Comment 6

11 years ago
My mistake. Just ignore that. Looks like we have already nighties for Mac now. 
http://ftp.mozilla.org/pub/mozilla.org/xulrunner/nightly/latest-trunk/
(Reporter)

Comment 7

11 years ago
Created attachment 247073 [details] [diff] [review]
Mac patch v1.1

This patch builds the AWT code only for the Mac, since that is the only platform I have tested (this patch is necessary for embedding XULRunner/Mac in SWT; see https://bugs.eclipse.org/bugs/show_bug.cgi?id=154597).  The other platforms require some extra code to dynamically find and load the jawt library.  For the Mac, I just link XUL against the JavaVM framework, since it is always available.

Benjamin, do you mind taking a look at the Makefile changes?  Do you have any problems with linking XUL/Mac to the JavaVM framework?
Attachment #245215 - Attachment is obsolete: true
Attachment #247073 - Flags: review?(benjamin)
Comment on attachment 247073 [details] [diff] [review]
Mac patch v1.1

JavaVM is a stable framework?
Attachment #247073 - Flags: review?(benjamin) → review+
(Reporter)

Comment 9

11 years ago
Yes, JavaVM is stable and available on all Mac OS X installs.
(Reporter)

Comment 10

11 years ago
Checked in to trunk. ->FIXED
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 11

11 years ago
Comment on attachment 247073 [details] [diff] [review]
Mac patch v1.1

Asking for 1.8.1.x branch approval.  This patch allows embedding XR in Java/SWT on Mac OS X.  XULRunner only - not part of default build.
Attachment #247073 - Flags: approval1.8.1.2?

Comment 12

11 years ago
Comment on attachment 247073 [details] [diff] [review]
Mac patch v1.1

Approved for 1.8 branch, a=jay for drivers.
Attachment #247073 - Flags: approval1.8.1.2? → approval1.8.1.2+
(Reporter)

Comment 13

11 years ago
Checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1.2
(Reporter)

Comment 14

11 years ago
Oops.  Shouldn't have closed this bug yet, since I only checked in the Mac OS X implementation.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

11 years ago
Duplicate of this bug: 368897
(Reporter)

Comment 16

11 years ago
Moving conversation from bug 368897:

(In reply to bug 368897 comment #4)
> Yes, getPeer() is deprecated. But it is not yet broken. At least, the return
> value of XComponentPeer.getWindow() and the content of
> jawt_X11DrawingSurfaceInfo.drawable are equal. I can attach my jni-enhanced
> test class if you don't believe me.
> What exactly does mozilla expect as window handle if not the Drawable?

(In reply to bug 368897 comment #5)
> jawt_X11DrawingSurfaceInfo.display seems to work slightly better as window
> handle.
> the jvm no longer crashes, and the webbrowser seems to work (causes network
> activity), but unfortunately, it is invisible!

(In reply to bug 368897 comment #6)
> Same result (no crash, but invisible) when I use the return value of
> gdk_window_foreign_new(myCanvassXID) as first parameter of
> nsIBaseWindow.initWindow().
> What might be the reason of the browser being invisible? Is it because the awt
> frame was not created by gtk? Is there some map or realize call missing?
> Perhaps some event is delivered to the wrong display?
> 
> Remember that the same code (using the hwnd as parentNativeWindow) runs on
> windows, so this issue must be somehow specific to gtk.


I don't know much about GTK, but looking through the code, it looks like |initWindow()| expects a |GtkWidget*|.  Not sure how that relates to the values in |jawt_X11DrawingSurfaceInfo|.  Maybe you are correct, and we would need to make another call in order to get the browser to display.

Comment 17

11 years ago
You are right, it has to be a GtkWidget*. Putting a GtkWindow or GtkPlug between the canvas and gecko and calling gtk_widget_show_all() on the window or plug indeed makes the webbrowser visible.
Unfortunately, no key events seem to reach gecko. When I use the mouse to select an html input element on a webpage, the caret in that input element starts to blink, but I can not enter anything.

Comment 18

11 years ago
Works after overiding
nsIEmbeddingSiteWindow.setFocus() with the following native code:
// x = baseWindow.getParentNativeWindow()
GtkWidget *gw = GTK_WIDGET(x); 
Display *display = GDK_WINDOW_XDISPLAY(gw->window);
Window focus = GDK_WINDOW_XWINDOW(gw->window);
gboolean retval;
g_signal_emit_by_name(gw, "focus_in_event", NULL, &retval);
XSetInputFocus(display, focus, RevertToParent, CurrentTime);

Comment 19

10 years ago
A little experimentation showed on OSX 
((CCanvas)getPeer()).getViewPtr();
returns the same value as:
mozilla.getNativeHandleFromAWT(this);

I haven't got either to actually display anthing yet on awt, but perhaps the ability to use the getViewPtr will make it easier for some people to do embedding without needing to upgrade or patch xulrunner.

Comment 20

10 years ago
Created attachment 258828 [details] [diff] [review]
patch for linux(tested) and win32(untested)

Comment 21

10 years ago
could not test the windows version because visual studio express refuses to install and i did not yet have time to try the mingw build instructions. can someone try/fix it?

Comment 22

10 years ago
This patch ( https://bugzilla.mozilla.org/attachment.cgi?id=258828 ) only retrieves the window handle (hWnd on windows, XID on linux). It does not include gtk glue code which is needed. I am still figuring out how to do this correctly. XAWT and gtk need proper synchronization with each other, otherwise you will get a lot of nasty xlib errors, deadlocks and crashes.

Comment 23

10 years ago
Created attachment 274882 [details] [diff] [review]
patch for linux(tested) and win32(tested)

added few fixes after testing on win32

Comment 24

10 years ago
Created attachment 274965 [details] [diff] [review]
Win32 Linux patch v1.2

added loading of the awt from an absolute path. This solves problems
1) not loading the library on win32 if current directory is not set to jre/bin
2) loading jawt.dll from correct directory if more jvms are installed on linux
Attachment #274882 - Attachment is obsolete: true

Comment 25

10 years ago
Created attachment 274966 [details] [diff] [review]
Win32 Linux patch v1.3

increased temp buffer size
Attachment #274965 - Attachment is obsolete: true
(Reporter)

Comment 26

10 years ago
Comment on attachment 274966 [details] [diff] [review]
Win32 Linux patch v1.3

>@@ -50,8 +50,19 @@
>+#if defined(XP_MACOSX) || defined(XP_UNIX) || defined(XP_WIN32)

Since these are the only supported platforms, there's no reason to have this #if statement.

>@@ -389,8 +400,24 @@
>-#ifdef XP_MACOSX
>-extern PRUint64 GetPlatformHandle(JAWT_DrawingSurfaceInfo* dsi);

This is still necessary, in order to link to the Mac OS X implementation, which resides in another file.

>+#if defined(XP_UNIX) || defined(XP_WIN32)

Be careful of using XP_UNIX, since Mac OS X also defines it.  If you only want non-Mac Unix/Linux systems, then do "#if defined(XP_UNIX) && !defined(XP_MACOSX)".

>@@ -399,10 +426,60 @@
>+#if defined(XP_MACOSX) || defined(XP_UNIX) || defined (XP_WIN32)

Same as above.

>+  //javaxpcom might not be initialized yet, get all classes
>+  //and MIDs we will need

In order to call |Mozilla.getNativeHandleFromAWT()|, JavaXPCOM must be initialized.  So you don't need to get systemClass.

>+  jstring jhkey = env->NewStringUTF("java.home");

Is this property always guaranteed to be set by the JVM, even if JAVA_HOME isn't set in the environment?

>+#ifdef XP_UNIX
>+  strcpy(awtlib[jhomeU8Len], "/bin/libjawt.so");
>+  void *libhandle = dlopen(awtlib, RTLD_LAZY);
>+  if (!libhandle) {
>+    NS_WARNING(dlerror());
>+    return 0;
>+  }
>+  _JAWT_GetAWT = (jboolean (*)(JNIEnv*, JAWT*)) dlsym(libhandle, "JAWT_GetAWT");
>+  if (!_JAWT_GetAWT) {
>+    NS_WARNING(dlerror());
>+    return 0;
>+  }
>+#else 

Break out all of this new AWT loading code into its own function. For Mac OS X, the function would be empty.

>@@ -421,6 +498,17 @@
>+  env->ReleaseStringUTFChars(jhome, jhomeU8);

You can move this after the memcpy, since |jhomeU8| is not used after that point.

>+  free(awtlib);

There are many return points in between the |memcpy| and the |free|, which will result in a leak.

> #else
>   NS_WARNING("getNativeHandleFromAWT JNI method not implemented");
> #endif

This is no longer necessary.
(Reporter)

Comment 27

10 years ago
It would also be nice if someone could create a self-contained testcase/snippet for embedding in AWT (at least for Win32/Mac OS X; Linux requires other fixes).

Comment 28

10 years ago
Created attachment 277308 [details] [diff] [review]
v1.4

cleaned the code, improved error handling, fixed mem leaks
I've tested this version on OSX, Win32, Linux.

An awt/swing widget using the code can be found here:
http://sourceforge.net/projects/mozswing

On Linux, OSX additional native patches are needed.
(OSX: running on AppKit thread, Linux: gtk helpers)
These are also commited there. Could them also be merged?
If yes, I'll post them as new bugs.
Attachment #274966 - Attachment is obsolete: true

Comment 29

10 years ago
Comment on attachment 277308 [details] [diff] [review]
v1.4

I think the patch is ready, so kindly asking for attention :-)
Attachment #277308 - Flags: review?
(Reporter)

Updated

10 years ago
Attachment #277308 - Attachment is patch: true
Attachment #277308 - Attachment mime type: application/octet-stream → text/plain
(Reporter)

Comment 30

10 years ago
Comment on attachment 277308 [details] [diff] [review]
v1.4

>@@ -50,9 +50,22 @@
>+#if defined(XP_UNIX)
>+#include <dlfcn.h>
>+#else
>+#include <windows.h>
>+#endif //XP_UNIX

I prefer to have UNIX be the final block in the if/else statement.  That way, any OS that is not explicitly named falls into the UNIX block.  (This should be fixed in other places in the patch, too.)

>@@ -399,31 +430,108 @@

You haven't addressed all of my issues from comment #26.  This method would look much cleaner if you moved the code for loading the JAWT library and method to its own method.

>+#if defined(XP_UNIX) || defined(XP_WIN32)

This covers the only supported/tested platforms, so it can be removed.

>+  strcpy(&awtlib[jhomeU8Len], rel);

This code would be much clearer by using "strncat()".

>+#ifdef XP_UNIX
>+  if (libhandle) dlclose(libhandle);
> #else
>-  NS_WARNING("getNativeHandleFromAWT JNI method not implemented");
>+  if (libhandle) FreeLibrary(libhandle);
> #endif

This code can be called multiple times.  We don't want to load/unload the JAWT lib every time.  Actually, doesn't Java have an issue with loading a native library multiple times, even if you do try to unload it?  Whether that's the case or not, the code probably shouldn't unload the lib until shutdown.

Please go back over comment #26 and make sure all those issues are addressed.
Attachment #277308 - Flags: review? → review-
(Reporter)

Comment 31

10 years ago
(In reply to comment #28)
> On Linux, OSX additional native patches are needed.

Please open separate bugs for the Linux and OSX patches.  We'll discuss the code there.

Comment 32

10 years ago
Created attachment 280496 [details] [diff] [review]
v1.5

updated wrt. comments
Attachment #277308 - Attachment is obsolete: true

Updated

10 years ago
Attachment #280496 - Flags: review?

Comment 33

10 years ago
> Please open separate bugs for the Linux and OSX patches.  We'll discuss the
> code there.

done. bugs: 395805, 395806

Comment 34

10 years ago
Hello Javier, I am just curious when or if you are planning on committing this fix.  
Attachment #280496 - Flags: review? → review?(jhpedemonte)

Updated

4 years ago
Assignee: jhpedemonte → nobody
Attachment #280496 - Flags: review?(jhpedemonte)
(Assignee)

Updated

3 years ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.