Hello, Thank you for the reviews.

I have updated the fix, it is now available at:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.01/
And the fx part:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.rt.01/

I have updated comments and used AWTAccessor.

Anthony wrote:
> Regarding the FX part, I'm a bit uncertain if it's a good idea to perform so 
> many actions with undefined semantics. I mean, the 
> SwingFXUtils.runOnFxThread() either executes the runnable here and now 
> (blocking the calling code), or just schedules the execution for later time. 
> Sometimes this indeed makes sense. But in some cases this may be the wrong 
> thing to do because e.g. we may want to block always. I believe we should 
> mention this explicitly in the spec for this method, so that people know what 
> they do when they use this method. Anyway, as long as this code is 
> experimental and is off by default I'm OK with it.
This method was added as an optimization so that we do not post a runnable if 
we are already on the needed thread. Currently it does not make any trouble as 
I have tested, but it is possible to remove it if we get any problems with it 
any time later. I have reduced it's visibility as it should only be used 
internally.

With best regards. Petr.

On May 29, 2013, at 6:54 PM, Anthony Petrov wrote:

> Hi Petr,
> 
> src/macosx/classes/sun/lwawt/macosx/LWCToolkit.java
>>  88     /* If we operate in FX/AWT single threaded mode the nested loop 
>> should run in default mode */
>>  89     private static final boolean inAWT;
> 
> src/macosx/native/sun/awt/LWCToolkit.m
>> 314         isRunning = [[NSRunLoop currentRunLoop] runMode:inAWT ? 
>> [JNFRunLoop javaRunLoopMode] : NSDefaultRunLoopMode
> 
> If I read the comment above right, then we should swap the modes in ?:, no? 
> Also, since ':' has a special meaning in ObjC, I suggest to enclose the ?: 
> expression in parentheses for better readability.
> 
> src/share/classes/sun/awt/FwDispatcher.java
>>   2  * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> Is it true that this code exists since 2011?
> 
>> 41  * @since 8.0
> 
> We use "1.8" to indicate that an API is available since JDK8.
> 
> Regarding the FX part, I'm a bit uncertain if it's a good idea to perform so 
> many actions with undefined semantics. I mean, the 
> SwingFXUtils.runOnFxThread() either executes the runnable here and now 
> (blocking the calling code), or just schedules the execution for later time. 
> Sometimes this indeed makes sense. But in some cases this may be the wrong 
> thing to do because e.g. we may want to block always. I believe we should 
> mention this explicitly in the spec for this method, so that people know what 
> they do when they use this method. Anyway, as long as this code is 
> experimental and is off by default I'm OK with it.
> 
> --
> best regards,
> Anthony
> 
> On 05/28/2013 01:28 PM, Petr Pchelko wrote:
>> Hello, AWT Team.
>> 
>> Please review the fix for the issue:
>> http://bugs.sun.com/view_bug.do?bug_id=8015477
>> The fix is available at:
>> http://cr.openjdk.java.net/~pchelko/8015477/webrev.00/
>> 
>> The issue is just created, so it might not show up on bugs.sun.com
>> 
>> This fix adds a private API to support a single threaded mode for AWT/FX 
>> interop.
>> The main idea is to delegate some methods of an EventQueue to FX, so that 
>> AWT events were processed on FX thread.
>> 
>> Here's the webrev for FX part of changes, for a usage sample:
>> http://cr.openjdk.java.net/~pchelko/8015477/webrev.rt/
>> 
>> With best regards. Petr.
>> 

Reply via email to