Re: Review request: 8010925: COPY AND PASTE TO AND FROM SIGNED APPLET FAILS AFTER FIRST INTERNAL COPY PRFRMD

Wed, 03 Apr 2013 05:38:44 -0700 

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.

Reply via email to