<AWT Dev> [8] Please review fix for 7175704: [macosx] "8" PIT: NPE in GetDisplayMode fullscreen test

[Leonid Romanov]

Looks good.

On 09.10.2012, at 14:38, Alexander Zuev <alexander.zuev at oracle.com> wrote:

> Because on Mac we are currently supporting display mode change operation in non-fullscreen mode.
> 
> On 10/9/12 14:35, Sergey Bylokhov wrote:
>> Also why we don't throw IllegalStateException("Must be in fullscreen mode in order to set display mode") when getFullScreenWindow==null?
>> 
>> 09.10.2012 14:31, Sergey Bylokhov wrote:
>>> 09.10.2012 14:25, Alexander Zuev wrote:
>>>> Sergey,
>>>> 
>>>>  the text message is not specified in the documentation and the new one is already used on MacOS X
>>>> in the jdk7 so i would let it be as it is for consistency.
>>> I don't see a reason not to unify it across tolkits. Why we should use different text on different platforms?
>>>> The check in obtaining matching display mode is already performed in the native code:
>>>> In CGraphicsDevice.m :
>>>> 
>>>>    CGDisplayModeRef closestMatch = getBestModeForParameters(allModes, (int)w, (int)h, (int)bpp, (int)refrate);
>>>>    if (closestMatch != NULL) {
>>>>     ...
>>>>    } else {
>>>>        [JNFException raise:env as:kIllegalArgumentException reason:"Invalid display mode"];
>>>>    }
>>>> 
>>>> With best regards,
>>>> Alexander Zuev
>>>> 
>>>> On 10/9/12 14:12, Sergey Bylokhov wrote:
>>>>> Hi, Alexander
>>>>> On win32 anx X11 we have another exception text message "Invalid display mode", also there is additional check in getMatchingDisplayMode() is it unnecessary on the macosx?
>>>>> 
>>>>> 09.10.2012 12:35, Alexander Zuev wrote:
>>>>>> Hello,
>>>>>> 
>>>>>>  please review my fix for CR 7175704: [macosx] "8" PIT: NPE in GetDisplayMode fullscreen test
>>>>>> 
>>>>>>  The NPE happens because we do not check the passed argument to null before using it. The correct exception according to the specification should be IllegalArgumentException.
>>>>>> 
>>>>>> Bug description is: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7175704
>>>>>> Webrev can be found here: http://cr.openjdk.java.net/~kizune/7175704/webrev.00
>>>>>> 
>>>>>> With best regards,
>>>>>> Alex
>>>>> 
>>>>> 
>>>> 
>>> 
>>> 
>> 
>> 
>