<AWT Dev> [8] Review request for 8024061: Exception thrown when drag and drop between two components is executed quickly

[Anthony Petrov]

Hi Anton,

The analysis of the problem looks good. Please clarify one point:

"(2) mouse moved to target in a single X11 event"

Is the "single event" the one from (1) or from (3) ? IOW, are the (1) 
and (2) a single event, or the (2) and (3) ?

Anyway, I have two concerns regarding the fix:

1. In startDrag() you now call the setNativeContext() and 
setCurrentJVMLocalSourceTransferable() under the awtLock, whereas 
previously these calls were preformed w/o the lock. This seems a bit 
risky as potentially this may cause some deadlocks. We should possibly 
rearrange the calls so that processMouseMove() is called before the 
above two calls, and the two calls remain outside the lock. 
Alternatively, we could acquire the lock again just for the sake of a 
call to processMouseMove().

2. It looks suspicious that we call processMouseMove() unconditionally. 
The dragGestureMouseMotion could be stale, or uninitialized yet. Also, 
the event might actually be processed twice because doProcessEvent() 
returns false in that case.

--
best regards,
Anthony

On 10/14/2013 06:26 PM, anton nashatyrev wrote:
> Hi Artem,
>
>      with the fix the 'drag entered' event is actually generated on the
> mouse move event (step 2). Currently this first move event is 'consumed'
> by the MouseGestureRecognizer and is not treated as the part of a drag
> operation, while this move could be important (as in our case).
>      During the process I've created a couple of fixes which tried to
> address the step 3, but all of them seemed to me rather like hacks than
> fixes.
>
>      BTW, I've run the regression DnD tests (from
> test/closed/java/awt/dnd + test/java/awt/dnd, totally about 75 tests) -
> all went fine.
>
> Thanks!
> Anton.
>
> On 14.10.2013 17:45, Artem Ananiev wrote:
>> Hi, Anton,
>>
>> thanks for details description of the bug and suggested fix. It helped
>> a lot.
>>
>> My understanding is that the problem is not with step 2, but step 3.
>> Generating "drag entered" events on mouse press looks incompatible, I
>> can't predict side effects of such a change. Fixing step 3, so we
>> don't assume drag entered events already sent, looks more correct.
>> What do you think?
>>
>> Thanks,
>>
>> Artem
>>
>> On 10/11/2013 7:49 PM, anton nashatyrev wrote:
>>> Hello,
>>>      could you please review the following fix:
>>>
>>> fix: http://cr.openjdk.java.net/~mcherkas/anton/8024061/webrev.00/
>>> <http://cr.openjdk.java.net/%7Emcherkas/anton/8024061/webrev.00/>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8024061
>>>
>>>      Problem:  when doing quick drag'n'drop in X11 the incorrect
>>> behavior appears in the different forms. One is the exception when
>>> trying to get 'local Jvm' Transferable from DropTargetListener.
>>>
>>>      Reason: on quick drag the following sequence of events may appear:
>>> (1) mouse pressed on source  -> (2) mouse moved to target in a single
>>> X11 event -> (3) mouse released. What's happening in that case: on event
>>> (2) only a drag gesture is recognized and no drag enter is generated
>>> yet. On event (3) the XDragSourceContextPeer assumes that entered event
>>> (if it happened) was already generated and the 'local JVM' transferable
>>> had been already captured by SunDropTargetContextPeer from the static
>>> field currentJVMLocalSourceTransferable. Thus on event (3) the
>>> XDragSourceContextPeer posts the additional mouseMove event (which turns
>>> into DragEnter later) and initiates the cleanup which then resets the
>>> currentJVMLocalSourceTransferable to null. Thus on DragEnter the
>>> currentJVMLocalSourceTransferable is already null and the
>>> SunDropTargetContextPeer appears in the inconsistent state.
>>>
>>>      Solution: the event (2) from the example above should not only
>>> initiate the DnD operation but also be a part of that operation, i.e.
>>> this event should also appear as a drag motion. For that I propose to
>>> keep a track of the XMotionEvents in the XDragSourceContextPeer to catch
>>> the mouse event which initiated the DnD and when a startDrag() is called
>>> process this event as the first drag motion event.
>>>
>>> Thanks!
>>> Anton.
>