<AWT Dev> Review request: 8010925: COPY AND PASTE TO AND FROM SIGNED APPLET FAILS AFTER FIRST INTERNAL COPY PRFRMD

[mikhail cherkasov]

Hello Anthony, Sergey,

JNFLoop was replaced with ThreadUtilities:
http://cr.openjdk.java.net/~mcherkas/8010925/webrev.03/ 
<http://cr.openjdk.java.net/%7Emcherkas/8010925/webrev.03/>

Thanks,
Mikhail.

On 03.04.2013 15:11, Anthony Petrov wrote:
> I agree with Sergey. That's what I was originally suggesting, too.
>
> Otherwise the fix looks fine.
>
> -- 
> best regards,
> Anthony
>
> On 4/2/2013 22:03, Sergey Bylokhov wrote:
>>
>> Hi, Mikhail,
>> You should use ThreadUtilities pomt instead of JNFLoop. Commented 
>> NSloog is unnecessary.
>>
>>
>> On 4/2/13 8:45 PM, mikhail cherkasov wrote:
>>> Hello again,
>>>
>>> http://cr.openjdk.java.net/~mcherkas/8010925/webrev.02/ 
>>> <http://cr.openjdk.java.net/%7Emcherkas/8010925/webrev.02/>
>>>
>>> Thanks,
>>> Mikhail.
>>> On 02.04.2013 20:16, Anthony Petrov wrote:
>>>> Hi Mikhail,
>>>>
>>>> Yes, I agree we should fix this issue. And as I said, the fix looks 
>>>> good to me. Here's just a few minor, mostly stylistic comments:
>>>>
>>>> src/macosx/classes/sun/lwawt/macosx/CClipboard.java
>>>>>  114     // 1.7 peer method
>>>>>  115     // introduced for implementation fix for 8010925
>>>>>  116     public native void checkPasteboard();
>>>>
>>>> This comment might better be converted to a javadoc-style comment. 
>>>> After all, this is a public method, even though it's in a private 
>>>> package. And btw, the @since tag would look just fine then. Also, 
>>>> the text should describe what this method actually do, not just 
>>>> mention the bug id (which in most cases is useless since this 
>>>> information can easily be retrieved using hg log/annotate).
>>>>
>>>>
>>>> src/macosx/classes/sun/lwawt/macosx/CEmbeddedFrame.java
>>>>>  115         if ( focused ) {
>>>>
>>>> This should read as "if (focused) {" - please note the spaces.
>>>>
>>>>
>>>> src/macosx/native/sun/awt/CClipboard.m
>>>>>  385     void (^checkPasteboardBlock)() = ^(){
>>>>>  386         [[CClipboard sharedClipboard] checkPasteboard:nil];
>>>>>  387     };
>>>>>  388     [JNFRunLoop performOnMainThreadWaiting:YES 
>>>>> withBlock:checkPasteboardBlock];
>>>>
>>>> Again, there's an extra variable which doesn't add any value. How 
>>>> about shortening this to simply:
>>>>
>>>> [JNFRunLoop performOnMainThreadWaiting:YES withBlock:^(){
>>>>     [[CClipboard sharedClipboard] checkPasteboard:nil];
>>>> }];
>>>>
>>>> ? We use this pattern all over the lwawt native code.
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 4/2/2013 19:35, mikhail cherkasov wrote:
>>>>> Hello Anthony, All,
>>>>>
>>>>> There's new version: 
>>>>> http://cr.openjdk.java.net/~mcherkas/8010925/webrev.01/ 
>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8010925/webrev.01/>
>>>>>
>>>>> Apple JDK has the same bug, without fix jdk7 shows the same 
>>>>> behavior as jdk6.
>>>>> Anyway this should be fixed.
>>>>>
>>>>> Thanks,
>>>>> Mikhail.
>>>>>
>>>>> On 01.04.2013 12:59, Anthony Petrov wrote:
>>>>>> I'm not sure if there's a reasonable way to test this fix. You 
>>>>>> can use the Clipboard API w/o creating/showing any top-level 
>>>>>> windows which would emulate the conditions where the bug is 
>>>>>> currently reproduced in the browser environment. However, you'll 
>>>>>> never receive the synthetic focus event then, so the fix won't 
>>>>>> help in this case I guess...
>>>>>>
>>>>>> BTW, did you test compare behavior to Apple JDK? Is it any 
>>>>>> different there?
>>>>>>
>>>>>> -- 
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>> On 3/29/2013 20:25, mikhail cherkasov wrote:
>>>>>>> Hi Anthony ,
>>>>>>>
>>>>>>> will fix, I thought that chain of method calls is something like 
>>>>>>> code convention.
>>>>>>>
>>>>>>> BTW is someone have idea how test can be implemented to this?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Mikhail.
>>>>>>>
>>>>>>> On 29.03.2013 20:10, Anthony Petrov wrote:
>>>>>>>> Hi Mikhail,
>>>>>>>>
>>>>>>>> The idea of the fix looks good to me. Note that you don't need 
>>>>>>>> the -javaCheckPasteboard helper method. There's a method in 
>>>>>>>> ThreadUtilities that takes a block as an argument. This way you 
>>>>>>>> can call the -checkPasteboard directly from the block right in 
>>>>>>>> the JNI method implementation w/o any intermediate methods.
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> best regards,
>>>>>>>> Anthony
>>>>>>>>
>>>>>>>> On 3/29/2013 19:43, mikhail cherkasov wrote:
>>>>>>>>> Hello all,
>>>>>>>>>
>>>>>>>>> Could you please review the following fix:
>>>>>>>>> http://bugs.sun.com/view_bug.do?bug_id=8010925
>>>>>>>>> http://cr.openjdk.java.net/~mcherkas/8010925/webrev.00/ 
>>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8010925/webrev.00/>
>>>>>>>>>
>>>>>>>>> Applet doesn't receive any NSApplication*Notification because it
>>>>>>>>> doesn't create any windows, so if we have applet with windows 
>>>>>>>>> - all works fine.
>>>>>>>>> But in this case all content are added to applet's ContentPane 
>>>>>>>>> that is
>>>>>>>>> embedded to browser and hasn't any window.
>>>>>>>>> But AWT updates clipboard data only on 
>>>>>>>>> NSApplicationDidBecameActiveNotification.
>>>>>>>>> So if we make copy action from applet , applet will never read 
>>>>>>>>> new data from system pastboard,
>>>>>>>>> it just will use cached data.
>>>>>>>>> To fix this I added pasteboard check on CEmbeddedFrame focus 
>>>>>>>>> receiving.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Mikhail.
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>>
>> -- 
>> Best regards, Sergey.