Review request for 7132809: [macosx] MAXIMIZED_BOTH set before setVisible(true) hides Frame

[Artem Ananiev]

Looks fine.

Thanks,

Artem

On 1/30/2012 8:10 PM, Anthony Petrov wrote:
> Artem and I have discussed the issue and here's an updated fix:
>
> http://cr.openjdk.java.net/~anthony/x-11-MaximizedBoth-7132809.1/
>
> The difference is that we now use peer.getState() in the if (!visible)
> branch of the extended state handling code in the
> CPlatformWindow.setVisible(). This is because we really want to cancel
> out the current state of the window as seen from native perspective -
> this is what LWWindoePeer.getPeer() returns.
>
> However, in the if (visible) branch we must apply the state which the
> shared code expects for the window, and as such we continue to use the
> frame.getExtendedState() method there.
>
> Other than that there's no other changes.
>
> --
> best regards,
> Anthony
>
> On 01/30/12 18:42, Anthony Petrov wrote:
>> On 01/30/12 18:41, Artem Ananiev wrote:
>>>>> A minor comment: can we use peer.getState() instead of (instanceof
>>>>> check
>>>>> + target.getExtendedState()) in CPlatformWindow.setVisible()?
>>>>
>>>> The LWWindowPeer.getState() returns the LWWindowPeer.windowState field
>>>> that gets updated only from native notifications
>>>> (notifyIconify()/notifyZoom()). While the window is hidden, the state
>>>> isn't changed, and as such the notifications aren't sent which makes
>>>> the
>>>> windowState field value stale.
>>>
>>> I'm not sure about it. What I see is that LWWindowPeer.setState() is
>>> called from Frame:
>>>
>>> FramePeer peer = (FramePeer)this.peer;
>>> if (peer != null) {
>>> peer.setState(state);
>>> }
>>
>> LWWindowPeer.setState() simply delegates to
>> CPlatformWindow.setWindowState(). If the window is visible, this causes
>> -zoom/-miniaturize native calls, and hence the notifications that
>> results in LWWindowPeer.notifyZoom/notifyIconify() which finally update
>> the LWWindowPeer.windowState.
>>
>> However, if the window is hidden, the CPlatformWindow.setWindowState()
>> is basically a no-op.
>>
>>
>> --
>> best regards,
>> Anthony
>>
>>> At the same time, I don't see where LWWindowPeer.setState() is called
>>> from CPlatformWindow.
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>>> As soon as the CPlatformWindow.setVisible(true) is called it needs to
>>>> read the real current state of the frame, and since the peer's cached
>>>> value is wrong, we read the state from the target.
>>>>
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Artem
>>>>>
>>>>> On 1/26/2012 7:41 PM, Anthony Petrov wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review a fix for
>>>>>> http://bugs.sun.com/view_bug.do?bug_id=7132809
>>>>>> at:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~anthony/x-11-MaximizedBoth-7132809.0/
>>>>>>
>>>>>> We shouldn't apply the extended state when a window is hidden. With
>>>>>> this
>>>>>> fix the CPlatformWindow.setVisible() re-applies the state when the
>>>>>> window gets shown.
>>>>>>
>>>>>> --
>>>>>> best regards,
>>>>>> Anthony