<AWT Dev> [9] Review Request: 8037485 Refactor java.awt.datatransfer to eliminate dependency on AWT

[Petr Pchelko]

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.chung at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20140724/b42f969b/attachment.html>