<AWT Dev> [9] Review Request for JDK-8024061: Exception thrown when drag and drop between two components is executed quickly

[Alexey Ivanov]

Hi Sergey,

Thank you for your review.

Regards,
Alexey.

On 17.04.2014 19:22, Sergey Bylokhov wrote:
> Hi, Alexey.
> The fix looks good.
>
> On 4/15/14 3:13 PM, Petr Pchelko wrote:
>> Hello, Alexey.
>>
>> I’m fine with your fix.
>>
>> Thank you for filing a new bug for JDK 9.
>>
>> With best regards. Petr.
>>
>> 15 апр. 2014 г., в 3:02 после полудня, Alexey Ivanov 
>> <alexey.ivanov at oracle.com> написал(а):
>>
>>> Hi Petr,
>>>
>>> Please look at the updated webrev with comment explaining why 
>>> additional 'if' clause is added:
>>> http://cr.openjdk.java.net/~dmarkov/8024061/jdk9/webrev.01/
>>>
>>> I have submitted a new bug JDK-8040248:
>>> "Drag and drop between two components fails if performed quickly"
>>> https://bugs.openjdk.java.net/browse/JDK-8040248
>>>
>>> Regards,
>>> Alexey.
>>>
>>> On 10.04.2014 18:31, Petr Pchelko wrote:
>>>> Hello, Alexey.
>>>>
>>>> Thank you for the clarification.
>>>>
>>>>> What actually happens is that native drag-and-drop operation 
>>>>> completed already whereas this drag-and-drop operation on EDT 
>>>>> didn't even started yet. Does throwing 
>>>>> InvalidDnDOperationException look more appropriate than returning 
>>>>> null in this case?
>>>> No, I think it’s not good. As a workaround it’s better to not 
>>>> return the data than to throw an exception. It would be really hard 
>>>> for the users to deal with such an exception.
>>>>
>>>> Please add some comment to the if clause to clarify what’s happening.
>>>> Otherwise the fix looks good to me.
>>>>
>>>> With best regards. Petr.
>>>>
>>>> 10 апр. 2014 г., в 5:39 после полудня, Alexey Ivanov 
>>>> <alexey.ivanov at oracle.com> написал(а):
>>>>
>>>>> Hi Petr,
>>>>>
>>>>> If the requested data flavor is 
>>>>> DataFlavor.javaJVMLocalObjectMimeType, then transferable is local 
>>>>> to the JVM. It is stored to a static variable in 
>>>>> SunDropTargetContextPeer.setCurrentJVMLocalSourceTransferable(). 
>>>>> When processEnterMessage() is handled, its value is copied to 
>>>>> instance variable "local". The instance variable is cleaned by 
>>>>> processExitMessage(), the static variable is cleared by 
>>>>> SunDragSourceContextPeer.cleanup().
>>>>>
>>>>> If drag-and-drop operation is performed quickly, 
>>>>> SunDragSourceContextPeer.cleanup() is called when mouse button is 
>>>>> released and *before* processEnterMessage() is called. So when EDT 
>>>>> finally handles MOUSE_ENTERED event and 
>>>>> DropTargetListener.dragEnter() gets called, "local" transferable 
>>>>> is already null. The listener tries to get the data from the 
>>>>> transferable:
>>>>> DropTargetDragEvent.getTransferable(), and then
>>>>> Transferable.getTransferData(dataFlavor).
>>>>>
>>>>> The dataFlavor has MIME type of 
>>>>> DataFlavor.javaJVMLocalObjectMimeType, and 
>>>>> SunDropTargetContextPeer.currentJVMLocalSourceTransferable is 
>>>>> already cleared. The representationClass field of the DataFlavor 
>>>>> object is set to java.io.InputStream.class. So when 
>>>>> SunDropTargetContextPeer.getNativeData() executes it tries to cast 
>>>>> the data object to java.io.InputStream to deserialize the data but 
>>>>> fails with the exception:
>>>>> java.lang.ClassCastException: DnD$DropObject cannot be cast to 
>>>>> java.io.InputStream.
>>>>>
>>>>> This exception is caught internally, and IOException is thrown as 
>>>>> it failed to get the data. At the same time, no IO should have 
>>>>> occurred because a local source was dragged, so this exception is 
>>>>> rather unexpected.
>>>>>
>>>>>
>>>>> What actually happens is that native drag-and-drop operation 
>>>>> completed already whereas this drag-and-drop operation on EDT 
>>>>> didn't even started yet. Does throwing 
>>>>> InvalidDnDOperationException look more appropriate than returning 
>>>>> null in this case?
>>>>>
>>>>>
>>>>> Sure, I'll submit a new bug for 9 to implement the proper fix.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Alexey.
>>>>>
>>>>> On 10.04.2014 12:52, Petr Pchelko wrote:
>>>>>> Hello, Dmitry.
>>>>>>
>>>>>> Thank you for the detailed explanation of the issue.
>>>>>>
>>>>>> I’m OK with this workaround as this should be back ported to 8u 
>>>>>> and 7u, but I think we could try to provide a real fix in 9.
>>>>>> Could you please add some comment which shortly explains why the 
>>>>>> if clause was added?
>>>>>> And could you also file a new bug for 9 so that we try to replace 
>>>>>> the workaround with a real fix?
>>>>>>
>>>>>> Thank you.
>>>>>> With best regards. Petr.
>>>>>>
>>>>>> 10 апр. 2014 г., в 12:18 после полудня, Alexey Ivanov 
>>>>>> <alexey.ivanov at oracle.com> написал(а):
>>>>>>
>>>>>>> Hello AWT team,
>>>>>>>
>>>>>>> Could you please review the fix for jdk9:
>>>>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8024061
>>>>>>>     webrev: 
>>>>>>> http://cr.openjdk.java.net/~dmarkov/8024061/jdk9/webrev.00/
>>>>>>>
>>>>>>> Problem description:
>>>>>>> Dragging and dropping an object from one panel to another very 
>>>>>>> quickly causes an exception.
>>>>>>>
>>>>>>> Fix:
>>>>>>> The fix addresses only the thrown exception.
>>>>>>> The dragged object would not be moved.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexey.
>
>