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

[Sergey Bylokhov]

Thanks for clarification.
Fix looks good.

09.10.2012 14:38, Alexander Zuev 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
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>>
>


-- 
Best regards, Sergey.