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

[Anthony Petrov]

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. 
>