Request for review: RT-28347 - DnD between two JFXPanels

[Anthony Petrov]

Hi Artem,

There's a lot of code changes and I'm not sure if I see the complete 
picture of how DnD works with embedded scenes (and not that I'm a great 
expert in DnD anyway.) It would be nice to have a short overview of the 
embedded DnD architecture (put in a class javadoc for, say, the 
EmbeddedSceneDnD class.)
However, generally all the changes look good. I have a few questions:

1. src/main/java/com/sun/javafx/tk/quantum/EmbeddedSceneDnD.java
>  67         dragStartListener.dragStarted(fxDragSource, TransferMode.COPY);

Why do we hard-code the transfer mode?


2.
>   94     // can be implemented using AWT nested event loop, however it just
>   95     // blocks the current thread. This is done by intention, because
>   99     <T> T executeOnFXThread(final Callable<T> r) {

I'm not sure I'm happy with this method. Could you provide more details 
on where and how exactly it is used? Also, could you please convert this 
comment into a javadoc?


3. src/main/java/javafx/embed/swing/JFXPanel.java
>  799         public void setEmbeddedScene(EmbeddedSceneInterface embeddedScene) {
>  804             if (scenePeer == null) {
>  805                 dnd.removeNotify();
>  806                 dnd = null;

Above we manipulate the dnd object directly on the current thread, while 
at lines 817-818:
>  814             SwingUtilities.invokeLater(new Runnable() {
>  817                     dnd = new SwingDnD(JFXPanel.this, scenePeer);
>  818                     dnd.addNotify();

... we do this on the EDT. This asymmetry looks a bit suspicious. Could 
you elaborate on that?


4. src/main/java/javafx/embed/swing/SwingDnD.java
>  183                 // Don't call updateContents() from drop(). In AWT, it is possible to
>  184                 // get data from the Transferable object in drop() only after the drop
>  185                 // has been accepted. Here we first let FX handle drop(), then accep

This looks like not the best solution from performance perspective. 
Also, I still don't understand why exactly we can't get the data when 
drop() occurs. Could you please explain this?

--
best regards,
Anthony

On 11/01/2013 07:37 PM, Artem Ananiev wrote:
> Hi,
>
> could you take a look at the following fix, please:
>
> http://cr.openjdk.java.net/~art/javafx/RT-28347/
>
> Some information about the changes is available in bug comments:
>
> https://javafx-jira.kenai.com/browse/RT-28347
>
> Thanks,
>
> Artem