On 22/07/2014 15:04, Petr Pchelko wrote:
Hello,
I've updated the fix. The new version is located here:
http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.01/
<http://cr.openjdk.java.net/%7Epchelko/9/8037485/webrev.01/>
I've reverted RMI changes as they will be handled separately.
Also I've changed package names, now the internal part of the
datatransfer module is called sun.datatransfer. This allowed to make
less renamings and leave sun.awt.datatransfer package as is.
With best regards. Petr.
I'm skimmed over the changes (not a detailed review, best for someone
working in this area to do that). It looks much better to me. We can
deal with the optional dependency on RMI separately.
I've mostly focused on the ServiceLoader usage in this webrev. The only
concern is that getDesktopService is "static synchronized" so that you
cause contention. You could replace this with a lazy holder idiom, or
just make make desktopService volatile, it shouldn't matter if multiple
threads cause desktopDatatransferService is set more than once. A
suggestion for getFlavorMap is use "FlavorMap map = this.flavorMap" and
check it for null to avoid doing two volatile reads.
A minor type on the declaration of flavorMap, it reads "if there is not
Desktop module", I think you mean "if there is not a desktop" or "if
there isn't a desktop module".
-Alan.
