<AWT Dev> [8] Review request for CR 8006406: lightweight embedding in other Java UI toolkits

[Anton V. Tarasov]

On 2/12/13 7:20 PM, Anthony Petrov wrote:
> Thanks for the clarification. I see your point and I sort of agree 
> that passing the call from *LWFP to LWF, and then to LWC would look 
> not better than the instanceof check in SunToolkit.
>
Right, "not better" (your variant was the first I tried and then 
rejected :)).

> I'm fine with the current code then, as long as it works as expected.

I can't say it works just fine (I know a number of unresolved issues), 
including performance. It works more or less stable to the moment.
But, I think our current task is to approve (or reject) the approach 
principally, so that we can address the issues further with some 
minimized risks to rollback current design decisions.

Thanks,
Anton.

>
>
> -- 
> best regards,
> Anthony
>
> On 2/12/2013 16:22, Anton V. Tarasov wrote:
>> Hi Anthony,
>>
>> (sorry, I missed your reply)
>>
>> On 2/11/13 5:46 PM, Anthony Petrov wrote:
>>> Hi Anton,
>>>
>>> If I read your explanation correctly, the current calls chain is:
>>>
>>>> popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus -> 
>>>> WindowPeer.grabFocus
>>>
>>> I.e. we want to end up in WindowPeer.grabFocus() anyway. Note that 
>>> for non-lightweight frames we want to call WindowPeer.grabFocus() as 
>>> well (even though currently it's named e.g. WWindowPeer.grab() 
>>> instead of grabFocus(), etc.)
>>>
>>> I suggest to reduce this call chain to just:
>>>
>>> popups -> SunToolkit.grab -> WindowPeer.grabFocus
>>>
>>> which would work for both lightweight and heavyweight frames.
>>>
>>> In other words, I suggest to rename the current 
>>> *WindowPeer.grab()/ungrab() methods in all toolkits to "@Override 
>>> grab/ungrabFocus()", and implement the SunToolkit.grab/ungrab() as 
>>> final methods just delegating to the peer's methods. For lightweight 
>>> frames, the *LightweightFramePeer classes must delegate the requests 
>>> to FX.
>>
>> The main difference here is that you suggest to delegate to FX from 
>> peers, while I tend to put delegation to the target (JLightweightFrame).
>>  From my point of view, delegating from peers has the following 
>> disadvantages:
>>
>> - grab/ungrab delegation to FX is platform independent, so peers 
>> don't seem the right place to implement it
>>
>> - anyway, a *LightweightFramePeer only connects to its target, it 
>> knows nothing about the LightweightContent delegate, so a peer would 
>> need to delegate via a JLightweightFrame,
>>    for which it would need to either call a suitable public method or 
>> use "shared secrets" approach to workaround access control. Passing a 
>> LightweightContent delegate to a peer
>>    is also not good from my point of view.
>>
>>>
>>> Note that we can't remove the SunToolkit.grab/ungrab() interface 
>>> anyway since JFXPanel code relies on it already.
>>
>> I think we can rewrite it in JFXPanel (it's implementation details) 
>> right after we have a new AWT grab API.
>>
>>> Therefore, I think that we don't want to introduce the Window. (or 
>>> currently just LightweightFrame.)grab/ungrabFocus() API at this 
>>> time. I believe that a proper peer interface should be enough for now.
>>
>> I supposed we could, at least, rewrite our own code to use 
>> Window.grabFocus and declare SunToolkit.grab deprecated. At last, 
>> this is an _internal_ public API which has a disclaimer.
>>
>> Thanks,
>> Anton.
>>>
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 02/11/13 12:39, Anton V. Tarasov wrote:
>>>> Hi Anthony,
>>>>
>>>> JLightweightFrame overrides focusGrab/focusUngrab and forwards the 
>>>> calls
>>>> to the client app. Ideally, when the grab API is public, it should be
>>>> called directly via Window.focusGrab/focusUngrab. Currently it is 
>>>> called
>>>> via SunToolkit.grab/ungrab (by swing popups). So, normal call chain
>>>> should look like:
>>>>
>>>> popups -> (JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus
>>>>
>>>> Temporarily it's as follows:
>>>>
>>>> popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus ->
>>>> WindowPeer.grabFocus
>>>>
>>>> If I called it directly it would be:
>>>>
>>>> popups -> SunToolkit.grab -> WindowPeer.grabFocus ->
>>>> (JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus -> ...
>>>>
>>>> So, this is quite the reverse direction.
>>>>
>>>> Thanks,
>>>> Anton.
>>>>
>>>> On 2/8/13 7:23 PM, Anthony Petrov wrote:
>>>>> Hi Anton,
>>>>>
>>>>> src/windows/classes/sun/awt/windows/WToolkit.java
>>>>>> 987 public void grab(Window w) {
>>>>>> 991 if (w instanceof LightweightFrame) {
>>>>>> 992 ((LightweightFrame)w).grabFocus();
>>>>>> 993 return;
>>>>>> 994 } 995 if (w.getPeer() != null) {
>>>>>> 996 ((WWindowPeer)w.getPeer()).grab();
>>>>>> 997 }
>>>>>> 998 }
>>>>>
>>>>> You've already added grab/ungrabFocus() to the WindowPeer interface.
>>>>> Why not use them here directly instead of these instanceof checks?
>>>>> Applies to other toolkits as well.
>>>>>
>>>>> The fix looks good otherwise.
>>>>>
>>>>> -- 
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>> On 2/8/2013 21:27, Anton V. Tarasov wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please, review the changes for the CR:
>>>>>> http://bugs.sun.com/view_bug.do?bug_id=8006406
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ant/8006406/webrev.6
>>>>>>
>>>>>> It introduces sun.swing.JLightweightFrame class, aimed at 
>>>>>> lightweight
>>>>>> embedding of Swing components into java-based toolkits.
>>>>>> The primary target is JavaFX toolkit, however the class is not
>>>>>> limited to this usage and the API it provides is quite generic.
>>>>>>
>>>>>> Below I'm giving a link to the jfx side implementation. This
>>>>>> implementation should not be reviewed in this thread (it is in a
>>>>>> pre-review phase),
>>>>>> it should just clarify how the introduced API is supposed to be 
>>>>>> used.
>>>>>> Namely, SwingNode.SwingNodeContent which implements
>>>>>> sun.swing.LightweightContent and forwards requests from
>>>>>> sun.swing.JLightweightFrame to NGExternalNode which does the 
>>>>>> rendering.
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ant/RT-27887/webrev.1
>>>>>>
>>>>>> Some comments on the awt/swing part:
>>>>>>
>>>>>> - Only Win and Mac implementation is currently available, X11 will
>>>>>> come lately.
>>>>>>
>>>>>> - Win implementation uses a heavyweight window behind the 
>>>>>> lightweight
>>>>>> frame, while it is not actually needed for lightweight embedding.
>>>>>> This is due to the architecture of the Win AWT peers which are
>>>>>> strongly tight to the native code, and it's not a trivial task to
>>>>>> separate them.
>>>>>> On Mac the lightweight frame peer is truly lightweight, meaning that
>>>>>> it doesn't create an NSWindow object behind it. The Mac port LW
>>>>>> abstraction
>>>>>> allows to override and substitute CPlatform* classes with their
>>>>>> lightweight stubs.
>>>>>>
>>>>>> - LightweightFrame, among others, introduces two new methods -
>>>>>> grabFocus() and ungrabFocus(boolean). Ideally, these methods should
>>>>>> go to
>>>>>> the super Window class where the grab API becomes public (which is a
>>>>>> long-term project...). Current host of the grab API is 
>>>>>> SunToolkit, which
>>>>>> now forwards the calls to LightweightFrame. This is necessary to
>>>>>> intercommunicate with the client when grab/ungrab happens on both 
>>>>>> sides.
>>>>>>
>>>>>> - Unresolved issues exist, like modal dialogs, d&d etc. They are to
>>>>>> be addressed further.
>>>>>>
>>>>>> Thanks,
>>>>>> Anton.
>>>>>>
>>>>
>>