<AWT Dev> [8] Review request for CR 8020210 [macosx] JVM crashes in CWrapper$NSWindow.screen(long)

[Artem Ananiev]

On 7/10/2013 7:29 PM, Petr Pchelko wrote:
> Hello, Anthony.
>
> I've updated the fix again. Now we always use GC.
> The new version is available here: http://cr.openjdk.java.net/~pchelko/8020210/webrev.02/

Looks fine to me.

Thanks,

Artem

> With best regards. Petr.
>
> On Jul 10, 2013, at 4:22 PM, Anthony Petrov wrote:
>
>> Thanks for the update. One more suggestion:
>>
>>> 481             Insets i =  ((CGraphicsDevice)getGraphicsDevice()).getScreenInsets();
>>> 482             Rectangle toBounds = getPeer().getGraphicsConfiguration().getBounds();
>>
>> Perhaps we should get the GC first, and then use gc.getDevice() instead of replying on a possibly default GD when requesting the screen insets?
>>
>> The fix looks good otherwise.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 07/10/2013 03:38 PM, Petr Pchelko wrote:
>>> Hello, Anthony.
>>>> Does getGraphicsDevice() always return a non-null value?
>>> Yes. If there's no device it returns the default one.
>>>
>>>> A specific GC is always associated with a window. Why do we use the default one then?
>>> You are right. That's a bug.
>>>
>>>> After removing CWrapper.NSWindow.screen(), do we still use any of the CWrapper.NSScreen methods?
>>> No, this is not used any more. Removed.
>>>
>>> Please see the updated version here:
>>> http://cr.openjdk.java.net/~pchelko/8020210/webrev.01/
>>>
>>> With best regards. Petr.
>>>
>>> On Jul 10, 2013, at 3:00 PM, Anthony Petrov wrote:
>>>
>>>> Hi Petr,
>>>>
>>>> A few questions:
>>>>
>>>> Does getGraphicsDevice() always return a non-null value?
>>>>
>>>> A specific GC is always associated with a window. Why do we use the default one then?
>>>>
>>>> After removing CWrapper.NSWindow.screen(), do we still use any of the CWrapper.NSScreen methods?
>>>>
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 07/10/2013 02:31 PM, Petr Pchelko wrote:
>>>>> Hello, AWT Team.
>>>>>
>>>>> Please review the fix for the following issue:
>>>>> http://bugs.sun.com/view_bug.do?bug_id=8020210
>>>>> The fix is available at:
>>>>> http://cr.openjdk.java.net/~pchelko/8020210/webrev.00/
>>>>>
>>>>> In case the window is offscreen the [NSWindow screen] method could return null, so we crashed in CFRetain(screen). So the CWrapper.Window.screen method is quite dangerous. I've replaced it with getting screen bounds/insets using a GraphicsDevice.
>>>>>
>>>>> With best regards. Petr.
>>>>>
>>>
>