Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-10-01 Thread Anthony Petrov
Looks fine. Thank you. -- best regards, Anthony On 09/30/2013 04:48 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7092283/webrev.02/ - synchronization on tree lock is used instead of the setter method - the readObject() metho

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-30 Thread Alexander Scherbatiy
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7092283/webrev.02/ - synchronization on tree lock is used instead of the setter method - the readObject() method is not modified Thanks, Alexandr. On 9/27/2013 5:59 PM, Anthony Petrov wrote: On 09/27/2013 05

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Anthony Petrov
On 09/27/2013 05:03 PM, Alexander Scherbatiy wrote: On 9/27/2013 4:24 PM, Anthony Petrov wrote: I'm not sure if this is the right approach because the setter is overridable. So whenever you update the field, you may actually call user's code which in most cases is undesirable. Also, you really s

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Alexander Scherbatiy
On 9/27/2013 4:24 PM, Anthony Petrov wrote: Hi Alexander, I'm not sure if this is the right approach because the setter is overridable. So whenever you update the field, you may actually call user's code which in most cases is undesirable. Also, you really shouldn't do this in the context of

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Anthony Petrov
On 09/27/2013 04:33 PM, Sergey Bylokhov wrote: On 27.09.2013 16:24, Anthony Petrov wrote: Hi Alexander, I'm not sure if this is the right approach because the setter is overridable. So whenever you update the field, you may actually call user's code which in most cases is undesirable. Also, you

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Sergey Bylokhov
On 27.09.2013 16:24, Anthony Petrov wrote: Hi Alexander, I'm not sure if this is the right approach because the setter is overridable. So whenever you update the field, you may actually call user's code which in most cases is undesirable. Also, you really shouldn't do this in the context of r

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Anthony Petrov
Hi Alexander, I'm not sure if this is the right approach because the setter is overridable. So whenever you update the field, you may actually call user's code which in most cases is undesirable. Also, you really shouldn't do this in the context of readObject() because the object isn't fully

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-27 Thread Alexander Scherbatiy
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/7092283/webrev.01 - the locationByPlatform property is updated by setter method - the open and closed tests are passed with the fix except one which fails without the fix too: java/awt/Dialog/MakeWindowAlwaysOnTop/

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-25 Thread Anthony Petrov
Hi Alexander, The setter and getter for this property access the boolean flag under the tree lock. I suppose this was the locking design for this field. So you should reset it under this lock, too. You could also go ahead and add proper locking to other methods that access it (e.g. show(), etc

Re: [8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-25 Thread Sergey Bylokhov
Hi, Alexander. The fix looks good. On 25.09.2013 16:50, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-7092283 webrev: http://cr.openjdk.java.net/~alexsch/7092283/webrev.00 The Window.hide() method does not clear the locationBy

[8] Review request for 7092283 Property Window.locationByPlatform is not cleared by calling setVisible(false)

2013-09-25 Thread Alexander Scherbatiy
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-7092283 webrev: http://cr.openjdk.java.net/~alexsch/7092283/webrev.00 The Window.hide() method does not clear the locationByPlatform property. Thanks, Alexandr.