My quick comments:
1. The changes to java.desktop module-info.java won't compile when
applied to jdk-client, since there is already a qualified export of the
sun.swing package to another internal class. Can you update your patch
to be based off of jdk-client? I note that it requires a small patch to
be able to build FX with JDK 11 (it's a known gradle issue), but it's
not hard to do.
2. As I mentioned in an off-line email, the final version of the
javafx.swing patch will need to use 'requires static
jdk.unsupported.desktop' in its module-info.java, so that it will still
run on JDK 10 EA. This means that you will need to ensure that
jdk.unsupported.desktop is loaded via a service loader as discussed.
This will not affect the public API at all, so that can proceed in
parallel, but this needs to be fixed.
3. As I mentioned in my earlier reply to Mandy's comment, Swing interop
apps will fail if a security manager is installed, because the
jdk.unsupported.desktop classes are loaded by the app class loader. This
can be sorted out later as a follow-on bug, with permissions added to
the default policy file, etc.
4. As Phil mentioned, some docs would be good. They don't need to be all
that detailed, since it isn't intended for use by applications, and will
not be included in the published API docs bundle (i.e., it should be
excluded from "make docs").
I will do a more detailed review early next week.
-- Kevin
On 5/4/2018 1:53 PM, Phil Race wrote:
Two quick comments.
1) I'd like "Peer" removed from all the exported API.
2) It is probably stable enough now that you can start to add some
javadoc.
-phil.
On 05/04/2018 05:00 AM, Prasanta Sadhukhan wrote:
Hi All,
Please review an enhancement to remove the tight coupling of JDK
internal class from FX so that
when javafx.* modules are removed from Openjdk build in jdk11, FX in
general, and fx swing interop, in particular, can still build and
function.
Right now, FX uses 6 jdk internal packages
sun.swing, sun.awt, java.awt.dnd.peer, sun.awt.dnd, sun.awt.image and
sun.java2d.
However, the goal is not to use qualified exports of these internal
packages once FX is removed to be built along with JDK,
The solution will define a new "jdk.unsupported.desktop" module that
exports public API
that is intended to be used by the javafx.swing module (but it does
so with public exports and not qualified exports).
The idea is the same as jdk.unsupported, in that it is documented as
being unsupported.
Further, since it is only intended to be used by javafx.swing, it
need not be in the default module graph.
The module-info.java will look like this:
|module jdk.unsupported.desktop { requires transitive java.desktop;
exports jdk.swing.interop; } |||
Enhancement: https://bugs.openjdk.java.net/browse/JDK-8202199,
https://bugs.openjdk.java.net/browse/JDK-8195811
webrev: cr.openjdk.java.net/~psadhukhan/fxswing.6/
CSR: https://bugs.openjdk.java.net/browse/JDK-8202175
Regards
Prasanta
||
