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 readObject() because the object
isn't fully initialized yet, so calling overridable methods at this
stage is asking for troubles.
I still suggest to access the field directly, but wrap the accessing
statement with synchronized (getTreeLock()) {}.
Is it a common practice in awt to use synchronized block directly
instead of the setter method?
If a user's code does not use synchronization for the
locationByPlatform setting he really violates the AWT design for this
variable.
Thanks,
Alexandr.
And I wouldn't modify the readObject() at all.
--
best regards,
Anthony
On 09/27/2013 02:33 PM, Alexander Scherbatiy wrote:
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/MakeWindowAlwaysOnTop.java
Thanks,
Alexandr.
On 9/25/2013 5:37 PM, Anthony Petrov wrote:
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.)
Please run Window, Frame, and Dialog automatic regression tests (open
and closed) to ensure no regressions after this change.
--
best regards,
Anthony
On 09/25/2013 04:50 PM, 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 locationByPlatform
property.
Thanks,
Alexandr.
