[9] Review Request for 6961123: setWMClass fails to null-terminate WM_CLASS string
Hi, I ran across some funny characters appearing in the application title under gnome shell and it turned out to be an old bug. Bug: https://bugs.openjdk.java.net/browse/JDK-6961123 Webrev: http://cr.openjdk.java.net/~omajid/webrevs/6961123-missing-null-wmclass/00/ The ICCCM [1] states: The WM_CLASS property (of type STRING without control characters) contains two consecutive null-terminated strings. ... Note that WM_CLASS strings are null-terminated and, thus, differ from the general conventions that STRING properties are null-separated. But AWT doesn't do that. It uses two null-separated strings for WM_CLASS. The webrev fixes that. Thanks, Omair [1] https://tronche.com/gui/x/icccm/sec-4.html -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] RFR: Allow using the system libjpeg
* Andrew Hughes gnu.and...@redhat.com [2014-05-22 16:10]: - Original Message - On 2014-05-22 02:49, Omair Majid wrote: Would you mind if I did this as a separate patch? I will need to fix other libraries too (libzip, and libgif). Ok, I thought the other ones were already in this form. In that case, leave it to me and I'll go through them as a whole. Thanks. The relevant bits are in spec.gmk.in: 565 USE_EXTERNAL_LIBJPEG:=@USE_EXTERNAL_LIBJPEG@ 566 USE_EXTERNAL_LIBGIF:=@USE_EXTERNAL_LIBGIF@ 567 USE_EXTERNAL_LIBZ:=@USE_EXTERNAL_LIBZ@ Further down in the file, other libraries have both CFLAGS and LIBS defined: 655 USE_EXTERNAL_LCMS:=@USE_EXTERNAL_LCMS@ 656 LCMS_CFLAGS:=@LCMS_CFLAGS@ 657 LCMS_LIBS:=@LCMS_LIBS@ 658 659 USE_EXTERNAL_LIBPNG:=@USE_EXTERNAL_LIBPNG@ 660 PNG_LIBS:=@PNG_LIBS@ 661 PNG_CFLAGS:=@PNG_CFLAGS@ Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] RFR: Allow using the system libjpeg
* Phil Race philip.r...@oracle.com [2014-05-22 16:02]: BTW .. I just realised I haven't seen a bug ID in this thread. Does one already exist ? No. I was going to create one before I push. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
AWT Dev RFR: 8043805: Allow using a system-installed libjpeg
* Anthony Petrov anthony.pet...@oracle.com [2014-05-22 16:48]: I think that it would be useful to have a bug id prior to sending a review request, so that a review thread for the bug can be easily found in the mailing archive. In the future, please do file a bug first and put its id in the subject line of your review requests. Apologies. I have filed https://bugs.openjdk.java.net/browse/JDK-8043805 and updated the subject of the thread. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] RFR: Allow using the system libjpeg
Hi, * Phil Race philip.r...@oracle.com [2014-05-20 18:34]: I only tried the default (not specifying the system libjpeg) but my builds all succeeded and SwingSet Java2D demo ran fine on Mac Windows with the closed builds (after generating a closed generated-configure.sh) so this all looks good to push. Thanks for testing! I will need to push that closed generated-configure.sh afterwards however so that closed builds don't break. Anything I need to do to ensure the pushes are co-ordinated correctly? Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] RFR: Allow using the system libjpeg
* Andrew Hughes gnu.and...@redhat.com [2014-05-21 12:22]: I'm not keen on the hardcoding of '-ljpeg' + LIBJPEG_LIBS := -ljpeg There's no pkg-config files for it. Any suggestions on how to get something generic? Could this not at least be conditionally assigned? (?=) It can still be overridden on the make command line: make LIBJPEG_LIBS=foo Is this not sufficient? Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] RFR: Allow using the system libjpeg
* Andrew Hughes gnu.and...@redhat.com [2014-05-21 20:23]: - Original Message - * Andrew Hughes gnu.and...@redhat.com [2014-05-21 12:22]: I'm not keen on the hardcoding of '-ljpeg' + LIBJPEG_LIBS := -ljpeg There's no pkg-config files for it. Any suggestions on how to get something generic? I know there isn't at present. I still think it's better to be consistent and set the values in configure, even if at present they have to be hardcoded defaults. Would you mind if I did this as a separate patch? I will need to fix other libraries too (libzip, and libgif). The make documentation is a little unclear here, and makes it sound like it will always be set if := or = is used. Indeed, it is unclear. But I discovered through trial and error that given a Makefile like: FOO=-v all: gcc ${FOO} these two forms are different: $ FOO=bar make $ make FOO=bar The first one sets the environment variable FOO, and the Makefile overrides it (it runs `gcc -v`). The second passes a command line argument that overrides the value in the Makefile (it runs `gcc foo`). Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using the system libjpeg
Hi, Updated webrevs: http://cr.openjdk.java.net/~omajid/webrevs/system-libjpeg/01/ http://cr.openjdk.java.net/~omajid/webrevs/system-libjpeg/01.jdk/ * Anthony Petrov anthony.pet...@oracle.com [2014-05-19 14:42]: make/lib/Awt2dLibraries.gmk 1236 LIBJPEG_CFLAGS := $(JDK_TOPDIR)/src/share/native/sun/awt/image/jpeg Since these are compiler flags, I suppose this should read -Ipath, not just path. Yes, it should. I wonder how my compiler accepted that. Did you build with --with-jpeg=bundled ? Can the resulting JDK read jpeg files? I did. I tested both combinations and made sure both the splash screen and javax.swing.ImageIcon could display jpegs files with the first webrev I posted. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
AWT Dev RFR: Allow using the system libjpeg
Hi, Webrevs are at: http://cr.openjdk.java.net/~omajid/webrevs/system-libjpeg/00 http://cr.openjdk.java.net/~omajid/webrevs/system-libjpeg/00.jdk This patch modifies OpenJDK to allow building and running using the system copy of libjpeg. This follows my previous changes for lcms and a lot of the decisions made on that patch apply here. This is the last of my system-library patches :) Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [OpenJDK 2D-Dev] Dont use libjpeg-internal macros in splashscreen
* Anthony Petrov anthony.pet...@oracle.com [2014-05-05 10:12]: The fix looks good to me, too. Thanks. I have pushed it: http://hg.openjdk.java.net/jdk9/client/jdk/rev/8b9bbe3b2e83 Regards, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Magnus Ihse Bursie magnus.ihse.bur...@oracle.com [2014-02-20 05:35]: From my point of view, you can go either way. The changes are mostly in the build code, except for an include statement. But if the AWT folks feel more confident that way, sure, you can go via client. Pushed: http://hg.openjdk.java.net/jdk9/client/rev/bfc1c131e540 http://hg.openjdk.java.net/jdk9/client/jdk/rev/5e503831b142 Just a note: Since you are updating the generated-configure.sh, someone with access to the closed sources will need to push a follow-up (the same bug number can be used) with an updated version of the closed generated-configure.sh. If you go in via the client forest, I'd prefer if someone from client did that. Please let me know if there is anything I can do to help. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Magnus Ihse Bursie magnus.ihse.bur...@oracle.com [2014-02-19 06:59]: On 2014-02-17 10:16, Erik Joelsson wrote: At least to me this looks good, but better let Magnus and Andrew have their say too. Looks good to me! Thanks for the reviews, everyone! I have filed https://bugs.openjdk.java.net/browse/JDK-8035341 to track the issue. Can someone please point me some docs that explains what I need to do to to this bug once I have pushed the fix? Should I be pushing this to jdk9/dev ? (Or to jdk9/client?) Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Erik Joelsson erik.joels...@oracle.com [2014-02-17 04:16]: At least to me this looks good, but better let Magnus and Andrew have their say too. Feedback from an AWT expert would be appreciated too! Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Sergey Bylokhov sergey.bylok...@oracle.com [2014-02-18 12:35]: cc 2d alias also. The fix looks good. I am curious who will document all new options in README-builds.html? One thing I have always loved about autotools is self documentation. With this patch, for example, ./configure --help gives me the new option and the description but also gives me the new _CFLAGS and _LIBS options added. I don't mind updating README-builds.html, but I think that might be over-documenting (if there's such a thing) :) Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Andrew Hughes gnu.and...@redhat.com [2014-02-13 23:59]: As I said in the previous e-mail, the minimum I'd be happy with is if the current patch was updated so settings weren't being hardcoded into the makefiles. Passing them from configure would be sufficient for now, then it can be replaced by PKG_CONFIG. How does this updated patch look: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/03/ http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/03-jdk/ Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
Hi, * Magnus Ihse Bursie magnus.ihse.bur...@oracle.com [2014-02-12 17:49]: On 2014-02-12 18:47, Andrew Hughes wrote: To be extremely clear: Andrew, do you object to bringing Omairs patch, as it is, into OpenJDK? Yes. Okay then. I'll put a mental note to revisit libpng when cleaning up libraries.m4. Assuming that I have the cycles to clean up some stuff, what's the minimal I could get away with (to enable system libpng support)? Are we in agreement that it should to the following: --with-libpng=bundled | system | some path and if system is selected, it should first check with pkg-config, and only if that fails, try hard-coded values. In any case it should try to compile a short code to see that it works properly. (This is not needed for bundled; that is assumed to be working). Would it be sufficient if I updated the patch to use PGK_CHECK_MODULES and simply fail if that doesn't work? I will be happy to implement something more complex if you can sketch it out. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Andrew Hughes gnu.and...@redhat.com [2014-02-04 19:26]: On 2014-02-03 18:43, Omair Majid wrote: The following webrevs modify the build system to allow building against the system-installed copy of libpng as well as using the bundled copy of libpng ROOT: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01/ JDK: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01-jdk/ A new configure option `--with-libpng` is introduced which defaults to `bundled` but can be set to `system` to use the system-installed copy of libpng. In this respect, why does this not just use AC_ARG_ENABLE as there are only two options here (system libpng or bundled libpng)? Also, this patch hardcodes the libpng cflags and ldflags when these should be obtained from pkg-config. This is how these values are obtained in IcedTea and using this patch would thus be a regression. The answer to both of these is the same: I followed how the existing code handles zlib. If this needs fixing, then it needs to be fixed in the other cases (zlib and giflib) too. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
AWT Dev RFR: Allow using a system installed libpng
Hi, The following webrevs modify the build system to allow building against the system-installed copy of libpng as well as using the bundled copy of libpng ROOT: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01/ JDK: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01-jdk/ A new configure option `--with-libpng` is introduced which defaults to `bundled` but can be set to `system` to use the system-installed copy of libpng. This patch (with minor differences) has been used in Fedora builds for a while now [1]. Thanks, Omair [1] http://pkgs.fedoraproject.org/cgit/java-1.8.0-openjdk.git/commit/system-libpng.patch?id=ff9254ee03799a98689692ee9ae060b3a31af213 -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Allow using a system installed libpng
* Magnus Ihse Bursie magnus.ihse.bur...@oracle.com [2014-02-03 14:42]: On 2014-02-03 18:43, Omair Majid wrote: Hi, The following webrevs modify the build system to allow building against the system-installed copy of libpng as well as using the bundled copy of libpng ROOT: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01/ JDK: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/01-jdk/ A new configure option `--with-libpng` is introduced which defaults to `bundled` but can be set to `system` to use the system-installed copy of libpng. Thank you for the patch! Looks good to me (but I'm no formal reviewer), apart from this line in libraries.m4: with_libpng=${DEFAULT_libpng} which should be DEFAULT_LIBPNG, I assume, otherwise the default case will not work. Whoops. Updated webrevs: root: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/02/ jdk: http://cr.openjdk.java.net/~omajid/webrevs/system-libpng/02-jdk/ Due to this, I'd like to ask you to please double check that --with-libpng=system | bundled | invalid-value | empty, and no flag at all, all work as expected. I re-ran all these 4 options. system builds with system, bundled and empty build a bundled copy and invalid-value fails during configure. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Support GNOME Shell as mutter
On 09/16/2013 10:02 AM, Anthony Petrov wrote: Hi Omair, Thanks for the patch. Looks fine to me. I've just filed a bug for this fix: 8024863: X11: Support GNOME Shell as mutter Thanks for the review! Do you have commit rights for the awt-gate, or do you want me to push the fix for you? I am a jdk8 committer (but not a member of the awt group). I believe I should be able to push it to jdk8/awt-gate. I will let you if I run into any issues. Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Support GNOME Shell as mutter
Hi Alexander, On 09/16/2013 10:27 AM, Alexander Zvegintsev wrote: The patch looks fine to me. Thanks for the review! I wanted to credit you in the Reviewed-by line of the commit message, but I cant find you in the census: http://openjdk.java.net/census Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Support GNOME Shell as mutter
Hi Anthony, On 09/16/2013 03:32 PM, Anthony Petrov wrote: Alexander doesn't have an OpenJDK id yet. But still, having an additional review never hurts. Agreed, the more reviewers the better :) Please feel free to push the fix listing only me as a reviewer, and let me know if you need any assistance with this. Pushed: http://hg.openjdk.java.net/jdk8/awt/jdk/rev/8530456e0091 Thanks once again! Cheers, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev RFR: Support GNOME Shell as mutter
On 09/08/2013 04:11 PM, Omair Majid wrote: Hi, With recent versions of GNOME shell, GNOME shell sets the _NET_WM_NAME to GNOME Shell instead of mutter. It's still mutter that's running, but with a different _NET_WM_NAME. The following webrev makes OpenJDK compatible with these newer versions: http://cr.openjdk.java.net/~omajid/webrevs/new-gnome-shell-compat/00/ Anyone have any comments or suggestions? Is this patch okay for jdk8? Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
AWT Dev RFR: Support GNOME Shell as mutter
Hi, With recent versions of GNOME shell, GNOME shell sets the _NET_WM_NAME to GNOME Shell instead of mutter. It's still mutter that's running, but with a different _NET_WM_NAME. The following webrev makes OpenJDK compatible with these newer versions: http://cr.openjdk.java.net/~omajid/webrevs/new-gnome-shell-compat/00/ Thanks, Omair -- PGP Key: 66484681 (http://pgp.mit.edu/) Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
Re: AWT Dev [8] Review request for 7190587 Open source and jtreg'ify JAWT regression test
Hi, On 08/27/2012 12:29 PM, Konstantin Shefov wrote: Please review a fix for the issue: 7190587 Open source and jtreg'ify JAWT regression test Test was modified in to be run with jtreg. The webrev is http://cr.openjdk.java.net/~kshefov/7190587/webrev.00/ Is this the test case I was referred to earlier [1] ? I notice that it exports LD_LIBRARY_PATH before executing the jawt-using program. This would have missed a regression (bug 7190813 [2]). Can we avoid exporting LD_LIBRARY_PATH? Thanks, Omair [1] http://mail.openjdk.java.net/pipermail/build-dev/2012-August/006612.html [2] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7190813
Re: AWT Dev [8] Review request for 7190587 Open source and jtreg'ify JAWT regression test
Hi Konstantin, On 08/29/2012 12:43 PM, Konstantin Shefov wrote: I can remove path to libjawt.so from LD_LIBRARY_PATH, but I have to leave working dir in it, i.e. export LD_LIBRARY_PATH=. Without it test cannot find libmylib.so in working directory. Oh, of course. I somehow thought the test used an explicit path to libmylib.so. I would be perfectly happy if libjawt.so was removed from LD_LIBRARY_PATH. Thanks, Omair
Re: AWT Dev JAWT Breakage in OpenJDK 7/8
Hi Anthony, On 08/07/2012 09:18 AM, Anthony Petrov wrote: I've also read the discussion that took place on build-dev@ regarding this issue, and it seems to me that adding an RPATH entry to launchers is the right solution for this issue. We may want to also add a comment there and state that this entry is added specifically to address the jawt use case. Perhaps it makes sense to also add a comment somewhere in a jawt makefile to state that we rely on this RPATH entry in the launcher. This way we can ensure that even if the jawt library is moved around, the relevant RPATH entry gets updated accordingly as well. Thanks for you suggestions! I am going to post a patch based on your suggestions to build-dev. Thanks, Omair
Re: AWT Dev JAWT Breakage in OpenJDK 7/8
On 07/25/2012 05:33 AM, Artem Ananiev wrote: Hi, Omair, I don't remember the exact reason for LD_LIBRARY_PATH changes, but it's very unlikely they can be reverted. Your proposed patch will probably solve the problem (I haven't checked, though), but it has major side-effect: increased startup time. I am not sure how significant that would be. It's a tiny library after all - only 0.5KB on my machine. That's why I vote for just updating JAWT documentation to include System.loadLibrary() call in the application code. While I agree that the docs should be updated, I dont think it's sufficient: existing applications that rely on this are currently broken. If the applications can not be modified (third-party or read-only) then this (for all intents and purposes) breaks the promised binary compatibility of OpenJDK :( Thanks, Omair
AWT Dev JAWT Breakage in OpenJDK 7/8
Hi, I recently discovered that a number of applications making use of JAWT [1] are broken on OpenJDK 7 and 8, while they were working with OpenJDK6. The docs for JAWT state [1]: The final step the most interesting one is to write the native rendering method, with an interface that conforms to the header file that javah generated, and build it as a standard shared library (called myRenderingLib in the above example) by linking it, for the Solaris operating environment, against the jre/lib/sparc/libjawt.so library. In other words, JAWT expects third-party native libraries to be linked against libjawt.so. However, nowhere on that page does it say that something like System.loadLibrary(jawt) is needed. In practice, this means that most existing applications rely on libjawt.so being magically found and loaded by the JVM. In OpenJDK6, LD_LIBRARY_PATH was set to a value such that the linker could resolve libjawt.so when a library depending on it was loaded. LD_LIBRARY_PATH was removed for OpenJDK7 [2]. So a number of existing applications fail to work with OpenJDK7 because they can not load libjawt.so. Some applications have been patched to load libjawt.so manually [3], but others haven't (or cant be). The most backwards-compatible fix is to undo the effects of the LD_LIBRARY_PATH change - by setting RUNPATH of the launcher, for example. The build-dev folks suggested that I only do that as a last resort [4]. An alternate fix seems to be to always load libjawt when awt starts up. There might be a small window of time where a third-party library fails to link against libjawt.so but this can happen only if awt has not started up. I would think such a case would not work with OpenJDK6 too since libjawt.so will not be able to resolve the dependency on libxawt.so. libxawt.so is only loaded after awt is started up, and can not be resolved through LD_LIBRARY_PATH. The attached patches for OpenJDK8 and OpenJDK7 attempt this approach. Does anyone have any thoughts or comments? I suspect something like this might be needed for Windows too, but I don't have access to a windows box to verify it. As an aside, should the docs for JAWT [1] be updated? Thanks, Omair [1] http://download.java.net/jdk8/docs/technotes/guides/awt/AWT_Native_Interface.html [2] https://blogs.oracle.com/darcy/entry/purging_ld_library_path [3] http://java-game-lib.svn.sourceforge.net/viewvc/java-game-lib/trunk/LWJGL/src/java/org/lwjgl/LinuxSysImplementation.java?r1=3418r2=3604 [4] http://mail.openjdk.java.net/pipermail/build-dev/2012-July/006492.html diff --git a/src/solaris/native/sun/awt/awt_LoadLibrary.c b/src/solaris/native/sun/awt/awt_LoadLibrary.c --- a/src/solaris/native/sun/awt/awt_LoadLibrary.c +++ b/src/solaris/native/sun/awt/awt_LoadLibrary.c @@ -148,6 +148,12 @@ (Ljava/lang/String;)V, JNU_NewStringPlatform(env, buf)); awtHandle = dlopen(buf, RTLD_LAZY | RTLD_GLOBAL); +/* + * Third-party applications expect libjawt to be available + */ +JNU_CallStaticMethodByName(env, NULL, java/lang/System, loadLibrary, + (Ljava/lang/String;)V, + JNU_NewStringPlatform(env, jawt)); } return JNI_VERSION_1_2; diff --git a/src/solaris/native/sun/awt/awt_LoadLibrary.c b/src/solaris/native/sun/awt/awt_LoadLibrary.c --- a/src/solaris/native/sun/awt/awt_LoadLibrary.c +++ b/src/solaris/native/sun/awt/awt_LoadLibrary.c @@ -187,6 +187,13 @@ awtHandle = dlopen(buf, RTLD_LAZY | RTLD_GLOBAL); +/* + * Third-party applications expect libjawt to be available + */ +JNU_CallStaticMethodByName(env, NULL, java/lang/System, loadLibrary, + (Ljava/lang/String;)V, + JNU_NewStringPlatform(env, jawt)); + return JNI_VERSION_1_2; }
Re: AWT Dev [7u6] Review request for 7043963: AWT workaround missing for Mutter.
On 06/06/2012 11:15 AM, Artem Ananiev wrote: Hi, Anthony, Omair, since the fix is a direct backport from JDK8, it should be fine. I've got a question, though. Since Mutter is a replacement for Metacity in GNOME 3, is it possible that it shares some code with Metacity and Actually, Mutter is a newer version of metacity (just like jdk8 is a newer jdk6). If you look through the mutter git log, you will see lots of METACITY_X_Y tags. The documentation in mutter for new developers also refers to metacity a lot. therefore some of the current isMetacity() and getWMID() == METACITY_WM check should be complemented with isMutter() and getWMID() == MUTTER_WM? For example, there is one of the checks in XWM.supportsExtendedState(). Probably. But I am not sure if mutter and metacity are exactly the same in all the ways that we care about. Specifically, mutter supports resizing window vertically or horizontally (see attached screenshot). I am not sure if this feature was present in newer versions of metacity or is completely new to mutter. Cheers, Omair attachment: resize-in-single-direction.png
Re: AWT Dev Add mutter as a window manager.
On 04/20/2012 11:01 AM, Artem Ananiev wrote: On 4/11/2012 11:21 PM, Anthony Petrov wrote: Hi Omair and Artem, On 4/11/2012 9:20 PM, Omair Majid wrote: PS. Perhaps it also makes sense to rewrite that comment in the XDecoratedPeer to replace the word bug with something saying that this is implemented according to the ICCCM specification with a reference to the paragraph 4.1.5 of it? Agreed. I would like to go so far as to make this the default, and add other window managers (which are deviating from ICCCM) as exceptions. I am afraid of introducing regressions, though. I wholeheartedly support this idea in theory. But it seems scary in practice. We would need to run all automatic and manual AWT regression tests on at least all major WMs (Metacity, Kwin, Compiz, and (sic!) CDE - as long as we support Solaris CDE desktops, not sure if this is relevant to JDK 8 though) to ensure no regressions arise. Note that we have to test this with quite old versions of the WMs, e.g. Metacity from Gnome 2.6 (or is it 2.4?) - this is what Solaris 10 has to offer, etc. And ideally we would also want to test on all those forgotten/rare creatures like SawFish, Motif, Enlightenment, etc. This looks like a lot of work for such a simple fix. However, your current fix looks pretty safe and is fully consistent with our current XAWT code. Artem, what's your opinion? Sorry for such a long delay from my side... Mario absolutely correctly wrote in his email: To be honest, I'm not that happy with the fix, but not because of Omair's patch, which is very fine given the current state of things, but because we keep adding this to each and every new WM we find, although fixing AWT in this regard will be probably more work than adding another exception to the list and an issue for perennial backward compatibility... and I feel exactly the same: the fix is fine, but the current AWT approach to workaround WM bugs isn't. Please, consider this as my approval :) Thanks. I have pushed the fix: http://hg.openjdk.java.net/jdk8/awt-gate/jdk/rev/79df0a4a6573 Cheers, Omair
Re: AWT Dev Add mutter as a window manager.
Hi Mario, On 04/11/2012 07:39 AM, Mario Torre wrote: I also agree with Omair. To be honest, I'm not that happy with the fix, but not because of Omair's patch, which is very fine given the current state of things, but because we keep adding this to each and every new WM we find, although fixing AWT in this regard will be probably more work than adding another exception to the list and an issue for perennial backward compatibility... Anyway, I just wanted to give Omair my +1 to this patch. Thanks for reviewing the patch. Unfortunately, since you are not listed as a reviewer for jdk8 [1], I couldnt list you as a reviewer in the changeset. Sorry about that. Thanks, Omair [1] http://openjdk.java.net/census#neugens
Re: AWT Dev Add mutter as a window manager.
On 04/11/2012 06:43 AM, Anthony Petrov wrote: Hi Omair, The analysis below sounds reasonable to me, and as I've already mentioned I'm OK with your fix. Let's hear what Artem says though. Yes, I was trying to convince Artem all this time ;) PS. Perhaps it also makes sense to rewrite that comment in the XDecoratedPeer to replace the word bug with something saying that this is implemented according to the ICCCM specification with a reference to the paragraph 4.1.5 of it? Agreed. I would like to go so far as to make this the default, and add other window managers (which are deviating from ICCCM) as exceptions. I am afraid of introducing regressions, though. Thanks, Omair
Re: AWT Dev Add mutter as a window manager.
Hi Anthony, On 04/09/2012 09:22 AM, Anthony Petrov wrote: Mutter is the direct descendant of Metacity, so there's nothing wrong with it inheriting some inconvenient behavior of its parent. Given that Mutter is the standard WM for Gnome 3.0. I'm fine with the fix. A comment regarding the test: 61 frame.pack(); 62 frame.setSize(500, 500); What's the point of this operations sequence? You should either simply set the desired size, or rely on the pack() alone if the automatically calculated size satisfies you. It just doesn't make sense to do both. You are right. I have removed the pack() line. Also, the @bug line in the test should mention a real CR for this issue, I think it is 7043963. Ah, I didn't have this number. Fixed now. Updated webrev at: http://cr.openjdk.java.net/~omajid/mutter-support/02/ Given Artem's comments, though, I not sure what to do here. Thanks, Omair
Re: AWT Dev Add mutter as a window manager.
Hi Artem, On 04/09/2012 07:10 AM, Artem Ananiev wrote: I really hope we can drop most of the ancient WMs listed in the XWM class (MOTIF, OPENLOOK, CDE, SAWFISH, etc) in JDK8. We know AWT/Swing works fine on the modern WMs that conform to ICCCM and NET standards, and I don't see any reasons to have (and add more!) workarounds for non-conformant window managers. I spent a little bit of time reading up on ICCCM (and X11), and here's a summary of my findings. I am not familiar with this, so it could be that I am making a mistake. Please correct me if I am wrong. The problematic case (that the reproducer shows) is that when we maximize a window by double clicking on the title bar under mutter, java does not detect that the window has moved/changed size. ICCCM 4.1.5 [1] states: If the window manager decides to respond to a ConfigureRequest request by: ... snip ... - Resizing the window or changing its border width (regardless of whether the window was also moved or restacked). A client that has selected for StructureNotify events will receive a real ConfigureNotify event. Note that the coordinates in this event are relative to the parent, which may not be the root if the window has been reparented. The coordinates will reflect the actual border width of the window (which the window manager may have changed). The TranslateCoordinates request can be used to convert the coordinates if required. The general rule is that coordinates in real ConfigureNotify events are in the parent's space; in synthetic events, they are in the root space. Advice to Implementors Clients cannot distinguish between the case where a top-level window is resized and moved from the case where the window is resized but not moved, since a real ConfigureNotify event will be received in both cases. Clients that are concerned with keeping track of the absolute position of a top-level window should keep a piece of state indicating whether they are certain of its position. Upon receipt of a real ConfigureNotify event on the top-level window, the client should note that the position is unknown. Upon receipt of a synthetic ConfigureNotify event, the client should note the position as known, using the position in this event. If the client receives a KeyPress , KeyRelease , ButtonPress , ButtonRelease , MotionNotify , EnterNotify , or LeaveNotify event on the window (or on any descendant), the client can deduce the top-level window's position from the difference between the (event-x, event-y) and (root-x, root-y) coordinates in these events. Only when the position is unknown does the client need to use the TranslateCoordinates request to find the position of a top-level window. To me, this says that when a window has been resized (by, say, double clicking on the title bar), a real ConfigureNotify event will be sent. The implementation (awt, in this case) should query for the coordinates relative to the root and use them. This is pretty much exactly what the CDE/MWM/Metacity/Sawfish bug currently does. It seems like this should be the correct default behaviour (for all window managers, including mutter). What do you think? Thanks, Omair [1] http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.5
Re: AWT Dev Add mutter as a window manager.
On 05/10/2011 12:07 PM, Phil Race wrote: So the repos aren't open any more. Only approved fixes can go in. There was email on this a couple of weeks ago. I have updated Denis' patch to apply to jdk8. Webrev available at: http://cr.openjdk.java.net/~omajid/mutter-support/01/ Thoughts? Cheers, Omair
Re: AWT Dev [RFC] Tray icons for applications are not displayed in the GNOME notification bar.
On 11/16/2011 07:25 AM, Anthony Petrov wrote: Hello Danesh and OpenJDK6 team, On 11/16/2011 1:13 AM, Danesh Dadachanji wrote: On 11/11/11 07:28 AM, Anthony Petrov wrote: Thank you Danesh for your contribution! I've just pushed the fix to JDK8 AWT repository. My pleasure, thank you for the review and your feedback! I'd like to request this be backported to JDK6 and JDK7 as well. The same webrev should work (some lines are offset in 6 but the patch applies fine). Let me know if you prefer a separate one for each though. I've just filed a SubCR for 7103610 to have this fix back-ported to a 7 update release. As with any such back-porting, this may take some time. Please stay tuned. I have push rights to jdk7u-dev and I would be happy to do the push on Danesh's behalf. Cheers, Omair
Re: AWT Dev Can't compile (external) code against sun.awt.event package?
On 09/28/2011 05:05 PM, Roman Kennke wrote: Hi, I am trying to compile the Cacio project against vanilla OpenJDK7. One of the classes references IgnorePaintEvent. This class cannot be compiled: [INFO] Compilation failure /home/roman/src/hg/caciocavallo/shared/src/main/java/sun/awt/peer/cacio/CacioComponentPeer.java:[70,20] error: package sun.awt.event does not exist /home/roman/src/hg/caciocavallo/shared/src/main/java/sun/awt/peer/cacio/CacioComponentPeer.java:[726,20] error: cannot find symbol It used to work when I explicitely set the bootclasspath to an OpenJDK build that I just built before (snippet from ant script: javac srcdir=${dir.src.shared.classes} destdir=${dir.build.shared.classes} bootclasspath=${openjdk}/classes debug=${compile.debug} / However, now that JDK7 is out I was thinking it would be great to compile against a vanilla OpenJDK7 rt.jar. I verified that the class is in that rt.jar and that I am really using the JDK7 javac. I wonder why javac cannot find it. Cacio code uses a lot of other internal classes (sun.awt.AppContext, sun.awt.SunToolkit, etc etc) and it only issues warnings about those. I think I must be missing something... Try setting the property ignore.symbol.file to true [1][2]. HTH, Omair [1] http://bugs.sun.com/view_bug.do?bug_id=6544224 [2] http://andrew-haley.livejournal.com/695.html
Re: AWT Dev Request for review: x11 - ensure non zero heights and widths
Hi Anthony, On 09/28/2010 10:00 AM, Anthony Petrov wrote: Thanks Omair. Now the problem is clear. And your last fix looks great. I've just filed Thanks for reviewing the patch and pointing out the problems. 6987945: XDecoratedPeer shouldn't allow to resize a frame to zero size so that you could push your fix. Thanks! Thanks for taking care of the bug id. I have pushed the changeset as 6994facc6a8b. Cheers, Omair -- best regards, Anthony On 09/27/2010 08:36 PM, Omair Majid wrote: Hi Anthony, On 09/24/2010 04:27 PM, Anthony Petrov wrote: Hi Omair, On 9/24/2010 6:12 PM, Omair Majid wrote: While trying out a Java program, I noticed that some BadValue X errors were being reported (I had to set -Dsun.awt.noisyerrorhandler=True first). The webrev at http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.00/ attempts to fix the issue. Updated webrev at http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.01/ Please make sure you put the true-branches of the if statements into {} blocks. I noticed that XBaseWindow is already doing a check like this using the constant MIN_SIZE and the math library. So I switched to that instead. I will keep this style guideline in mind for any future patches. Do I understand correctly that we're fixing an issue with top-level windows only? If so, then I suggest to not touch the code in the XBasWindow, but rather tweak the bounds in the XWindowPeer where we add them to the create params map. (and perhaps in the XDecoratedPeer - need to check if their logic of initializing the params is separate or not.) Yes, I can only create this issue with top-level windows. The issue does not happen on initialization (checkParams in XBaseWindow takes care of that). It only happens on resizing after initialization. checkShellRectSize() in XDecoratedPeer is supposed to correct the size, but it misses out the corner case where widths and heights are exactly zero. XWindowPeer (and other X peers) call xSetBounds which already takes care of the zero case. For the sake of consistency, I have made sure that all the places which check for the widht or height = 0 case use Math.max(MIN_SIZE, width) just like XBaseWindow.checkParams(). The problem is that there are some cases where the jdk is creating or resizing windows and setting widths and/or heights to 0. The man page What are the exact cases? The attached program, when run with -Dsun.awt.noisyerrorhandler=True prints out a BadValue error caused by X_ConfigureWindow. Before http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6678385, this would have caused the process to crash. Currently the resize in the test program does not do anything; with the fix applied it would set it to the smallest size possible. Thanks, Omair -- best regards, Anthony for XResizeWindow states If either width or height is zero, a BadValue error results. I looked at the source code for xorg-xserver and it appears that specifying a height or width of 0 for pretty much any X call will cause a BadValue error. The patch takes an approach similar to what is currently being done in XBaseWindow.xSetBounds(). It checks heights and widths, setting them to 1 if needed. Any comments? Cheers, Omair
Re: AWT Dev Request for review: x11 - ensure non zero heights and widths
Hi Anthony, On 09/24/2010 04:27 PM, Anthony Petrov wrote: Hi Omair, On 9/24/2010 6:12 PM, Omair Majid wrote: While trying out a Java program, I noticed that some BadValue X errors were being reported (I had to set -Dsun.awt.noisyerrorhandler=True first). The webrev at http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.00/ attempts to fix the issue. Updated webrev at http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.01/ Please make sure you put the true-branches of the if statements into {} blocks. I noticed that XBaseWindow is already doing a check like this using the constant MIN_SIZE and the math library. So I switched to that instead. I will keep this style guideline in mind for any future patches. Do I understand correctly that we're fixing an issue with top-level windows only? If so, then I suggest to not touch the code in the XBasWindow, but rather tweak the bounds in the XWindowPeer where we add them to the create params map. (and perhaps in the XDecoratedPeer - need to check if their logic of initializing the params is separate or not.) Yes, I can only create this issue with top-level windows. The issue does not happen on initialization (checkParams in XBaseWindow takes care of that). It only happens on resizing after initialization. checkShellRectSize() in XDecoratedPeer is supposed to correct the size, but it misses out the corner case where widths and heights are exactly zero. XWindowPeer (and other X peers) call xSetBounds which already takes care of the zero case. For the sake of consistency, I have made sure that all the places which check for the widht or height = 0 case use Math.max(MIN_SIZE, width) just like XBaseWindow.checkParams(). The problem is that there are some cases where the jdk is creating or resizing windows and setting widths and/or heights to 0. The man page What are the exact cases? The attached program, when run with -Dsun.awt.noisyerrorhandler=True prints out a BadValue error caused by X_ConfigureWindow. Before http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6678385, this would have caused the process to crash. Currently the resize in the test program does not do anything; with the fix applied it would set it to the smallest size possible. Thanks, Omair -- best regards, Anthony for XResizeWindow states If either width or height is zero, a BadValue error results. I looked at the source code for xorg-xserver and it appears that specifying a height or width of 0 for pretty much any X call will cause a BadValue error. The patch takes an approach similar to what is currently being done in XBaseWindow.xSetBounds(). It checks heights and widths, setting them to 1 if needed. Any comments? Cheers, Omair import java.awt.Dimension; import java.awt.Frame; public class X11ZeroValuesFrameTest { public static void main(String[] args) throws InterruptedException { Frame frame = new Frame(Test); frame.setSize(new Dimension(100,100)); frame.show(); Thread.sleep(1000); frame.setSize(new Dimension(5,3)); } } diff -r d21c9804c512 src/solaris/classes/sun/awt/X11/XBaseWindow.java --- a/src/solaris/classes/sun/awt/X11/XBaseWindow.java Mon Sep 27 17:38:57 2010 +0400 +++ b/src/solaris/classes/sun/awt/X11/XBaseWindow.java Mon Sep 27 11:04:22 2010 -0400 @@ -705,12 +705,8 @@ throw new IllegalStateException(Attempt to resize uncreated window); } insLog.fine(Setting bounds on + this + to ( + x + , + y + ), + width + x + height); -if (width = 0) { -width = 1; -} -if (height = 0) { -height = 1; -} +width = Math.max(MIN_SIZE, width); +height = Math.max(MIN_SIZE, height); XToolkit.awtLock(); try { XlibWrapper.XMoveResizeWindow(XToolkit.getDisplay(), getWindow(), x,y,width,height); diff -r d21c9804c512 src/solaris/classes/sun/awt/X11/XDecoratedPeer.java --- a/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java Mon Sep 27 17:38:57 2010 +0400 +++ b/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java Mon Sep 27 11:04:22 2010 -0400 @@ -763,12 +763,8 @@ } private void checkShellRectSize(Rectangle shellRect) { -if (shellRect.width 0) { -shellRect.width = 1; -} -if (shellRect.height 0) { -shellRect.height = 1; -} +shellRect.width = Math.max(MIN_SIZE, shellRect.width); +shellRect.height = Math.max(MIN_SIZE, shellRect.height); } private void checkShellRectPos(Rectangle shellRect) {
AWT Dev Request for review: x11 - ensure non zero heights and widths
Hi, While trying out a Java program, I noticed that some BadValue X errors were being reported (I had to set -Dsun.awt.noisyerrorhandler=True first). The webrev at http://cr.openjdk.java.net/~omajid/webrevs/x11-bad-widths-heights/webrev.00/ attempts to fix the issue. The problem is that there are some cases where the jdk is creating or resizing windows and setting widths and/or heights to 0. The man page for XResizeWindow states If either width or height is zero, a BadValue error results. I looked at the source code for xorg-xserver and it appears that specifying a height or width of 0 for pretty much any X call will cause a BadValue error. The patch takes an approach similar to what is currently being done in XBaseWindow.xSetBounds(). It checks heights and widths, setting them to 1 if needed. Any comments? Cheers, Omair
Re: AWT Dev Request for review: Crash on XIM server restart
Hi Artem, On 09/23/2010 05:33 AM, Artem Ananiev wrote: I'm not an i18n expert, just would like to understand the problem and the fix. Naoto Sato was kind enough to review the patch: http://mail.openjdk.java.net/pipermail/i18n-dev/2010-September/000221.html Could you provide the stack trace of the crash, please? Sure. I am attaching the GtkJTextFieldTest.java (a reproducer) and the backtrace I obtained. Here are the steps I have to take to reproduce the crash: 1. Start an XIM server (I use ibus in gnome) 2. Run the reproducer and type something in one of the JTextFields 3. Restart the XIM server (for ibus, this requires right clicking on the icon in the system tray and selecting restart). 4. Switch back to the java program and try to type something again. As I understand, the reason of calling getX11InputMethodData is that you need to destroy pX11IMData and set the corresponding field in Java to null, right? Yeah. The code already checks (in getX11InputMethodData) if the XIM server has stopped. It can also automatically reconnect to the XIM server too. the problem is that if the XIM server is stopped and then restarted, X calls OpenXIMCallback which set XIMim to non null, skipping the check in getX11IMInputMethodData. Since pX11IMData does not correspond to the new X11im, the program crashes. I posted an updated cr with comments clarifying the operation. http://cr.openjdk.java.net/~omajid/webrevs/crash-on-xim-server-restart/webrev.01/ Cheers, Omair On 9/16/2010 11:58 PM, Omair Majid wrote: Hi, While looking at the bug filed at https://bugzilla.redhat.com/show_bug.cgi?id=572147, I noticed that Java applications crash if IBus (an X input server) is restarted. The webrev is at http://cr.openjdk.java.net/~omajid/webrevs/crash-on-xim-server-restart/webrev.00/ The code in awt_InputMethod.c guards against the XIM server going away, but does not guard against it appearing again. When the XIM server is restarted, the X server can send async requests to call OpenXIMCallback in Java automatically: (gdb) bt #0 OpenXIMCallback (display=value optimized out, client_data=value optimized out, call_data=value optimized out) at ../../../src/solaris/native/sun/awt/awt_InputMethod.c:1464 #1 0x0036b406dc4f in _XimFilterPropertyNotify (display=0x7f503c0df300, window=value optimized out, event=value optimized out, client_data=value optimized out) at imInsClbk.c:126 #2 0x7f4ff24f0f3f in Java_sun_awt_X11_XlibWrapper_XFilterEvent ( env=value optimized out, clazz=value optimized out, ptr=value optimized out, window=value optimized out) at ../../../src/solaris/native/sun/xawt/XlibWrapper.c:486 #3 0x7f5039010f50 in ?? () #4 0x7f5039005953 in ?? () #5 0x in ?? () This causes the X11im pointer to get initialized and the checks in getX11InputMethodData are escaped. The patch simply makes sure that the checks are not escaped by calling getX11InputMethodData in DestroyXIMCallback. Any thoughts or comments? Cheers, Omair (gdb) bt #0 0x0036b10329a5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x0036b1034185 in abort () at abort.c:92 #2 0x7fb5e268c4b9 in os::abort (dump_core=true) at /usr/src/debug/icedtea6-1.8.1/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1481 #3 0x7fb5e27a7da1 in VMError::report_and_die (this=0x7fb5911dc9e0) at /usr/src/debug/icedtea6-1.8.1/openjdk/hotspot/src/share/vm/utilities/vmError.cpp:868 #4 0x7fb5e268fae3 in JVM_handle_linux_signal (sig=11, info= 0x7fb5911dcbb0, ucVoid=0x7fb5911dca80, abort_if_unrecognized=-1860318880) at /usr/src/debug/icedtea6-1.8.1/openjdk/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp:494 #5 signal handler called #6 0x7fb544030600 in ?? () #7 0x7fb5927cbb02 in Java_sun_awt_X11InputMethod_resetXIC (env= 0x7fb5dc3611b8, this=value optimized out) at ../../../src/solaris/native/sun/awt/awt_InputMethod.c:1824 #8 0x7fb5d9010f50 in ?? () #9 0x7fb5911dcf80 in ?? () #10 0x7fb5d9005953 in ?? () #11 0x7fb5911dcf30 in ?? () #12 0x in ?? () import java.awt.BorderLayout; import javax.swing.JFrame; import javax.swing.JPanel; import javax.swing.JTextField; import javax.swing.UIManager; import javax.swing.border.EmptyBorder; public class GtkJTextFieldTest { public static void main(String[] args) throws Exception { UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName()); final JTextField textField1 = new JTextField(); textField1.setText(the quick brown fox jumps over the lazy dog); final JTextField textField2 = new JTextField(); textField2.setText(THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG); final JPanel panel = new JPanel(); panel.setLayout(new BorderLayout()); panel.setBorder(new EmptyBorder(10, 10, 10, 10)); panel.add(textField1, BorderLayout.NORTH); panel.add(textField2, BorderLayout.SOUTH); final JFrame
AWT Dev Request for review: Crash on XIM server restart
Hi, While looking at the bug filed at https://bugzilla.redhat.com/show_bug.cgi?id=572147, I noticed that Java applications crash if IBus (an X input server) is restarted. The webrev is at http://cr.openjdk.java.net/~omajid/webrevs/crash-on-xim-server-restart/webrev.00/ The code in awt_InputMethod.c guards against the XIM server going away, but does not guard against it appearing again. When the XIM server is restarted, the X server can send async requests to call OpenXIMCallback in Java automatically: (gdb) bt #0 OpenXIMCallback (display=value optimized out, client_data=value optimized out, call_data=value optimized out) at ../../../src/solaris/native/sun/awt/awt_InputMethod.c:1464 #1 0x0036b406dc4f in _XimFilterPropertyNotify (display=0x7f503c0df300, window=value optimized out, event=value optimized out, client_data=value optimized out) at imInsClbk.c:126 #2 0x7f4ff24f0f3f in Java_sun_awt_X11_XlibWrapper_XFilterEvent ( env=value optimized out, clazz=value optimized out, ptr=value optimized out, window=value optimized out) at ../../../src/solaris/native/sun/xawt/XlibWrapper.c:486 #3 0x7f5039010f50 in ?? () #4 0x7f5039005953 in ?? () #5 0x in ?? () This causes the X11im pointer to get initialized and the checks in getX11InputMethodData are escaped. The patch simply makes sure that the checks are not escaped by calling getX11InputMethodData in DestroyXIMCallback. Any thoughts or comments? Cheers, Omair
Re: AWT Dev [PATCH] Fix X11 window size calculation
Hi all, Anthony Petrov wrote: Hi Omair, I've just been thinking: shouldn't the test include a legal notice in the beginning of the file? Please use any existing test as an example. Whoops. Patch updated. Also, I wont be getting commit rights any time soon. So if this patch is ok, could someone please go ahead and commit it? Cheers, Omair diff -r 63d087cacbf9 src/solaris/classes/sun/awt/X11/WindowDimensions.java --- a/src/solaris/classes/sun/awt/X11/WindowDimensions.java Tue Jan 13 20:04:05 2009 +0100 +++ b/src/solaris/classes/sun/awt/X11/WindowDimensions.java Mon Jan 19 11:51:39 2009 -0500 @@ -32,10 +32,18 @@ private Insets insets; private boolean isClientSizeSet; +/** + * If isClient is true, the bounds represent the client window area. + * Otherwise, they represent the entire window area, with the insets included + */ public WindowDimensions(int x, int y, int width, int height, boolean isClient) { this(new Rectangle(x, y, width, height), null, isClient); } +/** + * If isClient is true, then rec is treated as the client window rectangle. + * Otherwise rec is treated as the whole window rectangle with the insets included. + */ public WindowDimensions(Rectangle rec, Insets ins, boolean isClient) { if (rec == null) { throw new IllegalArgumentException(Client bounds can't be null); @@ -46,10 +54,18 @@ setInsets(ins); } +/** + * If isClient is true, then size represents the client window size. + * Otherwise, size represents the whole window size with the insets included. + */ public WindowDimensions(Point loc, Dimension size, Insets in, boolean isClient) { this(new Rectangle(loc, size), in, isClient); } +/** + * If isClient is true, the bounds represent the client window area. + * Otherwise, they represent the entire window area, with the insets included + */ public WindowDimensions(Rectangle bounds, boolean isClient) { this(bounds, null, isClient); } diff -r 63d087cacbf9 src/solaris/classes/sun/awt/X11/XDecoratedPeer.java --- a/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java Tue Jan 13 20:04:05 2009 +0100 +++ b/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java Mon Jan 19 11:51:39 2009 -0500 @@ -492,7 +492,11 @@ // do nothing but accept it. Rectangle reqBounds = newDimensions.getBounds(); Rectangle newBounds = constrainBounds(reqBounds.x, reqBounds.y, reqBounds.width, reqBounds.height); -newDimensions = new WindowDimensions(newBounds, newDimensions.getInsets(), newDimensions.isClientSizeSet()); +Insets insets = newDimensions.getInsets(); +Rectangle clientBounds = new Rectangle(newBounds.x, newBounds.y, newBounds.width - insets.left - insets.right, +newBounds.height - insets.top - insets.bottom); +newDimensions = new WindowDimensions(newDimensions.isClientSizeSet() ? clientBounds : newBounds, + insets, newDimensions.isClientSizeSet()); } XToolkit.awtLock(); try { diff -r 63d087cacbf9 test/java/awt/Frame/FrameSize/TestFrameSize.java --- /dev/null Thu Jan 01 00:00:00 1970 + +++ b/test/java/awt/Frame/FrameSize/TestFrameSize.java Mon Jan 19 11:51:39 2009 -0500 @@ -0,0 +1,69 @@ +/* + * Copyright 2009 Red Hat, Inc. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +/* + @test + @bug 6721088 + @summary X11 Window sizes should be what we set them to + @author Omair Majid oma...@redhat.com + @run main TestFrameSize + */ + + +/** + * TestFrameSize.java + * + * Summary: test that X11 Awt windows are drawn with correct sizes + * + * Test fails if size of window is wrong + */ + +import java.awt.Dimension; +import java.awt.Frame; + +public class TestFrameSize { + + static Dimension desiredDimensions = new Dimension(200, 200); + static int ERROR_MARGIN = 15; + static Frame mainWindow; + + public static void drawGui() { + mainWindow = new Frame
Re: AWT Dev [PATCH] Fix X11 window size calculation
Hi Artem, Artem Ananiev wrote: Hi, Omair, thank you for the patch. It seems that the bug was closed by mistake, so I have just reopened it against AWT. Thanks! As for proposed changes, there is a single question: --- jdk/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java.orig 2009-01-08 16:53:54.0 -0500 +++ jdk/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java 2009-01-08 16:54:08.0 -0500 @@ -478,7 +478,10 @@ // do nothing but accept it. Rectangle reqBounds = newDimensions.getBounds(); Rectangle newBounds = constrainBounds(reqBounds.x, reqBounds.y, reqBounds.width, reqBounds.height); -newDimensions = new WindowDimensions(newBounds, newDimensions.getInsets(), newDimensions.isClientSizeSet()); +Insets insets = newDimensions.getInsets(); +Rectangle fixedBounds = new Rectangle(newBounds.x, newBounds.y, newBounds.width - insets.left - insets.right, +newBounds.height - insets.top - insets.bottom); +newDimensions = new WindowDimensions(fixedBounds, insets, newDimensions.isClientSizeSet()); } XToolkit.awtLock(); try { WindowDimensions class contains either client size or full size of the window, depending on 'isClientSizeSet' flag. fixedBounds from your patch is always a client rect, however isClientSizeSet is used from newDimensions... I'd rewrite the last changed line this way: +newDimensions = new WindowDimensions(fixedBounds, insets, true); or, even better, this this one: +newDimensions = new WindowDimensions(newDimensions.isClientSizeSet() ? fixedBounds : newBounds, insets, newDimensions.isClientSizeSet()); Your comments? Thanks for looking over the patch! As for your change, that seems like the correct thing to do. Would it be possible to add the explanation for isClientSizeSet()'s usage to WindowDimensions class? I just want to make sure that nobody else makes the mistake I just did. Cheers, Omair
AWT Dev [PATCH] Fix X11 window size calculation
Hi, This patch fixes an error in the calculation of an X11 window size. The inset size was being included in the calculation of the client area size for a window. A jtreg test is included. This was tested with openjdk6 but seems to be present in openjdk7 too. Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6721088 Cheers, Omair --- jdk/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java.orig 2009-01-08 16:53:54.0 -0500 +++ jdk/src/solaris/classes/sun/awt/X11/XDecoratedPeer.java 2009-01-08 16:54:08.0 -0500 @@ -478,7 +478,10 @@ // do nothing but accept it. Rectangle reqBounds = newDimensions.getBounds(); Rectangle newBounds = constrainBounds(reqBounds.x, reqBounds.y, reqBounds.width, reqBounds.height); -newDimensions = new WindowDimensions(newBounds, newDimensions.getInsets(), newDimensions.isClientSizeSet()); +Insets insets = newDimensions.getInsets(); +Rectangle fixedBounds = new Rectangle(newBounds.x, newBounds.y, newBounds.width - insets.left - insets.right, +newBounds.height - insets.top - insets.bottom); +newDimensions = new WindowDimensions(fixedBounds, insets, newDimensions.isClientSizeSet()); } XToolkit.awtLock(); try { --- /dev/null 2009-01-09 04:32:08.413012246 -0500 +++ jdk/test/java/awt/Frame/FrameSize/TestFrameSize.java2009-01-09 11:30:54.0 -0500 @@ -0,0 +1,50 @@ +/* + @test + @bug 6721088 + @summary X11 Window sizes should be what we set them to + @author Omair Majid oma...@redhat.com + @run main TestFrameSize + */ + +import java.awt.Dimension; +import java.awt.Frame; + +/** + * TestFrameSize.java + * + * Summary: test that X11 Awt windows are drawn with correct sizes + * + * Test fails if size of window is wrong + */ + +public class TestFrameSize { + + static Dimension desiredDimensions = new Dimension(200, 200); + static int ERROR_MARGIN = 15; + static Frame mainWindow; + + public static void drawGui() { + mainWindow = new Frame(); + mainWindow.setPreferredSize(desiredDimensions); + mainWindow.pack(); + // mainWindow.setVisible(true); + + Dimension actualDimensions = mainWindow.getSize(); + // System.out.println(desiredDimensions); + // System.out.println(actualDimensions); + if (Math.abs(actualDimensions.height - desiredDimensions.height) ERROR_MARGIN) { + throw new RuntimeException(Incorrect widow size); + } + + } + + public static void main(String[] args) { + try { + drawGui(); + } finally { + if (mainWindow != null) { + mainWindow.dispose(); + } + } + } +}
AWT Dev [PATCH] Fix invalid PropertyChangeEvents from being propagated
Hi, The current implementation of AccessibleContext propagates PropertyChangeEvents that dont correspond do a change. This patch checks that the new value and the old value are different before firing a PropertyChangeEvent. This has been committed to icedtea as http://icedtea.classpath.org/hg/icedtea6/rev/136f40a0dae4. Apologies in advance if this isn't the correct list to post this patch. Cheers, Omair --- AccessibleContext.java.orig 2008-12-24 11:33:22.0 -0500 +++ jdk/src/share/classes/javax/accessibility/AccessibleContext.java 2008-12-24 11:35:29.0 -0500 @@ -736,6 +736,14 @@ Object oldValue, Object newValue) { if (accessibleChangeSupport != null) { + +if (oldValue == newValue) { +return; +} +if (oldValue != null newValue != null oldValue.equals(newValue)) { +return; +} + if (newValue instanceof PropertyChangeEvent) { PropertyChangeEvent pce = (PropertyChangeEvent)newValue; accessibleChangeSupport.firePropertyChange(pce);