Updated webrev to modify java.desktop module-info.java (only difference
between webrev7 and this) to remove the duplicate exports of sun.awt so
we will have now
exports sun.awt to
jdk.accessibility,
jdk.unsupported.desktop;
http://cr.openjdk.java.net/~psadhukhan/fxswing.8/
Regards
Prasanta
On 5/5/2018 2:28 PM, Prasanta Sadhukhan wrote:
Hi All,
I have tried to address all comments from Nir, Phil, Mandy and Kevin
got so far in this webrev
http://cr.openjdk.java.net/~psadhukhan/fxswing.7
>>In SwingNode, lines 870 -883, is there a reason that only the first
method uncommented @Override? I don't understand the comment that
wants to skip @Override.
If @override is not added, then it will cause recursion between
LightweightContent.java:130 and LightweightContent.java:187 thereby
causing StackOverflowError. I am also not sure about the comment of
skipping @Override but I have removed it from this webrev.
Regards
Prasanta
On 5/5/2018 4:32 AM, Kevin Rushforth wrote:
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
||
