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

Petr Pchelko petr.pchelko at oracle.com
Wed Jul 23 08:25:08 UTC 2014


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

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


More information about the awt-dev mailing list