Hello, Thank you for the review. I’ve updated the fix: http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.04/
I’ve fixed all suggestions except one: > Another optimization/simplification is possible. You could merge the > DesktopDatatransferServiceHolder and default implementation into one class: No, I could not. The default implementation is a member of desktop module while DataFlavorUtil is a part of data transfer module. > I skimmed through the webrev and looks good. It looks like that you have > cleanly removed the dependency. You can run jdk9/bin/jdeps [1] on the > java.awt.datatransfer.** and its implementation classes to double check if > there is no dependency to the desktop classes. I’ve run the tool on java.awt.datatransfer and sun.datatransfer packages and there are no dependencies on desktop. > DataFlavorUtil.java > line 544, 549 - some raw types and you may want to check if there are > others. Fixed and checked the patch for another raw/unchecked warning. With best regards. Petr. > On Jul 23, 2014, at 7:04 PM, Mandy Chung <mandy.ch...@oracle.com> wrote: > > > On 7/23/2014 6:53 AM, Petr Pchelko wrote: >> Hello, Alan. >> >>> I'm skimmed over the updated webrev, it mostly looks good except for >>> getFlavorMap where it doesn't set map, I assume you meant to do this: >>> >>> if (map == null) >>> flavorMap = map = supplier.get(); >> Thank you! Updated the fix: >> http://cr.openjdk.java.net/~pchelko/9/8037485/webrev.03/ >> >> > > I skimmed through the webrev and looks good. It looks like that you have > cleanly removed the dependency. You can run jdk9/bin/jdeps [1] on the > java.awt.datatransfer.** and its implementation classes to double check if > there is no dependency to the desktop classes. > > Minor comments I spotted: > java/awt/datatransfer/SystemFlavorMap.java > line 225 and 238 look like debugging statement to be removed. > > DataFlavorUtil.java > line 544, 549 - some raw types and you may want to check if there are > others. > > [1] http://docs.oracle.com/javase/8/docs/technotes/tools/unix/jdeps.html
