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

[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 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()) {}. 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.
>>>
>