Hello, Alan. Thank you for the review. I've updated the fix according to your comments. The new version is here: http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.02/
Only the DataFlavorUtil file is updated. DesktopService now uses a lazy holder, the doc is fixed, the getFlavorTable method is fixed. All the rest is the same as in the previous version. Thank you. With best regards. Petr. On 22 июля 2014 г., at 19:08, Alan Bateman <alan.bate...@oracle.com> wrote: > 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/ >> >> 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.
