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

[Anthony Petrov]

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.
>>>
>