<AWT Dev> Review request: 8010925: COPY AND PASTE TO AND FROM SIGNED APPLET FAILS AFTER FIRST INTERNAL COPY PRFRMD
Anthony Petrov
anthony.petrov at oracle.com
Wed Apr 3 05:37:14 PDT 2013
Looks great. Thank you.
--
best regards,
Anthony
On 4/3/2013 16:35, mikhail cherkasov wrote:
> 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.
>
More information about the awt-dev
mailing list