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

[Sergey Bylokhov]

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20130402/a1eebcf6/attachment.html